test(VT-REFTIER-HOST-ADDRESSABLE): check CUDA's host-addressability answer in the two halves it actually has - #1778
Merged
Merged
Conversation
…wer where it can be checked, not where it can only be skipped #1635 offered two ways to discharge its remaining owed item: give the real `CudaBackend` a pinning test, or state in the record that the default holds unpinned. This spec takes the first, in the only shape that runs. A runtime test was rejected on a CI fact rather than a preference. No job in `.github/workflows/ci.yml` has a GPU, `cuda-fat-build` configures `-DVLLM_CPP_BUILD_TESTS=OFF` and builds the `vllm` target alone, and the CUDA registrar leaves `kCUDA` unregistered when `cudaGetDeviceCount` finds no device. A `TEST_CASE` reading the real backend would therefore skip on every lane forever, and a skip reads as a pass, which is the shape of evidence #1635 was filed about. The record's claim is instead split into the two halves it actually has, each checked where it executes: a `static_assert` in `src/vt/cuda/cuda_backend.cu` that `CudaBackend` declares no override, compiled by `cuda-fat-build`; and a case in `tests/vt/test_backend.cpp` that the inherited default is `false`, run by every host lane. What stays unobserved -- a live `CudaBackend` answering on hardware -- is named as such rather than left to be inferred. The implementation follows in this branch, so the commit order proves the spec came first. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: Claude:claude-opus-5 [Claude Code]
…nswer in the two halves it actually has `CudaBackend::DeviceMemoryIsHostAddressable()` gates the portable reference tier, the `VT_ADOPT_DEVICE_BYTES` adoption and the logits-processor bounce, and nothing in the tree held it. The test cited as the pin reads a different backend: `FakeUnifiedAddressablePlatform` reports `device_type() == kCUDA` while its `backend()` returns `vt::GetBackend(DeviceType::kCPU)`. A runtime test cannot replace it, and the reason is a CI fact. No job has a GPU; `cuda-fat-build` is the only job with a CUDA toolchain and it builds with `-DVLLM_CPP_BUILD_TESTS=OFF`; and the CUDA registrar leaves `kCUDA` unregistered when `cudaGetDeviceCount` finds no device. A case reading the real backend would skip on every lane forever, and a skip reads as a pass -- the same shape of evidence #1635 was filed about. The claim has two independent halves, and each is now checked where it executes. `src/vt/cuda/cuda_backend.cu` gains a `static_assert` that `decltype(&CudaBackend::DeviceMemoryIsHostAddressable)` is `bool (Backend::*)() const`: taking the address of an inherited member through a derived class yields a pointer-to-member of the DECLARING class, so that type holds exactly while `CudaBackend` declares no override. `tests/vt/test_backend.cpp` gains a `Backend` subclass that declares no override and requires the inherited default to be `false` -- every other fake in the tree overrides the method and so measures its own override, never the default. `tests/vt/test_cuda_backend.cpp` gains the observation on a real device. It asserts nothing without a GPU and says so in its skip message, so it is the belt and not the braces. Mutation-proven on both halves. Adding an override returning `true` to `CudaBackend` fails `nvcc` 13.3.33 with the intended message (rc=2), an override returning `false` fails it identically, and the restored file recompiles clean at the same sha256. Flipping the base default in `include/vt/backend.h` to `true` turns the new case red (2 of 3 assertions, rc=1), and the restored header is byte-identical. `docs/ENVIRONMENT.md` said nothing pinned the real `CudaBackend`, which this change makes false, so it now names both halves and warns off the wrong citation. Refs #1635 FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: Claude:claude-opus-5 [Claude Code]
Keeps the branch current before the gate run; no conflict. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: Claude:claude-opus-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.
CudaBackend::DeviceMemoryIsHostAddressable()gates the portable CPU referencetier (
src/vt/op_provider.cpp), the weight loader'sVT_ADOPT_DEVICE_BYTESadoption (
src/vllm/model_executor/models/qwen3_5_weights.cpp) and thelogits-processor bounce (
src/vllm/v1/sample/logits_processor/builtin.cpp).Being wrong there hands a device pointer to a host memcpy, which is the SIGSEGV
that #844 and #1435 measured. Nothing in the tree held that answer.
The test cited as the pin reads a different backend:
FakeUnifiedAddressablePlatformintests/vllm/platforms/test_platform.cppreports
device_type() == kCUDAwhile itsbackend()returnsvt::GetBackend(DeviceType::kCPU), so itsCHECK_FALSEmeasures the CPUbackend. #1639 corrected that fixture's comment; this change supplies the pin
the record was still owed.
A runtime pin was rejected on a CI fact, not a preference
No job in
.github/workflows/ci.ymlhas a GPU.cuda-fat-buildis the only jobwith a CUDA toolchain, it runs the
nvidia/cuda:13.3.0-develcontainer onubuntu-latest, it configures-DVLLM_CPP_BUILD_TESTS=OFFand it builds thevllmtarget alone. The CUDA registrar returns early whencudaGetDeviceCountfinds no device, so
vt::GetBackend(kCUDA)throws on every machine thisproject's CI owns. A
TEST_CASEreading the real backend would report a skip onevery lane forever, and a skip reads as a pass. That is the shape of evidence
#1635 was filed about, so writing one and calling the debt discharged would
repeat the defect rather than close it.
One claim, two halves, each checked where it executes
The record's claim decomposes, and neither half alone is sufficient.
CudaBackend's answer IS the base default.src/vt/cuda/cuda_backend.cugains a
static_assertbeside the class requiringdecltype(&CudaBackend::DeviceMemoryIsHostAddressable)to bebool (Backend::*)() const. Taking the address of an inherited member through aderived class yields a pointer-to-member of the class that DECLARES it, so that
type holds exactly while
CudaBackenddeclares no override of its own, andbecomes
bool (CudaBackend::*)() constthe moment somebody adds one. It fires onANY override, including one returning
false, because an override invalidatesthe reasoning whatever it returns.
cuda-fat-buildcompiles this translationunit on every push.
The base default is
false.tests/vt/test_backend.cppgains aBackendsubclass that implements the pure virtuals and deliberately declares no
DeviceMemoryIsHostAddressable. Every other fake in the tree overrides thatmethod and takes the answer as a constructor argument, so each measures its own
override and none reads the default. This one does, and it answers
UnifiedMemory() == trueon purpose so the case cannot pass by the twopredicates happening to agree. Every host lane runs it.
Together they are a mechanical proof of
CudaBackendansweringfalse, checkedon surfaces that execute rather than skip.
Mutation evidence
Half one, against
nvcc13.3.33 from the CI container, compiling the real file:nvcc -std=c++20 -Iinclude -Isrc -c src/vt/cuda/cuda_backend.cud9d11f96...+ bool DeviceMemoryIsHostAddressable() const override { return true; }static assertion failedatcuda_backend.cu(363)falsed9d11f96...Half two,
include/vt/backend.hdefault flipped totrueand the targetrebuilt:
test_backend -tc="Backend::DeviceMemoryIsHostAddressable defaults to false"80edf388...assertions: 3 | 3 passed, rc=0false->trueassertions: 3 | 1 passed | 2 failed, rc=180edf388...assertions: 3 | 3 passed, rc=0Both mutated builds compiled, so neither red is a build failure wearing a test
failure, and both files were restored byte-for-byte and re-verified by sha256
and by re-running the check.
Suites:
test_backend,test_backend_cross_device,test_backend_cross_device_vt_attn_decode_d128,test_cuda_backend,test_op_provider,test_reference_tier-- 6/6 pass;test_backendalone is9 cases / 51 assertions.
What is still not held, stated rather than left to be inferred
No CI surface observes a live
CudaBackendobject answering the question,because no CI surface has a device.
tests/vt/test_cuda_backend.cppgains thatobservation using the skip convention every case in that file already follows,
and on a CPU host it reports
assertions: 0and says in its own skip messagethat nothing was observed and where the answer actually lives. It is the
empirical belt to the two structural braces. Citing it as the pin would repeat
#1635 exactly, and both the case and the spec say so.
docs/ENVIRONMENT.mdclaimed nothing pinned the realCudaBackend, which thischange makes false, so it now names both halves and warns off the wrong
citation. The
#1502row of.agents/issue-index.mdkeeps the original wrongcitation, because that index is append-only and can never be edited; no row is
edited here.
Fixes #1635
FOLLOWING_AGENTS_PROTOCOL
Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: Claude:claude-opus-5 [Claude Code]