[Bug] Fix DP-attention state-capturer crash on cuda_graph_batch=None (#30712) - #31100
aryanyadav0402 wants to merge 1 commit into
Conversation
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
@misaAle thanks again for pointing me at this one. The fix records the padded per-rank extent of the runner that actually executed on One thing I want to be transparent about: I couldn't reproduce the original runtime crash end-to-end on my hardware. On this build prefill CUDA-graph capture fails during warmup for every MoE/DP model I tried (unrelated to this bug — it happens on stock too, before any request), so the Separately, while testing I hit what looks like a distinct defect in the same subsystem: during prefill-graph capture with DP attention, |
…roject#30712) The routed-expert and indexer state capturers were passed cuda_graph_batch=getattr(self.decode_cuda_graph_runner, "bs", None) from ModelRunner.forward(), regardless of which runner actually executed the forward pass. On the prefill/piecewise CUDA-graph path (or when the decode runner was not the one that ran), that resolved to None, so get_dp_local_slice_cpu computed dp_rank * None and raised TypeError. Record the padded per-rank extent of the runner that actually executed on ModelRunnerOutput.cuda_graph_padded_extent (decode -> padded bs; prefill graph -> padded token count; eager -> None) and have the capturers read it. This makes the invariant "can_run_graph=True implies the extent is not None" hold by construction, rather than a None-guard that would silently fall back to the eager prefix-sum layout (wrong slice for dp_rank > 0). Also removes the over-defensive getattr, per the no-getattr-defensive convention. Add a CPU regression test that exercises the fix wiring itself: it asserts ModelRunnerOutput exposes cuda_graph_padded_extent, defaults it to None on the eager path, and that the graph-path extent feeds a valid rank-padded slice (not dp_rank * None). The test fails on the unpatched tree and passes with the fix; it also pins the underlying get_dp_local_slice_cpu slicing invariant. Developed with AI assistance (Claude); the change is authored and owned by me. Signed-off-by: Aryan Yadav <78024710+aryanyadav0402@users.noreply.github.com> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
24735c5 to
b1569bc
Compare
|
@aryanyadav0402 yeah actually I'm not able to reproduce the issue on the standard sglang docker image. I was using a SlimeRL image: Regardless, seems like this combination has just not been tested and the prefill path is disabled by intent. Would recommend following up on the issue you filed. Thank you for investigating! |
|
Thanks @misaAle — that's really helpful, and it lines things up nicely. The Totally agree the combo is gated by intent today. Since the driver here is the SlimeRL routed-experts-capture path, would you be open to treating these two as the enablement work for DP-attn × prefill-graph × routed-experts? Concretely: this PR (#31100) is a safe, backward-compatible fix — it only records the actually-executed runner's padded extent and removes a banned defensive |
Motivation
Fixes #30712.
On the prefill / piecewise CUDA-graph path with DP attention and routed-expert return enabled, the DP-attention state capturers crash with:
Root cause. The routed-expert and indexer state capturers in
ModelRunner.forward()were passed:regardless of which runner actually executed the forward pass. On the prefill/piecewise CUDA-graph path (or whenever the decode runner was not the one that ran), that resolves to
None, soget_dp_local_slice_cpucomputesdp_rank * Noneand raises fordp_rank > 0.get_dp_local_slice_cpuitself was already correct — the defect was the caller feeding it the wrong runner's extent.Modifications
ModelRunnerOutput.cuda_graph_padded_extent: Optional[int], set at each_forward_rawexit to the padded per-rank extent of the runner that actually executed:decode_cuda_graph_runner.bsprefill_cuda_graph_runner.padded_num_tokens(new property exposing the padded static token count of the last replay)Noneoutput.cuda_graph_padded_extentinstead of the hard-codedgetattr(self.decode_cuda_graph_runner, "bs", None).This makes the invariant
can_run_graph == True ⇒ extent is not Nonehold by construction. I deliberately did not add aNone-guard that falls back to the eager prefix-sum layout: on the graph path that would silently read the wrong slice fordp_rank > 0(a correctness bug) instead of the loudTypeError. The change also removes an over-defensivegetattr, per theno-getattr-defensiveconvention.The decode path is byte-identical (it already passed
.bs; it now passes the same value via the new field).Accuracy Tests
New CPU regression test
test/registered/unit/layers/test_dp_attention_local_slice.py:TestModelRunnerOutputCudaGraphExtent— exercises the fix wiring:ModelRunnerOutputexposescuda_graph_padded_extent, it defaults toNoneon the eager path, and the graph-path extent feeds a valid rank-padded slice (notdp_rank * None).TestGetDpLocalSliceCpu— pins the underlying slicing invariant (graph →dp_rank * padded_extent; eager → prefix sum; the two must diverge fordp_rank > 0;Noneon the graph path raises).Verified in the
lmsysorg/sglang:latestimage:3 failed, 4 passed— the three wiring tests fail because the field does not exist.7 passed.On reproducing the live server crash — full transparency: I attempted an end-to-end server repro on 4× H100 NVL (driver 595 / CUDA 13.2), but could not trigger it on this build for a reason unrelated to this bug: prefill CUDA-graph capture fails during warmup for every MoE/DP model I tried (DeepSeek-V2-Lite:
CUBLAS_STATUS_EXECUTION_FAILED/ MLA shape errors; Qwen1.5-MoE and OLMoE fail elsewhere in capture), on both stock and patched trees, before any request. With prefill-graph capture unavailable, the runtimedp_rank * Nonepath is unreachable here. The evidence for this PR is therefore the root-cause analysis plus the deterministic regression test above, which fails on the unpatched tree and passes with the fix. Happy to add a live before/after trace if a maintainer can point me to a build/config where prefill-graph capture with DP attention succeeds.(Separately, the Qwen capture failure surfaced inside the routed-experts capturer itself — an off-by-
dp_sizeshape mismatch — which looks like a distinct defect; I'll file it as its own issue.)Speed Tests and Profiling
No performance impact — the change only threads an already-computed integer through
ModelRunnerOutput; no new work on any hot path.Checklist
ruff/black/isortclean).Disclosure: developed with AI assistance (Claude). The change is authored and owned by me, and I can defend it in review.
CI States
Latest PR Test (Base): ❌ Run #29306267159
Latest PR Test (Extra): ❌ Run #29306267055