fix(moe): write fused-shared routing replay as routed-only [T, K] - #4894
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (5)
💤 Files with no reviewable changes (1)
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughRouting replay now uses a routed-only ChangesRouting replay support
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Bug fix · Severity of issue fixed: Medium Merge Risk: ⚪ Minimal · up to Replay buffers now retain the routed-only layout while accepting valid oversized CUDA-graph allocations. No concrete merge-blocking issue remains. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@flashinfer-bot run |
|
/bot run tests/moe |
|
[FAILED] Pipeline #66024680 — 14/17 executed test jobs passed Compared with nightly #65814627 (different CI configuration). Unit Tests
✅ Pass · 🟡 Old failure · ❌ New failure · ⏱ Test timeout · Multi-GPU and Multi-Node Tests — 6/6 passed
Failure detailsCould not compare
Timeouts, infrastructure, or incomplete jobs
|
Two review points from @feih-nv, who filed flashinfer-ai#5009. `num_tokens` was `Optional[int] = None` guarded by `is not None`, so a future entry point that forgot to pass it would silently skip the bound. Make it a required parameter: forgetting it is now a TypeError at the call site rather than a missing check at runtime. All seven callers already passed it; the two that also pass `num_fused_shared_experts` now name both keywords, since the parameter order changed. Add `test_routing_replay_out_rejects_undersized_dim0`, lifted from @feih-nv's `b81f0946` on flashinfer-ai#4894 with permission — FP8 and FP4, CPU tensors, no GPU needed. It pins the rejection this PR adds, which had no test of its own. flashinfer-ai#4894 drops that commit once this lands so the two do not collide on the validator and its call sites. Co-Authored-By: feih <feih@nvidia.com> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
8a42766 to
3b44f9d
Compare
|
/bot run tests/moe |
|
@flashinfer-bot run |
|
[FAILED] Pipeline #67171205 — 10/17 executed test jobs passed Compared with nightly #66937827. Unit Tests
✅ Pass · 🟡 Old failure · ❌ New failure · ⏱ Test timeout · Multi-GPU and Multi-Node Tests — 6/6 passed
Failure detailsNew relative to nightly (attribution uncertain)
Pre-existing failures
Could not compare
Timeouts, infrastructure, or incomplete jobs
|
|
/bot run tests/moe |
|
@flashinfer-bot run |
|
[FAILED] Pipeline #67329403 — 10/19 executed test jobs passed Compared with nightly #67144516 (different CI configuration). Unit Tests
✅ Pass · 🟡 Old failure · ❌ New failure · ⏱ Test timeout · Multi-GPU and Multi-Node Tests — 6/6 passed
Failure detailsNew relative to nightly (attribution uncertain)
Pre-existing failures
Timeouts, infrastructure, or incomplete jobs
|
DeepSeek routing packed ids at stride top_k + S, so replay used the same index and scrambled multi-token [T, K] buffers. Write replay at stride top_k, drop the S>0 host/C++ rejects, and cover FP8/FP4 plus a CUDA-graph capture of the oversized buffer. AI-assisted.
Deleting it read as done. The fused-shared replay coverage landed in the fused-MoE tests instead; the standalone routing harness still has no S>0 case, because it would first have to build fused-shared weight rows.
Drop the int64_t casts (packed idxTopK is 32-bit) and put dim0 before dim1 in the C++ replay validator comment. AI-assisted.
8eb2823 to
a65baec
Compare
|
@flashinfer-bot run |
|
/bot run tests/moe |
|
[SUCCESS] Pipeline #67710173: 18/19 executed test jobs passed |
📌 Description
Fixes #5009 part 1. DeepSeek
routingMainKernelwroterouting_replay_outwith the packed indextoken * (K+S) + k. Replay is[T, K](routed ids only), so token 1 starts atK+Sinstead ofK.S == 0hid it; host/C++ rejectedS > 0(#4239), so the combo was unavailable.This PR indexes replay with
token * K + kand drops theS > 0rejects. Packed ids/weights stay[T, K+S].🔍 Related Issues
🚀 Pull Request Checklist
✅ Pre-commit Checks
pre-commitby runningpip install pre-commit(or used your preferred method).pre-commit install.pre-commit run --all-filesand fixed any reported issues.🧪 Tests
unittest, etc.).CPU reject
dim1 == K+SS > 0GPU (SM100) accept
T ∈ {8,32},S ∈ {1,2}[32, K]Reviewer Notes
With
S > 0,routing_replay_outis still[T, K]int16: the sameKrouted expert ids asS = 0on the same logits, ids in[0, E). TheSfused-shared slots live only in the internal packed ids/weights ([T, K+S], idsE … E+S-1). They are not appended to replay. A[T, K+S]replay buffer is rejected (dim1must equaltop_k).Summary by CodeRabbit
New Features
top_k; shared-expert slots are not recorded.Documentation
Tests