[Bugfix] DeepEP-V2: expert_tokens_meta must be None on the decode/cudagraph path (empty recv_expert_num_tokens) - #52632
Conversation
Signed-off-by: Anton Alexander <dmvevents@gmail.com>
Signed-off-by: Anton Alexander <dmvevents@gmail.com>
|
Cross-referencing for the maintainers: @tlrmchlsmth's draft #50511 contains a functionally equivalent guard at this same call site (plus padding-row zeroing before whole-tensor dynamic quantization, which this PR does not touch). This PR is offered as the minimal, GPU-validated subset: just the Happy to defer to #50511 if the superset lands first — in that case this can be closed in its favor. Filing separately only because #50511 is marked draft/not-yet-GPU-validated and this subset is measured and independently landable. |
|
This pull request has merge conflicts that must be resolved before it can be |
…graph path On the graph-enabled path do_cpu_sync=False leaves recv_expert_num_tokens empty, so make_from_list built a present-but-EMPTY ExpertTokensMetadata, violating the decode-mode contract documented in this file (must be None) and crashing DeepEP combine during startup profile_run: reproduced as a Triton illegal-memory-access at deepep_v2.py (B200, EP16, 12+/16 ranks, 2/2 repro, Xid 43, ECC clean) and independently by AWS as CUDA_ERROR_LAUNCH_FAILED(719) from DeepEP's JIT handle (B300, EP16, DeepSeek-V2-Lite) - one root cause surfacing at different sync points. --enforce-eager avoids it; this guard makes default compilation safe. Signed-off-by: Anton Alexander <dmvevents@gmail.com>
0228d99 to
5b294d9
Compare
|
I get the same issue, the fix is working |
|
/ci run |
|
✅ @dmvevents, CI is now available for this PR.
|
|
✅ Triggered Buildkite CI #84584 for commit |
|
Hi @dmvevents, the pre-commit checks have failed. Please run: uv pip install pre-commit>=4.5.1
pre-commit install
pre-commit run --all-filesThen, commit the changes and push to your branch. For future commits, |
…path Fixes mypy union-attr error: expert_tokens_meta can be None on the decode/cudagraph path, but the recv_topk_idx is None (prefill) branch always has recv_expert_num_tokens populated. Co-authored-by: Kimi Code Signed-off-by: Roger Wang <hey@rogerw.io>
|
/ci run |
|
✅ Triggered Buildkite CI #84675 for commit |
…agraph path (empty recv_expert_num_tokens) (vllm-project#52632) Signed-off-by: Anton Alexander <dmvevents@gmail.com> Signed-off-by: Roger Wang <hey@rogerw.io> Co-authored-by: Roger Wang <hey@rogerw.io> Signed-off-by: Zhu, Zufang <zufang.zhu@intel.com>
The kill switch only nulled _align_m_fn, reverting the dispatch-side align. But _receiver unconditionally ran the cudagraph block that carries psum_recv_per_rank and creates a non-None expert_tokens_meta, which drives the feature's always-on apply-side rewrite (valid_tokens bounding of SITU + moe_fused_mul_sum). So DISABLE=1 still ran the new apply path and was NOT the pre-feature reference -- both DISABLE=0 and DISABLE=1 measured ~0.65 gsm8k. Gate that carry under the same switch (self._fused_moe_align_disabled) so DISABLE=1 restores the pre-feature contract: expert_tokens_meta is None on the cudagraph decode path (PR vllm-project#52632), valid_tokens is None, and SITU + moe_fused_mul_sum revert to unbounded reduction. The globalize kernel already sanitizes the padding tail to -1, so this is safe. This isolates whether the regression lives in the feature's apply-side rewrite. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Elvir Crncevic <elvircrn@gmail.com>
The VLLM_DEEPEP_DISABLE_FUSED_MOE_ALIGN kill switch was gating the per-rank psum carrier (`psum_recv_per_rank`) on `_fused_moe_align_disabled`, so DISABLE=1 also suppressed it. That carrier is the pre-fusion known-good mechanism (PR vllm-project#52632): under cudagraph decode it always populated expert_tokens_meta so num_valid bounds SITU + moe_fused_mul_sum. Suppressing it forced the untested has_num_valid=False path, which left stale cudagraph output rows and cost ~0.05 gsm8k (0.94 -> 0.89) even after the earlier padding-row fix. Restore the baseline `if self.use_cudagraph:` gate so the carrier always runs; the fused MoE-align metadata attach stays correctly gated by `fused_moe_align is not None` (None under DISABLE=1), keeping the fusion itself off. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Elvir Crncevic <elvircrn@gmail.com>
…agraph path (empty recv_expert_num_tokens) (vllm-project#52632) Signed-off-by: Anton Alexander <dmvevents@gmail.com> Signed-off-by: Roger Wang <hey@rogerw.io> Co-authored-by: Roger Wang <hey@rogerw.io>
… removed OFI_NCCL_GIN_GDAKI; retire "guard pending" wording
- serve.sh runs `set -u`; its launch banner still expanded `${OFI_NCCL_GIN_GDAKI}`,
which the previous commit stopped exporting — that would have aborted the
launch with "unbound variable". The banner now prints `gdaki=auto(--enable-gdaki build)`,
which is what actually selects GDAKI at the pinned plugin.
- README + serve.sh still described the empty-ExpertTokensMetadata guard
(vllm-project/vllm#52632) as "filed upstream / pending" and referred to a
"fix stack" that was deleted on 2026-08-17. #52632 merged 2026-08-20; it
post-dates the pinned e2f993dc4. Both now say: not shipped at this pin, a
VLLM_SHA bump past the merge commit + re-measure enables it, no patch step.
Verification: bash -n on every script (no `${OFI_NCCL_GIN_GDAKI}` / `${EP_EFA_*}` /
`${DEEP_EP_BACKEND}` expansion remains anywhere in the folder); markdownlint-cli2
0 issues. Not rebuilt or re-run on cluster.
Signed-off-by: Anton Alexander <dmvevents@gmail.com>
Purpose
Fix a deterministic startup crash in graph-enabled (default-compilation) serving with
--all2all-backend deepep_v2.In
DeepEPV2PrepareAndFinalize._receiver,expert_tokens_metawas built unconditionally viamk.ExpertTokensMetadata.make_from_list(recv_expert_num_tokens, ...). On the decode/cudagraph path,do_cpu_sync = not self.use_cudagraph(this file) intentionally skips the CPU sync, leavingrecv_expert_num_tokensempty — so the call builds a present-but-emptyExpertTokensMetadata. That violates the decode-mode contract documented at the top of this file (expert_tokens_metamust beNonewhen counts are absent), and downstream it crashes DeepEP'scombineduring startupprofile_run.Measured, two independent reproductions of the same call path (
profile_run → finalize_async → DeepEP elastic.py combine):deepep_v2.pycombine, 12+/16 ranks, deterministic ~48 s into startup, 2/2 repro (Xid 43, ECC clean).CUDA_ERROR_LAUNCH_FAILED (719)raised from DeepEP's JIT handle at the same phase — one root cause surfacing at different synchronization points.--enforce-eageravoids the crash (the sync happens); this guard makes default compilation safe.Changes
One guard: return
expert_tokens_meta = Nonewhenrecv_expert_num_tokensis empty (the decode/cudagraph path), matching the documented contract; the prefill path (do_cpu_sync=True) is unchanged.Test Plan / Validation
e2f993dc4-era pins: graph-enabledvllm serve(no--enforce-eager) on EP16/DP16 across 2 nodes over EFA completes CUDA-graph capture, reachesApplication startup complete, and serves: 153/153 HTTP 200 across a 1/8/16/32/64 concurrency sweep (previously: deterministic crash inprofile_runbefore startup completed).