Skip to content

[Fix] Skip redundant routed_scaling_factor in TBO DeepseekV2MoE.op_output - #35152

Closed
zkyue wants to merge 1 commit into
sgl-project:mainfrom
zkyue:fix/tbo-op-output-routed-scaling-double-apply
Closed

zkyue wants to merge 1 commit into
sgl-project:mainfrom
zkyue:fix/tbo-op-output-routed-scaling-double-apply

Conversation

@zkyue

@zkyue zkyue commented Aug 17, 2026 •

Copy link
Copy Markdown
Contributor

Bug

TBO (two-batch-overlap) with DeepEP MoE silently double-applies routed_scaling_factor. DeepseekV2MoE.op_output (the TBO op-pipeline epilogue) multiplied the routed output by self.routed_scaling_factor in every non-aiter case, while the non-TBO forward_deepep guards that multiply with experts.should_fuse_routed_scaling_factor_in_topk or _use_aiter (guard added in #8690 to forward_deepep only).

Trigger

--enable-two-batch-overlap on a quant method that folds the factor into topk weights (e.g. ModelOpt NVFP4: ModelOptNvFp4FusedMoEMethod sets the flag) with the deepep / flashinfer_cutedsl runner. The factor is already inside topk_weights, so the epilogue returned routed output scaled by rsf^2 (2.5x for DeepSeek-V3) with no error or warning. Decode / target-verify are affected (prefill TBO is blocked for deepep-LL).

Evidence

A harness executing the verbatim op_output source on a stub MoE state shows the TBO output == 2.5x the intended value in both the shared-expert and no-shared branches; non-TBO forward_deepep applies rsf exactly once for the same layer.

Fix

Mirror the forward_deepep guard inside op_output (two conditions) so the epilogue skips the multiply when the factor was already applied upstream. The included CPU-only unit test covers all 8 combos of (fold-flag, aiter, shared-experts) and fails on the pre-fix code in exactly the fold-flag combos.

Notes

  • Known interaction: WaterfillBalancer / remap_topk_for_per_rank_shared_slots hard-code the fused shared-expert weight to 1/rsf (which was only correct while the post-MoE multiply still ran). The companion fix ([Fix] shared expert under-weighted when rsf is pre-folded in topk weights #35163, stacked on this PR) sets that weight to 1.0 for pre-folded families; even without it this change is a net win (previously ALL routed experts were 2.5x off; the shared expert alone would be 0.4x until the companion lands).
  • The env-gated benchmark routing (SGLANG_SIMULATE_UNIFORM_EXPERTS, topk.py) overwrites topk weights with 1/k, so under that simulate mode rsf is now omitted - benchmark-only path, noted for completeness.
  • glm4_moe.py / glm4_moe_lite.py carry the same unguarded pattern but their decoder layers are not registered in OperationsStrategy.init_new_tbo, so they are TBO-unreachable today; left untouched to keep the diff minimal.

Test

test/registered/unit/batch_overlap/test_tbo_moe_op_output_routed_scaling.py: AST-extracts the verbatim op_output and asserts rsf is applied exactly once across all 8 combos; runs on CPU in seconds, no GPU / weights / server needed.


CI States

Latest PR Test (Base): ❌ Run #32026619896
Latest PR Test (Extra): ❌ Run #32026619628

…tput

op_output (the TBO DeepEP MoE epilogue) applied routed_scaling_factor
unconditionally, while the non-TBO forward_deepep guards the multiply
with `experts.should_fuse_routed_scaling_factor_in_topk or _use_aiter`.
When the guard flag is true the factor is already folded into the topk
weights (e.g. ModelOpt NVFP4 quant) or into aiter_biased_grouped_topk,
so TBO silently returned routed output scaled by
routed_scaling_factor**2 (2.5**2 for DeepSeek-V3).

Mirror the forward_deepep guard in op_output so the factor is applied
exactly once on both paths.
@github-actions

Copy link
Copy Markdown
Contributor

Thanks @zkyue. Closing this because you have 18 PRs open and none updated in 7 days, over our soft cap of 5 for idle PRs.

Reopen the two or three you most want reviewed - a short active set moves faster than a long idle one.

Some directories moved recently, so an older branch may need retargeting:
sgl-kernel/ -> python/sglang/kernels/aot/, python/sglang/jit_kernel/
-> python/sglang/kernels/jit/, docs/ -> docs/docs/ (.mdx),
bench_serving.py -> benchmark/serving.py, test/srt/ -> test/registered/.

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.

1 participant