fix(moe): prepare MXFP8 MXFP4 profiler inputs - #4308
Conversation
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe profiler now preserves MXFP8 activation-scaling state, sizes gated FC1 scale storage correctly, and selects MXFP8×MXFP4 parameters with initialized scales. Tests cover both default and autotuned fused MoE execution. ChangesMXFP8/MXFP4 fused MoE profiling
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
|
@waynehacking8 @nv-yunzheq @jimmyzho, could you review this as the profiler-repair alternative to #4066? This initializes the MXFP8×MXFP4 profiler inputs and retains tactic profiling; #4066 selects fallback tactics for this mode on SM120/SM121. |
|
/bot run tests/moe |
…fc1 SF sizing (#4010) ## 📌 Description Autotuning a native NVFP4 (FP4 activations x FP4 weights) CUTLASS MoE crashed with `Assertion failed: quant_1 && quant_2 && quant_3 && quant_4 && quant_5 && quant_6` (#4003): after #3738, the gemm profiler only allocated its scratch quant-scale buffers for the FP8-activation FP4 flavor, so native NVFP4 got none. #4080 has since fixed that crash on main. This PR is rebased on top of it and adds what's still missing: - Replace the ad-hoc `is_nvfp4_quant` check with an `isNativeWfp4Afp4Family()` helper used by both the allocation site (`getProfilerWorkspaces`) and the consumption site (`prepareQuantParams`), so the two can't drift apart again. This also drops the `mSM >= 100` guard that the allocation side had but the consumption side didn't. - Size the fc1 weight scale-factor buffer (`quant_2`) with `fc1_out_size` instead of `inter_size`. Gated activations make fc1 output 2x `inter_size`, so the old size under-allocated. #4308 has since landed this sizing for the wfp4afp8 flavor; this PR applies it to native NVFP4 and merges the two sizing branches so they cannot diverge. - Add autotune coverage to `test_moe_nvfp4`, the regression test that would have caught #4003. Credit to @eugr for the proposed patch this builds on. ## 🔍 Related Issues Fixes #4003 (crash itself already fixed on main by #4080). Regressed by #3738. ## 🚀 Pull Request Checklist Thank you for contributing to FlashInfer! Before we review your pull request, please make sure the following items are complete. ### ✅ Pre-commit Checks - [x] I have installed `pre-commit` by running `pip install pre-commit` (or used your preferred method). - [x] I have installed the hooks with `pre-commit install`. - [x] I have run the hooks manually with `pre-commit run --all-files` and fixed any reported issues. ## 🧪 Tests - [x] Tests have been added or updated as needed. - [x] All tests are passing (`unittest`, etc.). `test_moe_nvfp4` is now parametrized with `use_autotune`, same approach as #3558. Pre-rebase on a GB10 (SM121): every autotune case died with the assertion without the fix, and all 48 cases passed with it. Post-rebase, re-ran gated and non-gated autotune cases plus a non-autotune case on an RTX 5080 (SM120), all passing. After rebasing onto current main (post-#4308), all 48 cases pass on an RTX 5080 (SM120). ## Reviewer Notes The predicate deliberately has no SM version check, unlike its wfp4afp8 sibling: it must exactly match the consuming branch in `prepareQuantParams`, which checks dtypes only. FP4 x FP4 doesn't exist below SM100 anyway. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved Native NVFP4 fused Mixture-of-Experts (MoE) execution by correcting workspace and scaling-factor handling. * Fixed NVFP4 behavior when autotuning is enabled, preventing related execution failures. * **Tests** * Expanded NVFP4 coverage to validate both autotuned and non-autotuned execution paths. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
📌 Description
GemmProfilerBackend::init()receivesuse_mxfp8_act_scalingbut does not retain it, soprepareQuantParams()always constructsQuantParams::FP8MXFP4on the FP8-activation/MXFP4-weight path. That leaves themxfp8_mxfp4weight block-scale pointers null, so the MXFPX scale-factor descriptor setup is skipped during profiling.VictoriaLogs captured the affected DeepSeek-V4-Flash-0731 TP2 startup reaching fused-MoE profiling and then exiting after this error surfaced:
This change:
use_mxfp8_act_scalingin the profiler;QuantParams::MXFP8MXFP4for MXFP8-activation/MXFP4-weight profiling; andWhen
use_mxfp8_act_scalingis false, the existingQuantParams::FP8MXFP4branch remains in use. The shared fc1 scale-factor workspace sizing is corrected for the gated output in both branches. PR #4066 skips profiling and selects fallback tactics for MXFP8×MXFP4 on SM120/SM121; this change repairs the profiler inputs and retains tactic profiling. Both changes update the same regression test.If this change is accepted, PR #4066's SM120/SM121 fallback-tactic override should be removed, or #4066 closed. That override keeps SM120/SM121 on fallback tactics for this mode, which makes this profiler fix inert there.
🔍 Related Issues
Fixes #4049.
🚀 Pull Request Checklist
✅ Pre-commit Checks
uvx pre-commit.pre-commit install. Not used; hooks were run directly withuvx.csrc/fused_moe/cutlass_backend/cutlass_fused_moe_kernels.cuh,csrc/nv_internal/tensorrt_llm/kernels/cutlass_kernels/include/moe_kernels.h, andtests/moe/test_trtllm_cutlass_fused_moe.py.🧪 Tests
uv run --no-project python -m pytest -q tests/moe/test_trtllm_cutlass_fused_moe.py -k mxfp8_mxfp4on SM120: 12 passed, 6 skipped, 132 deselected.DeepSeek-V4-Flash-0731TP2 startup completed all 10 gemm1 and 10 gemm2 profiler candidates on both ranks, completed FlashInfer autotuning, captured the target and draft CUDA graphs, and reached ready state with zero restarts.Test limitations: the autotuned numerical test exercises this path and checks numerical parity, but it does not directly inspect the fabricated workspace contents. The non-MXFP8 activation branch touched by the shared fc1 scale-factor sizing has no focused autotune regression case in this validation.
Reviewer Notes
The table below compares heuristic fallback tactics with profiler-selected tactics. It is not a direct patched-versus-unpatched throughput comparison; the patch is what allows the profiled-tactic side to start successfully. Both sides used the same SGLang revision, model, TP2 configuration, FP8 KV cache, DSpark width 5, hardware, and workload. The control skipped
trtllm::fused_moe::gemm1andgemm2; the candidate profiled them.Methodology: each decode cell is one unreplicated 30-second run at C1, C2, C4, C8, C16, and C32. The coding row is the median of five sequential requests with a 2,000-token cap. The prefill rows are the median client-observed throughput from standalone cold-prefill runs at exact 8K, 64K, and 128K prompts with 7, 2, and 1 samples, respectively. All rows are single comparison runs with no variance estimate, and no effect size is claimed from any row.
The pinned 1,319-row GSM8K gate was not counted as passing because each run had one response finish at the 1,024-token cap. Two profiled-tactic runs scored 1,241 and 1,239 correct; the fallback-tactic control scored 1,240 correct. No quality difference is claimed from these runs.
AI was used to assist with implementation.
Summary by CodeRabbit
Bug Fixes
Tests