test(moe): cover GLM NVFP4 dispatch boundary#71
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe test helper now supports configurable expert counts, and a new NVFP4 test verifies that tensor-parallel MoE execution changes from micro to dynamic between ChangesMoE execution threshold coverage
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@tests/moe/test_execution_model.py`:
- Around line 254-267: Update the test around plan_tp_moe_execution to control
SPARKINFER_DYNAMIC_DETERMINISTIC_OUTPUT explicitly: clear or unset it before
asserting the default M=8 plan has deterministic_output disabled, then set it to
"1" and create a second M=8 plan to assert deterministic_output is enabled. Keep
the existing implementation assertions and use the test’s environment-management
utilities to restore ambient state.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 0742f242-defc-490b-b59b-c93805f12610
📒 Files selected for processing (1)
tests/moe/test_execution_model.py
|
The review finding is addressed in Validation: |
Summary
Why
The investigation behind closed PR #60 initially treated M=8 variation as evidence about fused-micro auxiliary-stream overlap. Real checkpoint validation showed that M=8 does not execute fused micro at all: top-k 8 produces 64 routed rows, so the production planner selects the dynamic backend. Its default BF16 atomic scatter is intentionally not bitwise deterministic. M=7 remains on micro and is bitwise repeatable.
This test makes that boundary explicit so future overlap investigations do not compare two different kernel families as if they were one.
Validation
pytest -q tests/moe/test_execution_model.pyin the CUDA test image: 16 passedSPARKINFER_DYNAMIC_DETERMINISTIC_OUTPUT=1made all eight M=8 repeats bitwise identicalSummary by CodeRabbit
SPARKINFER_DYNAMIC_DETERMINISTIC_OUTPUT=1is enabled.