Skip to content

[Fix] deepep_v2: pass expert_start/num_experts in ep_scatter_from_psum - #37211

Open
whn09 wants to merge 1 commit into
sgl-project:mainfrom
whn09:fix/ep-scatter-from-psum-missing-args
Open

[Fix] deepep_v2: pass expert_start/num_experts in ep_scatter_from_psum#37211
whn09 wants to merge 1 commit into
sgl-project:mainfrom
whn09:fix/ep-scatter-from-psum-missing-args

Conversation

@whn09

@whn09 whn09 commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Motivation

ep_scatter_from_psum launches _fwd_kernel_ep_scatter_2 without the expert_start
and num_experts parameters the kernel has required since 5f216fc, so every rank
dies on the first forward pass
when --moe-a2a-backend deepep_v2 is used:

sglang/srt/layers/moe/moe_runner/deep_gemm.py:1677 in pre_permute_deepep_v2_to_deep_gemm
    ep_scatter_from_psum(
sglang/kernels/ops/moe/ep_moe_kernels.py:1297 in ep_scatter_from_psum
    _fwd_kernel_ep_scatter_2[(grid,)](
TypeError: dynamic_func() missing 2 required positional arguments: 'expert_start' and 'num_experts'

Still present on main @ 29578d5.

5f216fc ("qwen 3.8 rebase", #35758) generalised _fwd_kernel_ep_scatter_2 from local
to global expert ids:

-    expert_id = tl.load(recv_topk + ...)
-    if expert_id >= 0:
+    global_expert_id = tl.load(recv_topk + ...)
+    expert_id = global_expert_id - expert_start
+    valid = (expert_id >= 0) & (expert_id < num_experts)
+    tl.store(output_index_ptr, -1)
+    if valid:

It added the two parameters and updated the ep_scatter call site. The other call
site, ep_scatter_from_psum, had landed two days earlier in a3ae667 (#35634, DeepEPv2
ElasticBuffer MoE A2A backend) and was not updated. ep_scatter_from_psum is reached
only from pre_permute_deepep_v2_to_deep_gemm, i.e. only with
--moe-a2a-backend deepep_v2, so the omission is invisible to every other backend and
fatal for that one. Nothing in CI exercises the path.

Reproduces on any deepep_v2 server that reaches the non-masked permute path. Ours:
GLM-4.6-style MoE (GLM-5.1-FP8) on 4x p5en.48xlarge (H200, EFA), prefill TP=16 EP=16,
--moe-a2a-backend deepep_v2 --deepep-v2-mode hybrid. The crash lands after weight
load and ElasticBuffer init, so everything up to it looks healthy. The masked path
(expand_to_masked_slab) returns early and does not reach this launch.

Modifications

  1. ep_scatter_from_psum passes expert_start and num_experts to
    _fwd_kernel_ep_scatter_2, and takes expert_start: int = 0 — the same defaulted
    parameter ep_scatter already has.

    expert_start=0 restores the pre-5f216fc3 behaviour rather than assuming it. The
    old kernel indexed expert_start_loc + expert_id directly, and this function is
    called with an expert_start_loc of length num_local_experts: deep_gemm.py
    builds it as torch.empty_like(psum_num_recv_tokens_per_expert) and reads that same
    shape as num_local_experts a few lines earlier. Global ids would therefore have
    indexed out of bounds on every rank but rank 0 from the day [Feature] Add DeepEPv2 (ElasticBuffer) MoE A2A backend  #35634 merged — the
    DeepEPv2 dispatch delivers already-local ids with -1 padding, so local_id - 0
    reproduces the old behaviour exactly and -1 still fails the >= 0 test. Exposing
    the parameter rather than hard-coding 0 gives a future caller holding global ids
    somewhere to say so; happy to pass a literal 0 instead if you would rather not
    widen the signature.

    num_experts is psum_num_recv_tokens_per_expert.shape[0], which the function
    already computes for the _fwd_kernel_ep_scatter_psum_init grid. The psum is an
    inclusive prefix sum of length num_local_experts
    (_fwd_kernel_ep_scatter_psum_init reads psum[e-1]..psum[e] as expert e's
    span), so the new expert_id < num_experts bound is exact — no off-by-one.

    This also satisfies the requirement that output_index be -1-initialised, which
    the same commit introduced when it switched post_reorder_deepgemm_triton_kernel
    from gating on expert_id >= 0 to dst_idx >= 0 (output_index = torch.empty_like(topk_ids) is otherwise uninitialised).

  2. New test/registered/kernels/ops/moe/test_ep_scatter.py. Neither ep_scatter nor
    ep_scatter_from_psum had any coverage, which is exactly why a signature change
    could update one call site and leave the other broken.

    Destination slots are chosen by atomic_add, so the order within one expert is not
    deterministic. The test asserts the invariants that pin the result down regardless:
    the -1 sentinel lands exactly on the padding lanes; every valid lane lands inside
    its own expert's slab; the slabs form a bijection onto [0, total); m_indices
    agrees with the routing and its BLOCK_E tail stays -1; expert_start_loc ends
    at the prefix sum; and the rows and FP8 scales themselves arrive. Parametrised over
    num_tokens in {1, 8, 256, 4096}, padding fraction, and bf16 / float8_e4m3fn,
    plus cases for a nonzero expert_start and for expert ids this rank does not own.

    It leaves expert_start at its default in the local-id case, so it exercises the
    call shape the DeepEPv2 permute path actually uses — which is what makes it a
    regression test for this bug rather than only for the signature.

Accuracy Tests

The new unit test passes (20/20) and fails on an unpatched tree with exactly the
TypeError above, so the before/after is observed rather than asserted (H200, one GPU):

A: main + this PR      ->  20 passed in 8.57s
B: main, unpatched     ->  TypeError: dynamic_func() missing 2 required positional
                           arguments: 'expert_start' and 'num_experts'

Also checked structurally: both launch sites now supply all 22 parameters preceding the
first tl.constexpr, and the two added arguments land on the kernel's expert_start /
num_experts formals rather than merely making the count add up.

There is no end-to-end accuracy comparison to offer, because the path cannot execute a
single forward pass without this change — there is no pre-fix baseline to diff against.
Correctness rests on the argument above that this restores pre-5f216fc3 semantics
exactly, plus the invariant checks in the test. Glad to add a gsm8k run on
--moe-a2a-backend deepep_v2 if you would like one attached.

Speed Tests and Profiling

None: the change passes two additional scalar arguments to an existing kernel launch
and adds no work. Not applicable.

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.
  4. After green CI and required approvals, ask Merge Oncalls or people with Write permission to merge the PR.

cc @Qiaolin-Yu (5f216fc, #35758) @MengYu10151 (a3ae667, #35634) — and whoever on the
/python/sglang/kernels CODEOWNERS list is on rotation. I cannot trigger CI myself.


CI States

Latest PR Test (Base): ❌ Run #33366659160
Latest PR Test (Extra): ❌ Run #33366659073
Latest PR Test (AMD ROCm 7.2): ❌ Run #33366659163

`ep_scatter_from_psum` launches `_fwd_kernel_ep_scatter_2` without the
`expert_start` and `num_experts` parameters the kernel has required since
5f216fc ("qwen 3.8 rebase", sgl-project#35758), so every rank dies on the first forward
pass:

  sglang/srt/layers/moe/moe_runner/deep_gemm.py:1677 in
      pre_permute_deepep_v2_to_deep_gemm
  sglang/kernels/ops/moe/ep_moe_kernels.py:1297 in ep_scatter_from_psum
      _fwd_kernel_ep_scatter_2[(grid,)](...)
  TypeError: dynamic_func() missing 2 required positional arguments:
             'expert_start' and 'num_experts'

5f216fc generalised the kernel from local to global expert ids and updated the
`ep_scatter` call site, but `ep_scatter_from_psum` was added two days earlier by
a3ae667 (sgl-project#35634, DeepEPv2 ElasticBuffer A2A backend) and is reached only from
`pre_permute_deepep_v2_to_deep_gemm`, i.e. only with
`--moe-a2a-backend deepep_v2`. Nothing else exercises it, so the omission is
invisible to every other backend and fatal for that one.

`expert_start=0` restores the pre-5f216fc3 behaviour rather than assuming it:
the old kernel indexed `expert_start_loc + expert_id` directly, and
`ep_scatter_from_psum` is called with an `expert_start_loc` of length
`num_local_experts` (deep_gemm.py builds it as
`torch.empty_like(psum_num_recv_tokens_per_expert)` and reads that same shape as
`num_local_experts`). Global ids would therefore have been out of bounds on
every rank but rank 0 since sgl-project#35634 merged — the DeepEPv2 dispatch delivers
already-local ids with -1 padding. The parameter is exposed with the same
`expert_start: int = 0` default `ep_scatter` uses, so a future caller with
global ids has somewhere to say so.

`num_experts` is `psum_num_recv_tokens_per_expert.shape[0]`, which the function
already computes for the `_fwd_kernel_ep_scatter_psum_init` grid.

Add test/registered/kernels/ops/moe/test_ep_scatter.py. Neither `ep_scatter` nor
`ep_scatter_from_psum` had any coverage, which is why a signature change could
update one call site and leave the other broken. Destination slots are picked by
atomic_add so their order within an expert is not deterministic; the test asserts
the invariants that pin the result down regardless — the -1 sentinel lands
exactly on padding lanes, every lane lands inside its own expert's slab, the
slabs form a bijection onto [0, total), m_indices agrees with the routing,
expert_start_loc ends at the prefix sum, and the rows and scales themselves
arrive. It also covers a nonzero `expert_start` and non-local expert ids, and it
leaves `expert_start` at its default in the local-id case so it exercises the
call shape the DeepEPv2 path uses. On an unpatched tree it fails with exactly
the TypeError above.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant