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 (2)
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughThe change corrects NVFP4 FC1 intermediate-dimension alignment for gated and non-gated activations. Tests now cover intermediate sizes 96 and 192, including scale padding, execution, finite outputs, and numerical agreement. ChangesNVFP4 alignment validation
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to The alignment update is covered for padded gated and non-gated NVFP4 shapes, with no actionable merge-blocking risk identified. 🚥 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 |
The CUTLASS NVFP4 fused MoE binding rejects
intermediate_sizevalues that are multiples of 64 but not of 128, for example 1856 (Nemotron-3.5-Lightning-30B-A3B), withfc1 weight block size must be (num_experts_on_rank, inter_size, hidden_size // 4 // block_scale_vector_size).As far as I can tell, the fc1 row term of that check aligns
inter_sizewithMinKDimAlignmentNVFP4(64), while both producers of that tensor pad the rows toMinNDimAlignmentNVFP4(128):getOffsetWeightSFin the kernel andprepare_cutlass_nvfp4_weights/CutlassNvfp4Runner.pack_inputson the Python side (round_up(gemm1_rows, 128)). The fc2 check in the same function already uses the row alignment forhidden_size. When I is a multiple of 128 the two constants agree, which is probably why the common shapes never hit this.This PR aligns the full fc1 row count (
inter_size, orinter_size * 2when gated) withMinNDimAlignmentNVFP4in both branches, and adds a test at intermediate sizes 96 and 192 for ReLU2 and SwiGLU so the padded-row shapes are exercised. The gated form aligns the doubled row count rather than doubling the aligned half, matching the kernel offset and the Python padding (for I=1856 gated the tensors carry 3712 rows, not 3840).I might be missing a reason the stricter constant was intended. If NVFP4 is meant to require 128-multiples, the cleaner fix would be a Python-side rejection in
CutlassNvfp4Runner, as the MXFP paths already do, and I am happy to switch to that instead.🔍 Related Issues
🚀 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
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.).On an RTX PRO 6000 Blackwell (SM120),
pytest tests/moe/test_unified_moe_cutlass.py -k "nvfp4 or mxfp8"gives 64 passed and 11 skipped (other architectures), including the new intermediate_size 96/192 x ReLU2/SwiGLU cases. The Nemotron shape (H=2688, I=1856, E=128, top_k=6, ReLU2, NVFP4 W4A4) also passes theflashinfer_benchmark.py --refcheckcomparison at 1, 512 and 4096 tokens, with CUTLASS latencies of 59 / 566 / 957 us, in line with the numbers reported in #4646.Summary by CodeRabbit
Bug Fixes
Tests