Skip to content

[CI] Fix test_resolution_is_reproducible after cuda_ipc became opt-in - #34746

Merged
hnyls2002 merged 1 commit into
mainfrom
mmangkad/fix-mm-feature-transport-resolution-test
Aug 13, 2026
Merged

hnyls2002 merged 1 commit into
mainfrom
mmangkad/fix-mm-feature-transport-resolution-test

Conversation

@mmangkad

@mmangkad mmangkad commented Aug 13, 2026

Copy link
Copy Markdown
Collaborator

Motivation

test/registered/unit/server_args/test_resolution_is_reproducible.py is failing on the GPU
registrations (seen on base-b-test-1-gpu-small, 1-gpu-5090):

FAIL: test_a_resolution_does_not_leak_into_the_next (intermediate='multimodal')
  File "test/registered/unit/server_args/test_resolution_is_reproducible.py", line 343
    self.assertEqual(after.mm_feature_transport, expected)
AssertionError: 'cpu' != 'cuda_ipc'

#34662 (69bf601e3c, "fix: restore VLM nightly regression coverage") made CUDA IPC opt-in: the
nnodes == 1 arm of _handle_multimodal_feature_transport now resolves to cpu instead of
cuda_ipc, because even an idle IPC pool consumes HBM that would otherwise back the KV cache. That
PR updated the CLI help text and the TestMultimodalFeatureTransport cases in test_server_args.py,
but this test still asserted expected = "cuda_ipc" if is_cuda() else "cpu". The CPU and ROCm
registrations kept passing because they already expect cpu, so only the CUDA runners went red.

The policy change itself is intentional, so the fix belongs in the test.

There is a second, quieter consequence worth calling out. With auto-resolution now picking cpu
everywhere, the multimodal intermediate writes exactly what the next resolution would have picked on
its own, so the documented _STICKY_ACROSS_RESOLUTIONS exception no longer pins anything observable.
Flipping the expectation to cpu and stopping there would have left a green but vacuous assertion.

Modifications

Test-only; no runtime behaviour is changed.

  1. Add a CUDA-gated multimodal_cuda_ipc shape (_CUDA_IPC_SHAPES) that requests
    mm_feature_transport="cuda_ipc" explicitly. Post-fix: restore VLM nightly regression coverage #34662 this is the only construction under
    which the SGLANG_USE_CUDA_IPC_TRANSPORT carry is observable — the handler writes 1 only for
    cuda_ipc, and the following text-only resolution adopts it via the legacy_ipc_is_set branch —
    and it is also the only remaining coverage of the handler's cuda_ipc arm, which no
    auto-resolution reaches any more. It is wired into _SHAPES (dual-resolve and sibling matrices)
    as well as the leak test's intermediates. Gated on is_cuda() rather than
    torch.cuda.is_available() because the handler raises for cuda_ipc off NVIDIA CUDA, ROCm
    included.

  2. Generalize the sticky assertion to run for every intermediate instead of only the
    label == "multimodal" one (every resolution runs the transport handler, so
    after.mm_feature_transport is pinned for all of them), and derive the expected value from what
    the intermediate itself resolved to rather than from its kwargs. What carries is the legacy
    boolean, not the tri-state field, so any non-cuda_ipc selection — including a cuda_vmm shape
    if one is ever added — carries cpu.

Verification

Run Result
CUDA host, this branch 4 tests / 16 subtests pass
No visible NVIDIA device (both gates drop out) 4 tests / 10 subtests pass
Pre-change, CUDA host fails at intermediate='multimodal' — reproduces the CI failure
Mutation check: handler's env write stubbed to always set("0") fails at intermediate='multimodal_cuda_ipc', so the new shape genuinely guards the carry rather than passing trivially
Mutation check: hypothetical explicit cuda_vmm intermediate passes under the new derivation; would have wrongly demanded cuda_vmm under a kwargs-derived expectation
Sibling suites: test_server_args.py (142), test_mm_process_config.py (42) pass
pre-commit run --files <file> clean

Accuracy Tests

Not applicable — this changes a ServerArgs resolution unit test only; no kernel, model forward, or
output path is touched.

Speed Tests and Profiling

Not applicable — no inference path is touched.

Checklist

Review and Merge Process

  1. Ping Merge Oncalls to start the process. See the PR Merge Process.
  2. Get approvals from CODEOWNERS and other reviewers.
  3. Trigger CI tests with comments or contact authorized users to do so.
    • Common commands include /tag-and-rerun-ci, /tag-run-ci-label, /rerun-failed-ci
  4. After green CI and required approvals, ask Merge Oncalls or people with Write permission to merge the PR.

CI States

Latest PR Test (Base): 🚫 Run #31710106430
Latest PR Test (Extra): ❌ Run #31710105883

#34662 made CUDA IPC opt-in: the `nnodes == 1` arm of
`_handle_multimodal_feature_transport` now resolves to `cpu` instead of
`cuda_ipc`. `test_a_resolution_does_not_leak_into_the_next` still expected
`cuda_ipc` and failed on the GPU registrations with `'cpu' != 'cuda_ipc'`;
the CPU and ROCm suites passed because they already expect `cpu`.

Flipping the expectation to `cpu` alone would have hidden a second
consequence of that change. With auto-resolution picking `cpu` everywhere,
the multimodal intermediate now writes exactly what the next resolution
would have picked on its own, so the `_STICKY_ACROSS_RESOLUTIONS` exception
stopped pinning anything observable. Add a CUDA-gated `multimodal_cuda_ipc`
shape that opts in explicitly: post-#34662 it is the only construction under
which the SGLANG_USE_CUDA_IPC_TRANSPORT carry is visible, and the only
coverage left of the handler's `cuda_ipc` arm. It is wired into `_SHAPES`
(dual-resolve and sibling matrices) as well as the leak test's intermediates.

Also generalize the sticky assertion to run for every intermediate instead of
one label, and derive the expected value from what the intermediate itself
resolved to rather than from its kwargs. What carries is the legacy boolean,
which the handler writes as 1 only for `cuda_ipc`, so any other selection --
including a `cuda_vmm` shape if one is ever added -- carries `cpu`.

Verified on a CUDA host: 4 tests / 16 subtests pass, and the leak case still
fails if the handler's env write is stubbed out. The gates drop out to 10
subtests without a visible NVIDIA device.
@mmangkad

Copy link
Copy Markdown
Collaborator Author

/rerun-test test/registered/unit/server_args/test_resolution_is_reproducible.py

@github-actions

github-actions Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Results for /rerun-test test/registered/unit/server_args/test_resolution_is_reproducible.py:

🚀 1-gpu-5090 (1 test): ✅ View workflow run

cd test/ && python3 registered/unit/server_args/test_resolution_is_reproducible.py

@Kangyan-Zhou

Copy link
Copy Markdown
Collaborator

/tag-and-rerun-ci

@hnyls2002
hnyls2002 merged commit 96db53e into main Aug 13, 2026
197 of 225 checks passed
@hnyls2002
hnyls2002 deleted the mmangkad/fix-mm-feature-transport-resolution-test branch August 13, 2026 19:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants