[#14146][bugfix] Fix CuteDSL MoE ghost-token and global-index bugs#15155
[#14146][bugfix] Fix CuteDSL MoE ghost-token and global-index bugs#15155waynehacking8 wants to merge 3 commits into
Conversation
📝 WalkthroughWalkthroughThis PR fixes two distinct issues: CuteDSL MoE token-to-expert selection now correctly skips zero-allocation experts and uses global expert indices in prioritization calculations, and DeepSeek V3 model loading now conditionally imports the ChangesCuteDSL MoE Expert Selection Fixes
DeepSeek V3 Weight Loading Conditional
🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tensorrt_llm/_torch/models/modeling_deepseekv3.py`:
- Around line 899-902: The current conditional only copies
weights[0]["e_score_correction_bias"] when present, leaving
self.e_score_correction_bias uninitialized if the key is absent; add an else
branch in the same block that deterministically initializes
self.e_score_correction_bias (e.g., zero-fill or a chosen default) so
checkpoints that omit "e_score_correction_bias" produce deterministic behavior.
Ensure the fallback matches self.e_score_correction_bias's dtype and device (use
methods like zero_() or torch.zeros_like semantics) and reference the same
attribute name self.e_score_correction_bias and the source weights container
weights[0] when implementing the change.
In `@tests/unittest/_torch/thop/parallel/test_cute_dsl_moe.py`:
- Around line 75-93: The test test_prioritization_uses_global_index currently
only checks ID bounds; change it to assert actual prioritization output matches
the expected global expert set: use
GroupedGemmInputsHelper.generate_num_tokens_per_expert to get
num_tokens_per_expert, compute the expected set of global expert IDs as indices
where num_tokens_per_expert > 0 shifted by local_expert_offset (or otherwise
derived per the helper's mapping), then call generate_token_selected_experts and
assert that the non-negative entries in its result correspond exactly to that
expected global set (or contain the expected top-k per-expert selection), so
that failures show a prioritization/regression rather than just an out-of-range
ID. Ensure you reference test_prioritization_uses_global_index,
GroupedGemmInputsHelper, generate_num_tokens_per_expert and
generate_token_selected_experts when updating the assertions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: afcacc7d-6420-4184-9fb0-03eb17e585d1
📒 Files selected for processing (3)
tensorrt_llm/_torch/custom_ops/cute_dsl_custom_ops.pytensorrt_llm/_torch/models/modeling_deepseekv3.pytests/unittest/_torch/thop/parallel/test_cute_dsl_moe.py
Two bugs in generate_token_selected_experts (cute_dsl_custom_ops.py): 1. Ghost-token assignment: when num_tokens_per_expert[j] == 0, the inner loop still enters and assigns 1 token to that expert before breaking. With DeepSeek-V3 EP=16, top_k=8, 1 token: 7 ghost experts get assigned instead of 0. Fix: skip when count <= 0. 2. Prioritization uses local index j instead of global (j + local_expert_offset). On any rank with offset > 0, the at-risk-token prioritization condition evaluates against wrong expert indices, making it effectively dead code in multi-rank EP. Fix: compute global_j and use it in the prioritization condition and the expert assignment. Adds two unit tests covering both bugs. Signed-off-by: WEI CHENG CHIU <waynehacking8@gmail.com>
Assert assigned experts are a subset of the expected global expert set (non-zero-allocation locals shifted by offset), not just within the local range. This catches prioritization regressions directly. Signed-off-by: WEI CHENG CHIU <waynehacking8@gmail.com>
0b462b2 to
5b6af51
Compare
|
Status check: this PR has been mergeable and review-ready since 06-09 (coderabbit pass addressed at open time). Could a maintainer trigger CI ( |
…illed tokens The previous test only checked that assigned expert IDs land in the local shard range, which holds even with the local-index bug (the assignment line already used the global index). Replace it with a last-rank config where global-index prioritization rescues an at-risk token to top_k (3 filled vs 2), so reverting global_j -> j is caught. Addresses the CodeRabbit review comment. Signed-off-by: WEI CHENG CHIU <waynehacking8@gmail.com>
Summary
Fixes #14146 — two bugs in
generate_token_selected_experts(cute_dsl_custom_ops.py), the helper that synthesizestoken_selected_expertsfor autotuner profiling (inputs_pre_hook). They affect how representative the profiling distribution is — not inference routing or accuracy — but the generated profile should still match the per-expert allocation:num_tokens_per_expert[j] == 0, the inner loop still assigns 1 token to that expert before breaking. With DeepSeek-V3 EP=16, top_k=8, 1 token: 7 ghost experts get assigned instead of 0. Fix:if num_tokens_j <= 0: continue.top_k - (num_experts - j)uses the loop variablej(local) instead ofj + self.local_expert_offset(global). Whenevernum_local_experts < num_experts(EP > 1) the threshold is always negative, so the at-risk-token prioritization is dead code on every rank; the global index restores it near the tail of the global expert range. Fix: computeglobal_jand use it consistently.Reproduction
Test plan
test_ghost_token_zero_allocation— experts with 0 allocation receive no tokens (reproduces the[1,1,0,…]→ assigned[0,1]case above)test_prioritization_rescues_at_risk_tokens— deterministic last-rank config (num_experts=4, top_k=2, 2 local experts, offset=2, 4 tokens): global-index prioritization fills 3 tokens to top_k vs 2 under the local-index bug, so revertingglobal_j → jfails the test (the previous assertion only checked the expert-id range, which is offset-invariant and missed bug 2)test_grouped_gemm_inputs_helperandtest_moe_sortpassCloses #14146
PR Checklist
Please review the following before submitting your PR:
PR description clearly explains what and why. If using CodeRabbit's summary, please make sure it makes sense.
PR Follows TRT-LLM CODING GUIDELINES to the best of your knowledge.
Test cases are provided for new code paths (see test instructions)
If PR introduces API changes, an appropriate PR label is added - either
api-compatibleorapi-breaking. Forapi-breaking, includeBREAKINGin the PR title.Any new dependencies have been scanned for license and vulnerabilities
CODEOWNERS updated if ownership changes
Documentation updated as needed
Update tava architecture diagram if there is a significant design change in PR.
The reviewers assigned automatically/manually are appropriate for the PR.
Please check this after reviewing the above items as appropriate for this PR.
Summary by CodeRabbit
Bug Fixes
Tests