Skip to content

Support GEMM + Swiglu fused MLP - #3971

Closed
ksivaman wants to merge 11 commits into
NVIDIA:mainfrom
ksivaman:fused_grouped_mlp_main
Closed

Support GEMM + Swiglu fused MLP#3971
ksivaman wants to merge 11 commits into
NVIDIA:mainfrom
ksivaman:fused_grouped_mlp_main

Conversation

@ksivaman

Copy link
Copy Markdown
Member

What does this PR do ?

This PR supports GEMM + Swiglu fused MLP via Transformer Engine sequential ops, including:

  • TE GroupedTensor support in distributed optimizer.
  • TE GroupedTensor checkpointing support for cases to load a multi-param (1 param per expert) checkpoint into a single param config.

Note: Support is already added in dev branch and this is cherry-picking #3890.

Contribution process

Pre-checks

  • I have added relevant unit tests
  • I have added relevant functional tests
  • I have added proper typing to my code Typing guidelines
  • I have added relevant documentation
  • I have run the autoformatter.sh on my PR

Code review

Feel free to message or comment the @mcore-oncall to help accelerate your merge into main. The less complex your PR is, the faster it will be approved and merged!

All PRs start as draft. If you open a non-draft PR, it will be automatically converted to draft.

Step 1: Mark PR as "Ready for Review"

  1. When your PR is ready, click Ready for Review.
  2. An oncall reviewer is auto-assigned and expert reviewers are notified based on your changes.
    • Some PRs may jump straight to step 2. This is determined by .github/CODEOWNERS.

⚠️ Only mark as ready once merge-conflicts are resolved and the CI is passing.
Final Review might get declined if these requirements are not fulfilled.

Step 2: Final Review

For PRs that change megatron/core, once all expert reviewers have approved, the Final Review label is applied automatically and final reviewers are assigned.

For PRs outside megatron/core, this step is skipped.

Step 3: Approved

Once all required reviewers have approved, the Approved label is applied automatically.

Merge

Any member of mcore-engineers will be able to merge your PR.

For MRs into `dev` branch The proposed review process for `dev` branch is under active discussion.

MRs are mergable after one approval by either eharper@nvidia.com or zijiey@nvidia.com.

Signed-off-by: Kirthi Shankar Sivamani <ksivamani@nvidia.com>
Signed-off-by: ksivamani <ksivamani@nvidia.com>
@ksivaman
ksivaman requested review from a team as code owners March 20, 2026 19:04
@copy-pr-bot

copy-pr-bot Bot commented Mar 20, 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.

@svcnvidia-nemo-ci
svcnvidia-nemo-ci marked this pull request as draft March 20, 2026 19:04
@github-actions

Copy link
Copy Markdown
Contributor

This PR has been automatically converted to draft because all PRs must start as drafts.

When you are ready for review, click Ready for Review to begin the review process. This will:

  1. Add the oncall reviewer (optional reviewer)
  2. Add required review teams based on your changes

See the contribution guide for more details.

@ksivaman
ksivaman marked this pull request as ready for review March 20, 2026 19:04
@svcnvidia-nemo-ci
svcnvidia-nemo-ci requested a review from a team March 20, 2026 19:05
@Phlip79

Phlip79 commented Mar 20, 2026

Copy link
Copy Markdown
Member

/claude review

@Phlip79

Phlip79 commented Mar 20, 2026

Copy link
Copy Markdown
Member

/ok to test ba650c7

@svcnvidia-nemo-ci svcnvidia-nemo-ci added this to the Core 0.16 milestone Mar 20, 2026
Comment thread megatron/core/transformer/moe/experts.py Outdated

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One minor typo flagged inline.

The new fused MLP path, grouped-tensor checkpoint normalization, and grouped quantized tensor handling in the distributed optimizer are substantial additions. The only test update is adding new config fields to a golden dict — there don't appear to be any unit tests covering the new logic (fused forward, _expand_quantized_param_shard_for_cast, _split_grouped_checkpoint_tensor, normalize_grouped_parameter_keys, etc.). Consider adding targeted tests for these, especially the shard-splitting and checkpoint key remapping logic which have multiple code paths.

@erhoo82 erhoo82 added the dev2main: mbridge dev to main: this PR is needed in main for mbridge label Mar 23, 2026
@Phlip79

Phlip79 commented Mar 24, 2026

Copy link
Copy Markdown
Member

/ok to test be3ec51

Signed-off-by: Kirthi Shankar Sivamani <ksivamani@nvidia.com>
@ksivaman

Copy link
Copy Markdown
Member Author

/ok to test ba650c7

@copy-pr-bot

copy-pr-bot Bot commented Mar 24, 2026

Copy link
Copy Markdown

/ok to test ba650c7

@ksivaman, there was an error processing your request: E2

See the following link for more information: https://docs.gha-runners.nvidia.com/cpr/e/2/

@ksivaman

Copy link
Copy Markdown
Member Author

/ok to test d64a6bf

"moe_router_force_biased": None,
"inference_grouped_gemm_backend": "auto",
"inference_moe_disable_fused_quant_kernels": False,
"moe_mlp_glu_interleave_size": None,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we should add some test when moe_mlp_glu_interleave_size is not None as this is something we are adding now ?

self.cached_grad_buffer_shard_list = [None] * len(self.buckets)
# Track grad mode used to create cached param views. Rebuild if mode changes to avoid
# mixing no_grad-created views with in-place updates in grad-enabled mode.
self._cached_param_buffer_shards_grad_enabled = None

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the purpose of this?

if self._cached_param_buffer_shards_grad_enabled != current_grad_enabled:
self.cached_param_buffer_shard_list = [None] * len(self.buckets)
self._cached_param_buffer_shards_grad_enabled = current_grad_enabled
with torch.no_grad():

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment on what this is trying to do?

if self.config.use_precision_aware_optimizer_no_fp8_or_ds_fp8:
return

fp8_params, shard_fp32_from_fp8, shard_offsets_in_fp8 = (

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment on what this is trying to do?

@Phlip79

Phlip79 commented Mar 30, 2026

Copy link
Copy Markdown
Member

/ok to test cb7d273

yaox12 added 2 commits March 31, 2026 02:29
Signed-off-by: Xin Yao <xiny@nvidia.com>
@yaox12

yaox12 commented Mar 31, 2026

Copy link
Copy Markdown
Member

/ok to test 439688e

else self.linear_fc2.weight0.dtype
)

# TODO:ksivamani: Why meta device?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ksivaman If we didn't specify device="meta", then we would allocate memory for the weights. However, we want to reuse the weights from the GroupedLinear module.

@dingqingy-nv dingqingy-nv added core_r0.17.0 Auto-cherrypick to release branch. Apply before merge; cherrypick happens after merge. 26.06 labels Apr 10, 2026
@zhongbozhu

Copy link
Copy Markdown
Contributor

use_transformer_engine_op_fuser is not added to the arguments.py, is that intended?

@Connor-XY

Copy link
Copy Markdown
Contributor

Close this as we have #4636

@Connor-XY Connor-XY closed this May 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

26.06 complexity: high core_r0.17.0 Auto-cherrypick to release branch. Apply before merge; cherrypick happens after merge. dev2main: mbridge dev to main: this PR is needed in main for mbridge

Projects

None yet

Development

Successfully merging this pull request may close these issues.