Skip to content

Fix the FC1 size of the fused shared expert with tensor parallel - #8145

Merged
gautham-kollu merged 1 commit into
NVIDIA:mainfrom
michal2409:fix/fused-shared-expert-tp-local-fc1
Oct 28, 2026
Merged

gautham-kollu merged 1 commit into
NVIDIA:mainfrom
michal2409:fix/fused-shared-expert-tp-local-fc1

Conversation

@michal2409

@michal2409 michal2409 commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor
  • I, the PR author, have personally reviewed every line of this PR.

What does this PR do?

Fix the FC1 out_features of FusedSharedExpertMLP for tensor parallel (TP) > 1. Then TE can fuse the shared expert MLP.

The problem

_make_fused_grouped_swiglu_ops makes the TE GroupedLinear op for FC1 with
out_features=fc1_weight.size(0) * tp_world_size. But fc1_weight is the local TP shard
(op.weight0 = fc1_weight), and te.pytorch.ops.GroupedLinear has no TP mode. So the op must use the local size.

With TP > 1, TE does not fuse GroupedLinear + ScaledSwiGLU + GroupedLinear, because
validate_grouped_mlp_dims needs fc1.out_features == 2 * fc2.in_features.
TE then runs the unfused ops. The unfused ScaledSwiGLU needs one scale for each row,
but the shared expert gives one unit scale. The forward pass stops with:

transformer_engine/pytorch/csrc/extensions/activation.cpp:367 in function scaled_activation_compute:
Assertion failed: scales_tensor.numel() == rows. scaled activation expects one scale per input row

This problem does not occur with TP=1, because then the two sizes are equal.
We found it with DeepSeek-V3 on GB200: TP=2 with sequence parallel, MXFP8,
use_grouped_gemm_for_shared_expert=True, and moe_shared_expert_glu_interleave_size=32.

Changes

  • Use out_features=fc1_weight.size(0) for the FC1 op.
  • Remove the get_pg_size import, because it has no use now.
  • Add test_make_fused_grouped_swiglu_ops_uses_local_shard_sizes_with_tp.
    The existing tests set the TP size to 1, so they did not find the problem.

Tests

  • tests/unit_tests/transformer/moe/test_shared_experts.py on one GB200 node (4 GPUs): 23 tests pass.
    test_shared_expert_forward_backward[2-4-alltoall] needs 8 GPUs (TP2 x EP4), so it cannot run there.
  • The new test on main without this fix fails with assert 16 == 8.
  • black, isort, ruff, and pylint (versions from pyproject.toml) pass on the two changed files.
  • End-to-end test with the same one-line change on 26.09-alpha.rc2:
    • Model: a DeepSeek-V3 proxy with 19 layers and 256 experts.
    • System: 16 GB200 nodes, TP2 PP4 VP4 EP16, MXFP8, full-iteration CUDA graph.
    • Without the fix, warmup step 0 stops with the assertion above.
    • With the fix, 10 training steps and the evaluation complete.
    • The eval loss (9.698) agrees with a run that uses the non-fused shared expert (9.696).

Issue tracking

Linked issue: none. Related: #8024 makes the fused path use the TP group of the module for its collectives.
The two changes do not overlap. A TP > 1 run needs both if tp_group is not the global TP group.

Contribution process

Pre-checks

  • I have added relevant unit tests
  • I have added relevant functional tests
  • Determinism test for a changed GPU kernel: not applicable, because this PR changes no kernel.
  • I have added proper typing to my code Typing guidelines
  • I have added relevant documentation
  • I have run the autoformatter.sh on my PR

🤖 Generated with Claude Code

FusedSharedExpertMLP makes the TE GroupedLinear op for FC1 with
out_features=fc1_weight.size(0) * tp_world_size. But fc1_weight is the
local tensor-parallel shard, and GroupedLinear has no tensor-parallel mode.
So out_features must be the local size.

With TP > 1, TE does not fuse GroupedLinear + ScaledSwiGLU +
GroupedLinear, because the fusion needs FC1 out_features ==
2 * FC2 in_features. The unfused ScaledSwiGLU then gets the single unit
scale of the shared expert and stops with
"scales_tensor.numel() == rows". This problem does not occur with TP=1.

Use the local size, and remove the get_pg_size import that has no use
now. Add a unit test for TP=2.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Michal Futrega <mfutrega@nvidia.com>
@copy-pr-bot

copy-pr-bot Bot commented Oct 12, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@michal2409
michal2409 marked this pull request as ready for review October 14, 2026 13:55
@michal2409
michal2409 requested review from a team as code owners October 14, 2026 13:55
@svcnvidia-nemo-ci svcnvidia-nemo-ci added the Final Review PR is in the "final review" stage label Oct 27, 2026
@gautham-kollu

Copy link
Copy Markdown
Contributor

/ok to test bc56a09

@svcnvidia-nemo-ci svcnvidia-nemo-ci added Approved All necessary approvals have been made and removed Final Review PR is in the "final review" stage labels Oct 27, 2026
@gautham-kollu
gautham-kollu added this pull request to the merge queue Oct 28, 2026
@nemo-automation-bot

Copy link
Copy Markdown

🔄 Merge queue validation started!

You can track the progress here: https://github.com/NVIDIA/Megatron-LM/actions/runs/40678856079

@nemo-automation-bot

Copy link
Copy Markdown

🔄 Merge queue validation started!

You can track the progress here: https://github.com/NVIDIA/Megatron-LM/actions/runs/40683345995

Merged via the queue into NVIDIA:main with commit 3e47f31 Oct 28, 2026
124 of 126 checks passed

This branch was previously deployed

2 inactive deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Approved All necessary approvals have been made

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants