fix(BACKEND-ROCM): a narrow predicate makes the device-fit refusal reachable on ROCm - #2027
Open
joral wants to merge 3 commits into
Open
fix(BACKEND-ROCM): a narrow predicate makes the device-fit refusal reachable on ROCm#2027joral wants to merge 3 commits into
joral wants to merge 3 commits into
Conversation
Issue mudler#1934: RocmPlatform::needs_weight_staging() is stale-false, so CheckDeviceWeightFit's one call site never runs on ROCm regardless of mudler#1870/mudler#1935's arithmetic fix -- measured directly, a real checkpoint load with VT_DEVICE_WEIGHT_BUDGET_BYTES=1 produced no refusal. Records why flipping needs_weight_staging() itself was rejected (IndexedGdnStateIoEnabled already checks op registration directly and would regress; several sibling GDN consumers have no such fallback and would activate unverified kernels) in favor of a new, narrower Platform predicate before any implementation lands. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-sonnet-5 [claude-code]
…achable on ROCm Issue mudler#1934. CheckDeviceWeightFit's one call site read target.needs_weight_staging(), which RocmPlatform hardcodes false -- a stale W0-era placeholder never revisited despite ROCm landing grouped-GEMM, MoE combine/gate, ROCM_ATTN and hipGraph decode capture since. Measured directly: VT_DEVICE_WEIGHT_BUDGET_BYTES=1 against a real checkpoint produced no refusal at all, while the actual device allocation the refusal guards (ResidentWeight's d.b.Alloc) runs unconditionally on any non-CPU platform since issue mudler#125's is_cpu() fix -- so mudler#1870's crash stayed reachable regardless of that row's arithmetic fix. Rejected flipping needs_weight_staging() itself: that flag also gates several GDN kernel-dispatch defaults (merged/packed projections, fp8/bf16 resident prep) with no op-registration fallback, which would newly activate unconditionally on ROCm the moment it moved -- IndexedGdnStateIoEnabled already takes ROCm's fast arm today by checking op registration directly rather than trusting this flag, and flipping it would remove that check for that one consumer while blindly trusting the others. Adds Platform::allocates_bounded_device_memory(), default-delegating to needs_weight_staging() (byte-identical for every platform that overrides neither), RocmPlatform overrides only this new method to true. residency_policy() now carries a real, once-probed device_memory_total_bytes via a new HIP-free vt::rocm::DeviceMemoryTotalBytes free function (mirrors DeviceAvailable()'s registrar-independence reasoning; not Backend::DeviceMemoryInfo, whose live per-request probe is a different, currently-CUDA-dead seam per issue mudler#1126). The other two ResidencyPolicy fields stay at their existing default. model_loader.cpp's ONE CheckDeviceWeightFit call site now reads the new predicate; the other needs_weight_staging() read in the same function (the streamed-expert-lane condition) is untouched. Red-before: every new test references the method before it exists, captured by stashing the implementation and rebuilding (test_platform and test_gguf_device_fit_reach both fail to compile). Green-after: both pass in full, including new cases proving the two predicates move independently in both directions and that every existing GDN kernel-dispatch consumer of needs_weight_staging() is unmoved. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-sonnet-5 [claude-code]
…same box Records the before/after vllm-cli transcript on this gfx1200 card (no refusal -> named refusal, same command, same file, VT_DEVICE_WEIGHT_BUDGET_BYTES=1) and the rejected alternatives (flipping needs_weight_staging(), reusing Backend::DeviceMemoryInfo) with the reasons. Also notes mudler#2021/mudler#2022, the unrelated GCC 15 build break this row's own verification needed applied and reverted locally, never landing on this branch. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-sonnet-5 [claude-code]
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Makes the
#1123/#1870load-time device-fit refusal reachable on ROCm —the actual remaining blocker
#1870was reopened against — without movingneeds_weight_staging()or any of its other consumers.CheckDeviceWeightFit's one call site readstarget.needs_weight_staging()to decide whether to run at all.
RocmPlatformhardcodes thatfalse— aW0-era comment says a discrete AMD card "will eventually answer true...
Revisit at M2", never revisited despite ROCm landing grouped-GEMM, MoE
combine/gate,
ROCM_ATTNand hipGraph decode capture since. Measureddirectly on this box (gfx1200):
VT_DEVICE_WEIGHT_BUDGET_BYTES=1against areal checkpoint produced no refusal at all. Meanwhile the real device
allocation the refusal exists to guard (
ResidentWeight'sd.b.Alloc) runsunconditionally on any non-CPU platform since issue #125's
is_cpu()fix —so
#1870's crash stayed reachable regardless of that row's arithmetic fix.Why not just flip
needs_weight_staging()Read literally:
IndexedGdnStateIoEnabled(qwen3_5.cpp) already takesROCm's fast arm today, independent of this flag — its non-staging branch
falls through to an op-registration check
(
kCausalConv1dUpdate/kGdnDecode/kGdnStateGather/kGdnStateScatter, allfour registered for ROCm) rather than trusting the flag. Flipping it would
make this consumer take the other branch, which drops that check and just
assumes the ops exist — a regression for this one consumer, not an
improvement.
MergedGdnBaEnabled/MergedGdnQkvzEnabledand the packed-decode/ fp8-resident-prep gates have no such fallback at all: flipping the flag
would activate them on ROCm unconditionally, with no verification that the
underlying merged/packed GEMM and fp8 paths are numerically correct there.
So this PR adds a new, narrow
Platform::allocates_bounded_device_memory()instead — default-delegates to
needs_weight_staging()(byte-identical forevery platform that overrides neither),
RocmPlatformoverrides only thisnew method to
true.needs_weight_staging()itself, and all six-plus ofits other GDN-dispatch consumers, are unmoved — tests pin this in both
directions.
residency_policy()now carries a real, once-probeddevice_memory_total_bytesvia a new HIP-freevt::rocm::DeviceMemoryTotalBytes(mirrors
DeviceAvailable()'s registrar-independence reasoning; deliberatelynot
Backend::DeviceMemoryInfo, a live per-request probe with a different,currently CUDA-dead consumer per issue #1126).
Real hardware, both directions, same command, same box
vllm-cli --model .../Bonsai-27B-Q1_0.gguf --device auto --max-tokens 4withVT_DEVICE_WEIGHT_BUDGET_BYTES=1:unrelated dequant error three load-stages later.
device 'rocm' cannot serve this GGUF: staging its weights needs at least 3787168768 bytes (3.52 GiB) of device memory across 851 tensors, ... and this device's memory pool is 1 bytes (0.00 GiB).Red-before/green-after on both
test_platformandtest_gguf_device_fit_reach: every new case fails to compile against theunfixed tree (stash + rebuild), both suites pass in full afterward on this
box — CPU build and, separately, a full ROCm build run on real
gfx1200hardware (17/17 cases, 133/81 assertions).
Note: an unrelated build break found along the way
A fresh CPU build of the claim base failed under GCC 15's
-Werror=nonnullin unrelated Tenstorrent debug-dump code. Filed and fixed separately as
#2021/#2022 rather than riding this branch — this PR's own local
verification applied that fix and reverted it before every commit here, so
it does not appear in this diff.
Spec:
.agents/specs/rocm-device-fit-bounded-memory.md(this PR).FOLLOWING_AGENTS_PROTOCOL
Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: AGENT:claude-sonnet-5 [claude-code]