docs(VT-REFTIER-HOST-ADDRESSABLE): VT_ADOPT_DEVICE_BYTES reaches Metal and integrated ROCm, and its numbers do not (#1502) - #1615
Closed
localai-bot wants to merge 2 commits into
Closed
Conversation
…l and integrated ROCm, and its numbers do not (#1502) `docs/ENVIRONMENT.md` described the lever's reach as "Vulkan today" and closed with "No effect on CUDA/CPU/Metal, whose backends do not advertise the property". Both halves stopped being true at `cffe59b02` (#1477). That change moved `ReferenceTierEligible` off the wider `UnifiedMemory()` onto `Backend::DeviceMemoryIsHostAddressable()`, and added truthful overrides so no backend silently lost the reference tier. `MetalBackend` now answers `MetalContext::unified_memory()` and `RocmBackend` answers its `unified_memory_`. The weight loader gates this lever on exactly that predicate, at both `AdoptDeviceBytesAsHost` branches in `qwen3_5_weights.cpp`, so the lever acts on Apple silicon and on an integrated ROCm part. The correction is not "add two backend names". Every number in that row is GB10 through Vulkan, and nobody has measured the lever on either new arm. The row read as if the measurement covered the reach, so it now says which backends it is MEASURED on and which merely satisfy the predicate. CUDA and CPU are unchanged and still inert: neither overrides the default `false`, which `tests/vllm/platforms/test_platform.cpp` pins for GB10, and the CPU backend reporting `UnifiedMemory() == true` while the narrower predicate stays `false` is the whole reason the two properties are separate. Verified against the tree rather than against the issue: the three overrides, the two gate sites, the absent CUDA and CPU overrides, and the GB10 assertion were each read at this head. The measurement on Metal and on integrated ROCm stays owed. It needs an Apple-silicon box or an integrated AMD part, which this row has not taken, so it is recorded under `## Owed` in the spec that widened the predicate rather than left to be discovered from a document that now promises less than it did. No checker relates a backend predicate to a prose sentence, and none can be built cheaply, which is why this landed as a document defect rather than a red gate. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-opus-5 [Claude Code]
Collaborator
Author
|
Superseded by the branch rebased onto current |
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.
docs/ENVIRONMENT.mddescribed the lever's reach as "Vulkan today" and closedwith "No effect on CUDA/CPU/Metal, whose backends do not advertise the property".
Both halves stopped being true at
cffe59b02(#1477).That change moved
ReferenceTierEligibleoff the widerUnifiedMemory()ontoBackend::DeviceMemoryIsHostAddressable(), and added truthful overrides so nobackend silently lost the reference tier.
MetalBackendnow answersMetalContext::unified_memory()andRocmBackendanswers itsunified_memory_.The weight loader gates this lever on exactly that predicate, at both
AdoptDeviceBytesAsHostbranches inqwen3_5_weights.cpp, so the lever acts onApple silicon and on an integrated ROCm part.
The correction is not "add two backend names". Every number in that row is GB10
through Vulkan, and nobody has measured the lever on either new arm. The row read
as if the measurement covered the reach, so it now says which backends it is
MEASURED on and which merely satisfy the predicate. CUDA and CPU are unchanged
and still inert: neither overrides the default
false, whichtests/vllm/platforms/test_platform.cpppins for GB10, and the CPU backendreporting
UnifiedMemory() == truewhile the narrower predicate staysfalseisthe whole reason the two properties are separate.
Verified against the tree rather than against the issue: the three overrides, the
two gate sites, the absent CUDA and CPU overrides, and the GB10 assertion were
each read at this head.
The measurement on Metal and on integrated ROCm stays owed. It needs an
Apple-silicon box or an integrated AMD part, which this row has not taken, so it
is recorded under
## Owedin the spec that widened the predicate rather thanleft to be discovered from a document that now promises less than it did.
No checker relates a backend predicate to a prose sentence, and none can be built
cheaply, which is why this landed as a document defect rather than a red gate.
Fixes #1502
FOLLOWING_AGENTS_PROTOCOL
Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: AGENT:claude-opus-5 [Claude Code]