[https://nvbugs/6221450][fix] AutoDeploy: Qwen3.5 400B NVFP4 accuracy regression fix - #14667
Conversation
|
/bot run --extra-stage "DGX_B200-8_GPUs-AutoDeploy-Post-Merge-1" --disable-fail-fast |
|
PR_Github #50691 [ run ] triggered by Bot. Commit: |
|
/bot help |
GitHub Bot Help
Provide a user friendly way for developers to interact with a Jenkins server. Run See details below for each supported subcommand. Details
Launch build/test pipelines. All previously running jobs will be killed.
kill
Kill all running builds associated with pull request. skip
Skip testing for latest commit on pull request. reuse-pipeline
Reuse a previous pipeline to validate current commit. This action will also kill all currently running builds associated with the pull request. IMPORTANT NOTE: This is dangerous since lack of user care and validation can cause top of tree to break. |
|
/bot run --stage-list "DGX_B200-8_GPUs-AutoDeploy-Post-Merge-1" --disable-fail-fast |
|
PR_Github #50696 [ run ] triggered by Bot. Commit: |
|
PR_Github #50691 [ run ] completed with state |
|
PR_Github #50696 [ run ] completed with state
|
2225ea9 to
b99e762
Compare
|
/bot run --stage-list "DGX_B200-8_GPUs-AutoDeploy-Post-Merge-1" --disable-fail-fast |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (4)
💤 Files with no reviewable changes (1)
📝 WalkthroughWalkthroughThis PR refactors Qwen 3.5 MoE tensor-parallel sharding to use hint-driven exclusion for shared experts. It adds a new ChangesQwen MoE Shared Expert Sharding Exclusion
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
PR_Github #50981 [ run ] triggered by Bot. Commit: |
|
/bot run --extra-stage "DGX_B200-8_GPUs-AutoDeploy-Post-Merge-1" --disable-fail-fast |
|
PR_Github #50993 [ run ] triggered by Bot. Commit: |
|
PR_Github #50981 [ run ] completed with state |
|
@taylor-yb-lee What I suggest to do:
Sure you can argue that sometimes, blacklist is more convenient than whitelist, but I'd prefer to avoid having both - it's not clear what if these two contradict each other. |
|
PR_Github #50993 [ run ] completed with state
|
|
/bot run --extra-stage "DGX_B200-8_GPUs-AutoDeploy-Post-Merge-1" --disable-fail-fast |
7b9fb18 to
f0ed2b9
Compare
|
/bot run --extra-stage "DGX_B200-8_GPUs-AutoDeploy-Post-Merge-1" --disable-fail-fast |
f0ed2b9 to
5130806
Compare
|
PR_Github #51171 [ run ] triggered by Bot. Commit: |
|
PR_Github #51172 [ run ] triggered by Bot. Commit: |
|
PR_Github #51215 [ run ] completed with state
|
greg-kwasniewski1
left a comment
There was a problem hiding this comment.
Good for now to fix the bug, but let's follow-up with a more structured approach for passing sharding IR info
|
/bot run --disable-fail-fast --extra-stage "DGX_B200-4_GPUs-AutoDeploy-1,DGX_H100-4_GPUs-AutoDeploy-1,DGX_B200-8_GPUs-AutoDeploy-Post-Merge-1" |
Signed-off-by: Taylor Yeonbok Lee <249374542+taylor-yb-lee@users.noreply.github.com>
…d nodes under IR sharding Add a shard_exclude_filter config so nodes whose weight keys match (e.g. the Qwen3.5-MoE shared expert) are replicated instead of TP-sharded, and reorder the MoE all-reduce so the replicated shared output is added after it (not scaled by world size). Signed-off-by: Taylor Yeonbok Lee <249374542+taylor-yb-lee@users.noreply.github.com>
…M all-reduce Exclude the shared expert from TP sharding via shard_exclude_filter (cheaper replicated at this size), set SYMM_MEM all-reduce on the IR path, and drop the now-dead detect_sharding block. test_nvfp4[8] MMLU 86.4% (was ~27%). Signed-off-by: Taylor Yeonbok Lee <249374542+taylor-yb-lee@users.noreply.github.com>
…_layers whitelist - Tag shared-expert linears layer_type="shared_expert" (was "moe"). - Drop shard_exclude_filter blacklist; reuse existing shard_layers whitelist. - yaml: shard_layers=["moe","delta","mha"] omits shared_expert -> replicated. Signed-off-by: Taylor Yeonbok Lee <249374542+taylor-yb-lee@users.noreply.github.com>
- SwiGLU fusion dropped the layer_type op-arg, so hint-driven sharding (shard_layers) saw lt=None and TP-sharded nodes meant to be replicated. - Add optional layer_type to the unfused SwiGLU ops and propagate the source linears' layer_type onto the fused node in the matchers. gate/up/down linear(layer_type=X) --fuse--> swiglu_mlp() [hint lost] gate/up/down linear(layer_type=X) --fuse--> swiglu_mlp(layer_type=X) [kept] Signed-off-by: Taylor Yeonbok Lee <249374542+taylor-yb-lee@users.noreply.github.com>
2e7db8b to
4fa4379
Compare
|
/bot run --disable-fail-fast --extra-stage "DGX_B200-4_GPUs-AutoDeploy-1,DGX_H100-4_GPUs-AutoDeploy-1,DGX_B200-8_GPUs-AutoDeploy-Post-Merge-1" |
|
PR_Github #51413 [ run ] triggered by Bot. Commit: |
|
PR_Github #51413 [ run ] completed with state
|
|
/bot run --disable-fail-fast --extra-stage "DGX_B200-4_GPUs-AutoDeploy-1,DGX_H100-4_GPUs-AutoDeploy-1,DGX_B200-8_GPUs-AutoDeploy-Post-Merge-1" |
|
PR_Github #51534 [ run ] triggered by Bot. Commit: |
|
PR_Github #51534 [ run ] completed with state |
…ccuracy fix) - Shared-expert NVFP4 TP-sharding corrupts deswizzled weight scales (GSM8K 92.27); tag layer_type="shared_expert", propagate it across the SwiGLU fusion, and exclude it via shard_layers => replicated. GSM8K 94.77@1319 (legacy 95.03), 119 tps. - Reorder MoE merge so the replicated shared output is added AFTER the routed all-reduce (adding before would scale shared by the TP world size). - Ports PR NVIDIA#14667 (Qwen3.5 fix) to the DeepSeek NVFP4 IR path. NOTE: swiglu.py + fuse_swiglu.py here duplicate PR NVIDIA#14667 generic infra (already in upstream/main); drop them on rebase, keep modeling_deepseek.py + the IR config. MoE merge (before -> after): add(routed_partial, shared_partial) --all_reduce("moe")--> [shared TP-sharded] all_reduce("moe")(routed_partial) --> + shared_full [shared replicated] Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Taylor Yeonbok Lee <249374542+taylor-yb-lee@users.noreply.github.com>
…rites AutoDeploy pattern matchers that collapse N fine-grained ops into one (e.g. the SwiGLU matchers) dropped the per-op layer_type sharding hint. The hint-driven sharder (apply_sharding_hints / shard_layers) then read layer_type=None on the fused op and, because the whitelist is fail-open, TP-sharded nodes meant to be replicated -- the root cause of the Qwen3.5-MoE NVFP4 shared-expert accuracy regression fixed point-wise in NVIDIA#14667. Generalize the fix into the matcher infra: add an optional layer_type arg to the intermediate SwiGLU ops, add collect_classification_hints / stamp_hints helpers in node_utils, and propagate the consensus layer_type from the matched nodes onto the replacement op in ADReplacementPatternEntry. Per-weight mechanics (tp_mode, output_sizes, ...) are intentionally not propagated: the fused op's ShardableNode re-derives them structurally and they legitimately differ across constituents (an MLA layer mixes none/colwise/rowwise yet shares one layer_type), so generic propagation of the sharding decision is ill-posed. Tested: GPU-free unit tests for the helpers (consensus / conflict-drop / schema-guarded stamping), an NVFP4 end-to-end regression mirroring the Qwen3.5-MoE bug, and the full transformations/library suite (329 passed). Fixes NVIDIA#14834 Signed-off-by: greg-kwasniewski1 <213329731+greg-kwasniewski1@users.noreply.github.com>
…_oss IR port Closes several pre-existing gaps in the sharding-IR pipeline that together prevented IR-marked modeling files from actually exercising the IR path, and adds the supporting infrastructure for GPT-OSS-style models. Core fixes (transform/library/sharding_ir.py): * IR-vs-legacy dispatch bug: ``has_sharding_ir_markers`` compared ``node.target == torch.ops.auto_deploy.all_reduce`` (OpOverloadPacket), which always returned False because FX nodes carry the ``.default`` OpOverload as their target. Renamed to ``is_shardingIR_enabled`` and fixed to use ``is_op(...)`` so the comparison matches both forms. * Rowwise-bias double-counting: ``LinearShardableNode`` was leaving the bias intact for rowwise sharding. Mirrored the legacy ``detect_column_row_shard`` "drop bias on all but one rank" pattern (reusing ``_load_hook_remove``) so the bias appears exactly once after reduction. * ``torch_attention`` registered to ``WeightedParamShardableNode`` (with a new ``enable_sharding: bool = False`` schema kwarg). Slices per-head free Parameters passed directly to ``torch_attention`` (e.g. GPT-OSS's ``sinks``) along the head dim. * New ``DenseMLPMoEShardableNode`` registered for ``torch.ops.auto_deploy.torch_moe_dense_mlp``. Stacked-tensor MoE variants other than ``triton_mxfp4_moe`` were previously unregistered (left replicated). Verified end-to-end on gpt_oss-ep-only: 8 collectives inserted, rel_rmse=0.0049 (tol=0.02). Modeling: * modeling_gpt_oss.py: IR-port the attention block (Q/K/V/O linears -> torch_linear_simple, head reshapes -> auto_deploy.view with tp_scaled_dim=2, o_proj rowwise + all_reduce, ``enable_sharding=True`` on the ``torch_attention`` call so ``sinks`` is sliced). * modeling_qwen3_5_moe.py: revert the all_reduce-vs-add ordering inversion from NVIDIA#14667 (Taylor's NVFP4 fix). The current ``all_reduce(routed) + shared_partial`` order is mathematically wrong for BF16 sharded routed+shared MoE: routed gets reduced TP-fold while shared stays partial, so the sum is full+partial. Restoring the canonical "add first, all_reduce once" pattern matches the other IR-ported MoE models. NVFP4 path behavior may need a separate investigation. Test harness: * Renamed ``test_sharding_ir_equivalence`` -> ``test_sharding_num_correctness``. Deduplicated to import the production ``is_shardingIR_enabled`` helper instead of a parallel implementation. Dropped an erroneous ``pytest.skip`` inside a ``spawn_multiprocess_job`` worker (would surface as ``Process exited with code 1`` and mask the actual outcome). Added a small ``_KNOWN_FAILING_MODELING_FILES`` skip list with ``granite_moe_hybrid`` as the sole entry (pre-existing modeling-side sharding bug, unchanged from origin/main). * custom_ops/attention/torch_attention.py: added ``enable_sharding: bool = False`` to the op schema (real impl + fake). Skill: * .claude/skills/ad-sharding-ir-port/SKILL.md: documented the new per-head Parameter sharding pattern, corrected stale Pitfall #9. Signed-off-by: greg-kwasniewski1 <213329731+greg-kwasniewski1@users.noreply.github.com>
…ccuracy fix) - Shared-expert NVFP4 TP-sharding corrupts deswizzled weight scales (GSM8K 92.27); tag layer_type="shared_expert", propagate it across the SwiGLU fusion, and exclude it via shard_layers => replicated. GSM8K 94.77@1319 (legacy 95.03), 119 tps. - Reorder MoE merge so the replicated shared output is added AFTER the routed all-reduce (adding before would scale shared by the TP world size). - Ports PR NVIDIA#14667 (Qwen3.5 fix) to the DeepSeek NVFP4 IR path. NOTE: swiglu.py + fuse_swiglu.py here duplicate PR NVIDIA#14667 generic infra (already in upstream/main); drop them on rebase, keep modeling_deepseek.py + the IR config. MoE merge (before -> after): add(routed_partial, shared_partial) --all_reduce("moe")--> [shared TP-sharded] all_reduce("moe")(routed_partial) --> + shared_full [shared replicated] Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Taylor Yeonbok Lee <249374542+taylor-yb-lee@users.noreply.github.com>
…ccuracy fix) - Shared-expert NVFP4 TP-sharding corrupts deswizzled weight scales (GSM8K 92.27); tag layer_type="shared_expert", propagate it across the SwiGLU fusion, and exclude it via shard_layers => replicated. GSM8K 94.77@1319 (legacy 95.03), 119 tps. - Reorder MoE merge so the replicated shared output is added AFTER the routed all-reduce (adding before would scale shared by the TP world size). - Ports PR NVIDIA#14667 (Qwen3.5 fix) to the DeepSeek NVFP4 IR path. NOTE: swiglu.py + fuse_swiglu.py here duplicate PR NVIDIA#14667 generic infra (already in upstream/main); drop them on rebase, keep modeling_deepseek.py + the IR config. MoE merge (before -> after): add(routed_partial, shared_partial) --all_reduce("moe")--> [shared TP-sharded] all_reduce("moe")(routed_partial) --> + shared_full [shared replicated] Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Taylor Yeonbok Lee <249374542+taylor-yb-lee@users.noreply.github.com>
…ccuracy fix) - Shared-expert NVFP4 TP-sharding corrupts deswizzled weight scales (GSM8K 92.27); tag layer_type="shared_expert", propagate it across the SwiGLU fusion, and exclude it via shard_layers => replicated. GSM8K 94.77@1319 (legacy 95.03), 119 tps. - Reorder MoE merge so the replicated shared output is added AFTER the routed all-reduce (adding before would scale shared by the TP world size). - Ports PR NVIDIA#14667 (Qwen3.5 fix) to the DeepSeek NVFP4 IR path. NOTE: swiglu.py + fuse_swiglu.py here duplicate PR NVIDIA#14667 generic infra (already in upstream/main); drop them on rebase, keep modeling_deepseek.py + the IR config. MoE merge (before -> after): add(routed_partial, shared_partial) --all_reduce("moe")--> [shared TP-sharded] all_reduce("moe")(routed_partial) --> + shared_full [shared replicated] Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Taylor Yeonbok Lee <249374542+taylor-yb-lee@users.noreply.github.com>
…ccuracy fix) - Shared-expert NVFP4 TP-sharding corrupts deswizzled weight scales (GSM8K 92.27); tag layer_type="shared_expert", propagate it across the SwiGLU fusion, and exclude it via shard_layers => replicated. GSM8K 94.77@1319 (legacy 95.03), 119 tps. - Reorder MoE merge so the replicated shared output is added AFTER the routed all-reduce (adding before would scale shared by the TP world size). - Ports PR NVIDIA#14667 (Qwen3.5 fix) to the DeepSeek NVFP4 IR path. NOTE: swiglu.py + fuse_swiglu.py here duplicate PR NVIDIA#14667 generic infra (already in upstream/main); drop them on rebase, keep modeling_deepseek.py + the IR config. MoE merge (before -> after): add(routed_partial, shared_partial) --all_reduce("moe")--> [shared TP-sharded] all_reduce("moe")(routed_partial) --> + shared_full [shared replicated] Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Taylor Yeonbok Lee <249374542+taylor-yb-lee@users.noreply.github.com>
…ccuracy fix) - Shared-expert NVFP4 TP-sharding corrupts deswizzled weight scales (GSM8K 92.27); tag layer_type="shared_expert", propagate it across the SwiGLU fusion, and exclude it via shard_layers => replicated. GSM8K 94.77@1319 (legacy 95.03), 119 tps. - Reorder MoE merge so the replicated shared output is added AFTER the routed all-reduce (adding before would scale shared by the TP world size). - Ports PR NVIDIA#14667 (Qwen3.5 fix) to the DeepSeek NVFP4 IR path. NOTE: swiglu.py + fuse_swiglu.py here duplicate PR NVIDIA#14667 generic infra (already in upstream/main); drop them on rebase, keep modeling_deepseek.py + the IR config. MoE merge (before -> after): add(routed_partial, shared_partial) --all_reduce("moe")--> [shared TP-sharded] all_reduce("moe")(routed_partial) --> + shared_full [shared replicated] Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Taylor Yeonbok Lee <249374542+taylor-yb-lee@users.noreply.github.com>
…ccuracy fix) - Shared-expert NVFP4 TP-sharding corrupts deswizzled weight scales (GSM8K 92.27); tag layer_type="shared_expert", propagate it across the SwiGLU fusion, and exclude it via shard_layers => replicated. GSM8K 94.77@1319 (legacy 95.03), 119 tps. - Reorder MoE merge so the replicated shared output is added AFTER the routed all-reduce (adding before would scale shared by the TP world size). - Ports PR NVIDIA#14667 (Qwen3.5 fix) to the DeepSeek NVFP4 IR path. NOTE: swiglu.py + fuse_swiglu.py here duplicate PR NVIDIA#14667 generic infra (already in upstream/main); drop them on rebase, keep modeling_deepseek.py + the IR config. MoE merge (before -> after): add(routed_partial, shared_partial) --all_reduce("moe")--> [shared TP-sharded] all_reduce("moe")(routed_partial) --> + shared_full [shared replicated] Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Taylor Yeonbok Lee <249374542+taylor-yb-lee@users.noreply.github.com>
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Chores
Description
Background / Motivation
Qwen3.5-397B-A17B-NVFP4 accuracy dropped to near 0 after PR#13478.
Root cause
The model was migrated to the hint-based IR sharding path in #13478
(
92c5030, "Switch ... to sharding-IR canonical models"). That changed how theMoE shared expert is handled:
detect_sharding): the shared expert was replicated — it wasintentionally left out of the manual
tp_planand excluded bysimple_shard_filter, so it was never TP-sharded.apply_sharding_hints): the shared expert is matched into afused
torch_nvfp4_swiglu_mlpop and unconditionally TP-sharded. TheNVFP4 swiglu scale-sharding additionally deswizzled every projection's
per-block
weight_scalewith the gate projection's shape, which is wrong forthe differently-shaped
down_projscale — corrupting the shared-expert output.A second, independent regression from the same migration: the IR path reads
allreduce_strategyfromapply_sharding_hints, but the model only declaredSYMM_MEMunder the (now-disabled)detect_sharding, so all-reduces silentlyfell back to NCCL — a perf regression vs. the pre-IR path.
Summary of changes
Replicate the shared expert (correctness). Add a
shard_exclude_filteroption to the IR sharder: nodes whose weight-parameter keys match a substring
(e.g.
shared_expert) are left replicated instead of TP-sharded — the sameconvention as the legacy
simple_shard_filter. The Qwen3.5-MoE config usesshard_exclude_filter: ["shared_expert"], restoring the pre-IR behavior. TheMoE all-reduce is reordered so the replicated shared output is added after
the routed all-reduce (adding before would scale it by the TP world size).
Restore SYMM_MEM all-reduce (perf). Set
allreduce_strategy: SYMM_MEMunder
apply_sharding_hintsso the IR path uses it, and remove the deaddetect_shardingblock (disabled on the IR path; superseded by in-modelsharding hints).
Validation
accuracy/test_llm_api_autodeploy.py::TestQwen3_5_397B_MoE::test_nvfp4[8](8×B200):
SYMM_MEM (0 NCCL), matching the pre-IR baseline.
Impact
shard_exclude_filterdefaults toNone(no behavior change for othermodels); only Qwen3.5-MoE opts in.
Test Coverage
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.