Skip to content

[Dev] Remove calculation of padding token in moe routing loss - #2754

Merged
HaochenYuan merged 39 commits into
NVIDIA:devfrom
HaochenYuan:dev
Jan 6, 2026
Merged

[Dev] Remove calculation of padding token in moe routing loss#2754
HaochenYuan merged 39 commits into
NVIDIA:devfrom
HaochenYuan:dev

Conversation

@HaochenYuan

@HaochenYuan HaochenYuan commented Dec 24, 2025

Copy link
Copy Markdown
Contributor

What does this PR do ?

Fix error in mlp recompute with padding mask in previous MR 2121
Related issue: #1984
PR to the main branch: #2142

When enabling mlp recompute, got below functional test error:

File "/opt/megatron-lm/megatron/core/transformer/transformer_layer.py", line 674, in _forward_mlp
[default0]:[rank0]:     mlp_output_with_bias = tensor_parallel.checkpoint(
[default0]:[rank0]:                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^
[default0]:[rank0]: TypeError: checkpoint() got an unexpected keyword argument 'padding_mask'

Fixed it by binding kwargs to recompute function before calling CheckpointFunction.apply

mlp_output_with_bias = tensor_parallel.checkpoint(
                    functools.partial(self.mlp, padding_mask=padding_mask),
                    False,
                    pre_mlp_layernorm_output,
                )

⚠️ For major changes (either in lines of code or in its impact), please make sure to first share discuss a design-doc with the team.

Contribution process

flowchart LR
    A[Pre-checks] --> B[PR Tests]
    subgraph Code Review/Approval
        C1[Expert Review] --> C2[Final Review]
    end
    B --> C1
    C2 --> D[Merge]
Loading

Pre-checks

  • I want this PR in a versioned release and have added the appropriate Milestone (e.g., Core 0.8)
  • 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

The following process is enforced via the CODEOWNERS file for changes into megatron/core. For changes outside of megatron/core, it is up to the PR author whether or not to tag the Final Reviewer team.

For MRs into `main` branch

(Step 1): Add PR label Expert Review

(Step 2): Collect the expert reviewers reviews

  1. Attach the Expert Review label when your PR is ready for review.
  2. GitHub auto-assigns expert reviewers based on your changes. They will get notified and pick up your PR soon.

⚠️ Only proceed to the next step once all reviewers have approved, merge-conflict are resolved and the CI is passing.
Final Review might get declined if these requirements are not fulfilled.

(Step 3): Final Review

  1. Add Final Review label
  2. GitHub auto-assigns final reviewers based on your changes. They will get notified and pick up your PR soon.

(Optional Step 4): Cherry-pick into release branch

If this PR also needs to be merged into core_r* release branches, after this PR has been merged, select Cherry-pick to open a new PR into the release branch.

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.

Merging your PR

Any member of core-adlr and core-nemo will be able to merge your PR.

@Phlip79
Phlip79 removed their request for review December 24, 2025 17:39
@yaox12 yaox12 added Expert Review [deprecated] Apply this label to indicate that your PR is ready for expert review. dev branch Dev branch related issues and development labels Jan 4, 2026
@Victarry
Victarry added this pull request to the merge queue Jan 5, 2026
github-merge-queue Bot pushed a commit that referenced this pull request Jan 5, 2026
Co-authored-by: Li Tao <lit@nvidia.com>
Co-authored-by: Dennis(Zhenhuan) Liu <denliu@nvidia.com>
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jan 5, 2026
@HaochenYuan
HaochenYuan added this pull request to the merge queue Jan 5, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jan 5, 2026
@HaochenYuan
HaochenYuan enabled auto-merge January 6, 2026 03:29
@HaochenYuan
HaochenYuan added this pull request to the merge queue Jan 6, 2026
Merged via the queue into NVIDIA:dev with commit dfa6cc1 Jan 6, 2026
115 of 121 checks passed
@HaochenYuan
HaochenYuan deleted the dev branch January 6, 2026 07:41
@HaochenYuan HaochenYuan mentioned this pull request Jan 16, 2026
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dev branch Dev branch related issues and development Expert Review [deprecated] Apply this label to indicate that your PR is ready for expert review. Run functional tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants