Skip to content

[Bugfix] Make GPU sync checks safe under torch.compile - #56904

Merged
khluu merged 4 commits into
vllm-project:mainfrom
khluu:agent/fix-gpu-sync-compile
Sep 16, 2026
Merged

khluu merged 4 commits into
vllm-project:mainfrom
khluu:agent/fix-gpu-sync-compile

Conversation

@khluu

@khluu khluu commented Sep 14, 2026 •

Copy link
Copy Markdown
Member

Summary

gpu_sync_debug's patched tensor-copy wrappers currently read a ContextVar
before checking whether Dynamo is tracing. A full-graph compile therefore fails
with an unsupported ContextVar.get() instead of bypassing sync checks as
intended.

This change:

  • checks torch.compiler.is_compiling() before accessing either ContextVar;
  • routes the original C++ Tensor.to descriptor through a weak-referenceable
    Python callable registered with torch.compiler.allow_in_graph, so Dynamo
    treats the call as opaque while AOT/Inductor can still lower it;
  • adds a full-graph torch.compile regression with device and dtype derived
    dynamically from another tensor;
  • runs the two new Voxtral CUDA-graph parameter cases in explicitly spawned
    processes so neither prior pytest CUDA state nor the other graph mode can
    contaminate them.

CI incident and test-selection evidence

Daily main build #88920 first exposed this in
test_voxtral_realtime_cudagraph[FULL_DECODE_ONLY] in the H200 Multimodal
Models (Extended Generation 1) job. PR #51167 added the compiled Voxtral path,
but that exact job was blocked in both its PR build #88857 and post-merge main
build #88899, so the failure was not exercised before the daily run.

I searched open vLLM issues and PRs for the exact ContextVar.get / Voxtral
full-graph failure and did not find an existing fix.

Validation

  • pre-commit run --files vllm/utils/gpu_sync_debug.py tests/utils_/test_gpu_sync_debug.py tests/models/multimodal/generation/test_voxtral_realtime.py — passed all applicable hooks
  • git diff --check — passed
  • Local pytest was not run because this regression requires CUDA and this development host has no GPU.
  • Exact H200 build #88941 passed Async/Inputs/Utils/Worker but exposed the
    saved C++ Tensor.to descriptor as Dynamo-unsupported in the Voxtral
    full-graph test.
  • Replacement #88962 proved that registering the method descriptor directly
    is invalid at import time because allow_in_graph stores a weak reference.
  • Replacement #88969 passed Async/Inputs/Utils/Worker and the original Voxtral
    FULL_DECODE_ONLY compile/CUDA-graph surface at literal head c2fbacb1.
    The following FULL_AND_PIECEWISE parameter then hit the normal startup
    guard with only 25.73/32.5 GiB free after the prior in-process compiled
    runner retained about 6.5 GiB; later failures were cascades from that state.
  • Replacement #89000 passed Async/Inputs/Utils/Worker, but the default CUDA
    isolation helper chose fork after the parent pytest process had initialized
    CUDA. Both Voxtral cases therefore failed before engine compilation with
    Cannot re-initialize CUDA in forked subprocess.
  • #89010 used an invalid copied full SHA and failed in bootstrap; it contains
    no test evidence.
  • #89011 also passed Async/Inputs/Utils/Worker, then reproduced the same two
    pre-compilation CUDA-fork failures. Its literal head still contained the
    helper's default mode because the intended one-line spawn edit had not
    been staged into the prior commit.
  • Current exact replacement #89026 runs only the same two H200 lanes at
    literal signed head f7f21d97caecf810db069402ed02c4624311af32, which
    explicitly passes method="spawn" to the isolation helper.

No model evaluation is required because the production change only alters
debug-wrapper control flow while Dynamo is tracing. The exact affected
generation test is included in targeted CI.

AI assistance disclosure

This change was prepared with AI assistance. Human review is required before
this draft is marked ready or merged.

@khluu

khluu commented Sep 14, 2026

Copy link
Copy Markdown
Member Author

Exact isolated validation is running in Buildkite #88941 at literal signed head b9077f56211b9255fef5e6f63a6e546bcd2dd6f5. It requests only async-engine-inputs-utils-worker (the focused CUDA regression owner) and multi-modal-models-extended-generation-1 (the escaped Voxtral end-to-end lane). Human review remains required before readiness.

@khluu
khluu force-pushed the agent/fix-gpu-sync-compile branch from b9077f5 to 9d6c9de Compare September 15, 2026 00:32
@khluu

khluu commented Sep 15, 2026

Copy link
Copy Markdown
Member Author

Exact validation update:

  • Buildkite #88941 passed H200 Async/Inputs/Utils/Worker, but the direct Multimodal Extended Generation 1 gate found a second full-graph integration issue: the checker now short-circuited its ContextVars, then Dynamo rejected the saved C++ TensorBase.to descriptor when the real Voxtral path supplied device and dtype from another tensor. The later memory/init failures in that job were process fallout, not independent regressions.
  • I updated the same draft so the saved Tensor.to descriptor is explicitly admitted through torch.compiler.allow_in_graph, which is the public Dynamo escape hatch for this C/C++ callable. The regression now matches the real dynamic device=like.device, dtype=like.dtype call and uses the default compiler backend rather than backend="eager".
  • Current signed head: 9d6c9de919e9b014f6ad580ac5441300f72fb297.
  • Local validation: applicable pre-commit hooks passed (ruff, mypy, SPDX, configuration checks, and the rest of the selected suite); git diff --check passed. This host has no CUDA/PyTorch runtime, so the exact GPU test is CI-only.
  • Replacement exact-head Buildkite #88962 requests the same two H200 lanes and is pending.

The PR remains draft and still requires @khluu to review and defend every line before it is made ready.

@khluu
khluu force-pushed the agent/fix-gpu-sync-compile branch from 9d6c9de to 80290ca Compare September 15, 2026 01:21
Co-authored-by: Sherlock <sherlock@raft.ai>
Signed-off-by: Kevin Luu <51931015+khluu@users.noreply.github.com>
@khluu
khluu force-pushed the agent/fix-gpu-sync-compile branch from 80290ca to c2fbacb Compare September 15, 2026 01:22
@khluu

khluu commented Sep 15, 2026

Copy link
Copy Markdown
Member Author

Replacement exact #88962 caught a genuine flaw in the previous revision before tests collected: torch.compiler.allow_in_graph(_torch_to) attempts to weak-reference the saved C++ method_descriptor, so importing gpu_sync_debug.py failed with TypeError: cannot create weak reference to 'method_descriptor' object in both selected H200 lanes.

I corrected that by registering a weak-referenceable Python wrapper instead. The wrapper calls the original descriptor, preserving the no-recursion property; Dynamo treats the wrapper as opaque and AOT/Inductor can trace/lower the underlying call. I also rebased the draft onto current main and reran all applicable pre-commit hooks plus git diff --check successfully.

Current signed head: c2fbacb13756e9b54a4ebc3902af02fe6a794c86
Replacement exact gates: https://buildkite.com/vllm/ci/builds/88969

#88969 is pinned to that literal head and only the H200 Async/Inputs/Utils/Worker and Multimodal Extended Generation 1 lanes. It is currently building the image; this draft remains held until both gates pass.

@khluu

khluu commented Sep 15, 2026

Copy link
Copy Markdown
Member Author

Exact H200 build #88969 at c2fbacb13756e9b54a4ebc3902af02fe6a794c86 validated the source fix and exposed one independent test-isolation problem:

  • H200 Async Engine / Inputs / Utils / Worker: passed.
  • Voxtral FULL_DECODE_ONLY: passed through Dynamo, Inductor, CUDA graph capture, and inference. This is the original failure surface (the previous head failed while Dynamo traced the saved C++ Tensor.to descriptor).
  • The following FULL_AND_PIECEWISE parameter started with only 25.73/32.5 GiB free because the prior in-process compiled runner retained about 6.5 GiB, then failed the normal 0.9 startup guard; two later tests cascaded from that memory state.

Commit bee6beb40f now gives each newly added Voxtral CUDA-graph parameter a fresh process, matching the isolation pattern used by other compile-heavy tests. All applicable pre-commit hooks and git diff --check pass locally. I am launching the same two exact lanes at the literal new head; the PR remains draft for human review.

