[https://nvbugs/6434512][fix] Select Marlin for Qwen3.5 MoE on Hopper - #17293
Conversation
WalkthroughQwen3.5 MoE models now share hardware-aware backend defaults. Supported Hopper configurations select MARLIN for MoE and NVFP4 GEMM. Tests cover Hopper, mixed-precision, and Blackwell configurations. ChangesQwen3.5 NVFP4 backend defaults
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 `@tensorrt_llm/_torch/models/modeling_qwen3_5.py`:
- Line 58: Annotate every listed function: add parameter and return annotations
to _get_qwen35_moe_model_defaults, Qwen3_5MoeForCausalLM.get_model_defaults, and
Qwen3_5MoeVLModel.get_model_defaults in
tensorrt_llm/_torch/models/modeling_qwen3_5.py; annotate monkeypatch and add ->
None to the three affected test functions in
tests/unittest/_torch/modeling/test_modeling_qwen3_5_vl_moe.py at lines 161,
184, and 201.
🪄 Autofix
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: 88037976-5a87-4cc1-85d9-fe18ea04a3b1
📒 Files selected for processing (2)
tensorrt_llm/_torch/models/modeling_qwen3_5.pytests/unittest/_torch/modeling/test_modeling_qwen3_5_vl_moe.py
|
/bot run --disable-fail-fast |
|
PR_Github #63983 [ run ] triggered by Bot. Commit: |
|
PR_Github #63983 [ run ] completed with state
|
|
/bot run --disable-fail-fast |
|
PR_Github #64036 [ run ] triggered by Bot. Commit: |
|
PR_Github #64036 [ run ] completed with state
|
|
/bot run --disable-fail-fast |
|
PR_Github #64064 [ run ] triggered by Bot. Commit: |
|
PR_Github #64064 [ run ] completed with state |
brnguyen2
left a comment
There was a problem hiding this comment.
Approving — the comments below are optional touch-ups, not blockers.
Change looks right for the reported H20 failure and the marlin-only GEMM list is fine in practice — the other NVFP4 backends are all SM100+, so nothing is lost on Ada/Hopper.
Two things to confirm before merge (details inline): whether the trigger covers the ModelOpt MIXED_PRECISION NVFP4 exports this file already normalizes, and whether the dense Qwen3.5 classes need the same nvfp4_gemm_config treatment.
Description nit: the auto-generated summary claims three new test functions (test_qwen35_moe_model_defaults_select_marlin_on_hopper, ..._keep_non_nvfp4_backends, ..._keep_blackwell_backends) — the diff actually adds one parametrized test with three params. Worth fixing so a future reader grepping for those names isn't confused.
One more question: MarlinFusedMoE doesn't support the MoE load balancer. Since this now flips the backend by default, is there a configuration (EPLB / wide-EP) where the new default turns a working run into a startup error? If so, a note in the helper's comment would help the next person.
Signed-off-by: Fanrong Li <23290157+lfr-0531@users.noreply.github.com>
Signed-off-by: Fanrong Li <23290157+lfr-0531@users.noreply.github.com>
7ea8d3f to
69b54e4
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
…Hopper Signed-off-by: Fanrong Li <23290157+lfr-0531@users.noreply.github.com>
62cbf9e to
050a0d3
Compare
|
/bot run --disable-fail-fast |
|
PR_Github #64574 [ run ] triggered by Bot. Commit: |
|
PR_Github #64574 [ run ] completed with state
|
|
/bot run --disable-fail-fast |
|
PR_Github #64824 [ run ] triggered by Bot. Commit: |
|
PR_Github #64824 [ run ] completed with state
|
|
/bot run --disable-fail-fast |
|
PR_Github #64907 [ run ] triggered by Bot. Commit: |
|
PR_Github #64907 [ run ] completed with state |
…NVIDIA#17293) Signed-off-by: Fanrong Li <23290157+lfr-0531@users.noreply.github.com> Signed-off-by: Xin He (SW-GPU) <200704525+xinhe-nv@users.noreply.github.com>
Description
Qwen3.5 MoE checkpoints with NVFP4 experts currently inherit the Qwen3-Next defaults without selecting a Hopper-compatible MoE GEMM backend. This affects both globally NVFP4 and mixed-precision checkpoints. On H20, the default CUTLASS W4A4 path requires Blackwell and fails during warmup because no supported MoE tactic remains.
This change preserves the existing Qwen3-Next defaults and selects Marlin for Qwen3.5 text and VL MoE models when the checkpoint quantization algorithm is NVFP4 or MIXED_PRECISION and the GPU is in the Marlin-supported SM range. Other quantization configurations and Blackwell keep their existing backend selection.
The change restores the default H20 execution path without introducing an API, dependency, documentation, ownership, or architecture change. Same-node 8-GPU H20 measurements confirmed that the fixed automatic selection matches an explicit-Marlin reference within normal run-to-run variation.
Related NVBug: https://nvbugs/6434512
Test Coverage
pytest tests/unittest/_torch/modeling/test_modeling_qwen3_5_vl_moe.py -k qwen35_moe_model_defaults: 3 passed, 6 deselected.050a0d3036and the modifiedmodeling_qwen3_5.py.tests/integration/test_lists/qa/llm_perf_core.yml: all passed on 8 H20 GPUs.Qwen3.5-397B-A17B-NVFP4-V2verification on 8 H20 GPUs:MIXED_PRECISIONwith 375 FP8 and 60 NVFP4 layer entries.No supported MoE GEMM tactic remains after replacing unsupported NO_SMEM epilogues.MoeConfig(backend='MARLIN')withallowed_backends=['marlin'].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.
GitHub Bot Help
To see a list of available CI bot commands, please comment
/bot help.