@mergify mergify Bot added multi-modality Related to multi-modality (#4194) mistral Related to Mistral models labels Sep 15, 2026
@khluu

khluu commented Sep 15, 2026 •

Copy link
Copy Markdown
Member Author

Current replacement exact H200 build #88988 uses literal signed head 544bf9df374ebe65a0f59defa76caf7a42400fba and only async-engine-inputs-utils-worker plus multi-modal-models-extended-generation-1. Superseded #88987 was canceled before tests when I amended the test-isolation commit to add the AI co-author trailer. (This API-triggered replacement does not publish a PR status, so the build link is the authoritative gate.)

@khluu
khluu force-pushed the agent/fix-gpu-sync-compile branch from bee6beb to 544bf9d Compare September 15, 2026 02:33
Co-authored-by: Sherlock <sherlock@raft.ai>
Signed-off-by: Kevin Luu <51931015+khluu@users.noreply.github.com>
@khluu
khluu force-pushed the agent/fix-gpu-sync-compile branch 2 times, most recently from 544bf9d to a6b806a Compare September 15, 2026 04:06
@khluu

khluu commented Sep 15, 2026

Copy link
Copy Markdown
Member Author

Exact Buildkite #89000 update:

  • H200 Async/Inputs/Utils/Worker passed at 544bf9df374e.
  • Multimodal Extended Generation 1 reached both new Voxtral CUDA-graph parameters, but the default isolation helper uses fork on CUDA. Earlier tests had already initialized CUDA in the pytest parent, so both child processes failed immediately with Cannot re-initialize CUDA in forked subprocess.
  • This is an isolation-mechanism failure, not a recurrence of the original Dynamo graph break: no Voxtral engine reached compilation in those forked children.

I changed the decorator to the helper's explicit spawn mode and force-pushed signed/DCO head a6b806ade02639b2412f1551898f766ff92dc5b3. All applicable local pre-commit hooks and diff checks pass. Replacement exact build #89010 uses the literal new head and only the same Async/Utils plus Multimodal Extended Generation 1 gates.

AI assistance was used; human line review remains required before readiness.

@khluu

khluu commented Sep 15, 2026

Copy link
Copy Markdown
Member Author

Correction: I copied an invalid full suffix after the displayed short SHA into #89010, so bootstrap correctly rejected a6b806ade026... as not a repository ref. That build contains no test evidence.

The verified local, fork, GitHub pull-ref, and PR head are all exactly a6b806ade04e45c10925677a14d5127deb670d38. Replacement #89011 uses that literal head and the same two H200 gates. I also corrected the PR body below; #89010 must not be cited as validation.

Co-authored-by: Sherlock <sherlock@raft.ai>

Signed-off-by: Kevin Luu <51931015+khluu@users.noreply.github.com>
@khluu

khluu commented Sep 15, 2026

Copy link
Copy Markdown
Member Author

Exact #89011 closed red: Async/Inputs/Utils/Worker passed, while both new Voxtral parameters failed before engine compilation with Cannot re-initialize CUDA in forked subprocess (30 passed / 2 failed in the lane). Inspection found that its literal head still had @create_new_process_for_each_test()—the intended explicit-spawn edit was present locally but had not been staged into the prior commit. I corrected the PR body so #89011 is not misrepresented as spawn evidence.

Current signed/DCO head f7f21d97caecf810db069402ed02c4624311af32 now contains @create_new_process_for_each_test(method="spawn"). All applicable pre-commit hooks for the changed test file and git diff --check pass. Replacement exact H200 build #89026 is pinned to that literal head and only the same Async/Utils plus Multimodal Extended Generation 1 gates.

The draft remains held for terminal exact evidence and human line review. AI assistance was used.

@khluu

khluu commented Sep 15, 2026

Copy link
Copy Markdown
Member Author

Terminal exact validation at current head f7f21d97caecf810db069402ed02c4624311af32:

  • Buildkite #89026 passed all four selected jobs
  • H200 Async Engine / Inputs / Utils / Worker passed
  • H200 Multimodal Models (Extended Generation 1) passed with 32 passed, 26 skipped, 64 deselected, then mapping 4 passed / 1 skipped
  • both decisive fresh-process cases passed:
    • test_voxtral_realtime_cudagraph[FULL_DECODE_ONLY]
    • test_voxtral_realtime_cudagraph[FULL_AND_PIECEWISE]

This confirms explicit spawn isolation avoids the inherited-CUDA failure and prevents the first graph-mode run from leaking memory into the second. The PR remains draft for human line review.

AI disclosure: This validation report was prepared with AI assistance.

@njhill
njhill marked this pull request as ready for review September 15, 2026 23:41

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@njhill

njhill commented Sep 15, 2026

Copy link
Copy Markdown
Member

/ci test

@njhill
njhill enabled auto-merge (squash) September 15, 2026 23:41
@github-actions github-actions Bot added the ready ONLY add when PR is ready to merge/full CI is needed label Sep 15, 2026
@khluu
khluu disabled auto-merge September 16, 2026 04:37
@khluu
khluu merged commit af1c014 into vllm-project:main Sep 16, 2026
8 checks passed
@github-project-automation github-project-automation Bot moved this from To triage to Done in torch.compile integration Sep 16, 2026
@khluu khluu added this to the v0.30.0 cherry picks milestone Sep 16, 2026
khluu added a commit that referenced this pull request Sep 17, 2026
Signed-off-by: Kevin Luu <51931015+khluu@users.noreply.github.com>
Co-authored-by: Sherlock <sherlock@raft.ai>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working mistral Related to Mistral models multi-modality Related to multi-modality (#4194) ready ONLY add when PR is ready to merge/full CI is needed torch.compile

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants