Skip to content

Fix fused MLA and MTP checkpointing for Megatron-FSDP. - #6271

Merged
rapatel merged 5 commits into
NVIDIA:mainfrom
rapatel:ripatel/fsdp_chkpt_fix_main
Aug 20, 2026
Merged

rapatel merged 5 commits into
NVIDIA:mainfrom
rapatel:ripatel/fsdp_chkpt_fix_main

Conversation

@rapatel

@rapatel rapatel commented Aug 4, 2026 •

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

What does this PR do?

Fixes fused MLA and MTP checkpointing for Megatron-FSDP.

Aside from the newly added unit tests, this PR was also tested manually for save/load parity check for the fused MLA + MTP path:

  • Ran a checkpoint save/load round trip on 2 GPUs through the real code in this PR (preprocess_fsdp_dtensor_state_dict -> the MLA/MTP handlers -> split_fused_fsdp_param), with a real torch.distributed.checkpoint save and load and the new validate_fsdp_dtensor_model_load. All checks passed.
  • The model reproduces: a fused linear_qkv_down_proj with asymmetric sections, an input_layernorm absorbed as layer_norm_weight, and an MTP layer whose inner layer is mtp_model_layer in the model but transformer_layer on disk, and which itself contains a fused attention, so the MLA-before-MTP handler ordering is covered.

The 3 main checks were:

  1. Unfused checkpoint -> fused model
    • 16 of 16 tensors bit-identical to what was saved, 0 mismatches across ranks
    • loss 1.2154645919799805 vs 1.2154645919799805, delta 0.000e+00, bit-identical
  2. Fused -> fused round trip (destination model starts from a different seed)
    • 16 of 16 on-disk tensors bit-identical
    • 13 of 13 weights the forward pass reads bit-identical
    • loss 1.1673169136047363 before save and after load, bit-identical
    • all gradients bit-identical, e.g. linear_qkv_down_proj.weight gradient norm 0.16416169703006744 before and after
  3. Negative control, identical load with the MLA handler disabled
    • CheckpointingException raised, naming the 6 unloadable fused parameters
    • with --dist-ckpt-strictness ignore_all, the same load silently keeps those 6 at their initial values, which is the behavior that broke convergence before this change
  • Additionally, we ran a 64-node (256 GB300 GPUs) DSv3 convergence run with these changes, which ran as expected within the target samples-to-converge range for the target GBS.

⚠️ For major changes (either in lines of code or in its impact), please make sure to first share a design doc with the team. If you're unsure what's the best way to do so, contact @NVIDIA/mcore-oncall.

Issue tracking

For PRs from open-source community contributors:

  • New features: a linked issue is required. Please open a feature request and reference it here before submitting the PR.
  • Small updates (bug fixes, minor improvements): a linked issue is recommended and will accelerate the PR review process.

Linked issue:

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 @NVIDIA/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.

@rapatel
rapatel requested a review from a team as a code owner August 4, 2026 22:23
@rapatel
rapatel requested a review from a team August 4, 2026 22:23
@rapatel
rapatel requested review from a team as code owners August 4, 2026 22:23
@copy-pr-bot

copy-pr-bot Bot commented Aug 4, 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 August 4, 2026 22:23
@github-actions

github-actions Bot commented Aug 4, 2026

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.

@rapatel
rapatel marked this pull request as ready for review August 4, 2026 22:34
@rapatel

rapatel commented Aug 4, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test aa30d8f.

@cspades cspades left a comment •

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.

I think it LGTM (there's a lot of code but a good bit is familiar code). Have 1 comment on arguments, and also an ask that you check we at least have loss and grad parity loading a non-trivial checkpoint with the model this PR is supporting, thanks!

Comment on lines 574 to 592
strict_fsdp_dtensor_model_load: Literal[
"assume_ok_unexpected",
"log_unexpected",
"log_all",
"raise_unexpected",
"raise_all",
"return_unexpected",
"return_all",
"ignore_all",
] = "raise_unexpected"
"""Determine handling of model weights an FSDP DTensor partial load cannot supply.
Check StrictHandling docs for flags meaning. Only applies when strict_fsdp_dtensor_load is
False, where the underlying load would otherwise skip such weights silently and leave them
at their initialized values. Unlike strict_fsdp_dtensor_load this covers only the model
sections, so optimizer and RNG state that is intentionally absent does not trip it. Weights
the checkpoint holds but this rank does not need are always fine, as expected under
pipeline, tensor and expert parallelism. Ignored when finetune is set."""

dist_ckpt_strictness: Literal[

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.

Can we just pipe the existing dist_ckpt_strictness argument to FSDP?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done.

Comment on lines +577 to +581
def _intersect_slice(s1, s2):
"""Intersection of two step-1 slices, or an empty slice when they do not overlap."""
start = max(s1.start, s2.start)
stop = min(s1.stop, s2.stop)
return slice(0, 0) if start >= stop else slice(start, stop)

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.

There's a similar function in the SwiGLU transforms, I think this can be used for that.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done.

@rapatel

rapatel commented Aug 5, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test 6fee9a5.

@rapatel

rapatel commented Aug 6, 2026

Copy link
Copy Markdown
Contributor Author

@cspades Please see updated description of this PR - it includes the details from the save/load parity testing. Thanks.

@gautham-kollu gautham-kollu added the Final Review PR is in the "final review" stage label Aug 10, 2026
Signed-off-by: Ritesh Patel <ripatel@nvidia.com>
…e helpers.

Signed-off-by: Ritesh Patel <ripatel@nvidia.com>
@rapatel
rapatel force-pushed the ripatel/fsdp_chkpt_fix_main branch from 6fee9a5 to 1515dc7 Compare August 11, 2026 23:03
@rapatel

rapatel commented Aug 11, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test 1515dc7.

…nit test.

Signed-off-by: Ritesh Patel <ripatel@nvidia.com>
@rapatel
rapatel force-pushed the ripatel/fsdp_chkpt_fix_main branch from e61006c to a4f7722 Compare August 19, 2026 18:03
@gautham-kollu gautham-kollu added the Final Review PR is in the "final review" stage label Aug 19, 2026
@svcnvidia-nemo-ci svcnvidia-nemo-ci removed the Final Review PR is in the "final review" stage label Aug 19, 2026
@svcnvidia-nemo-ci svcnvidia-nemo-ci added the Approved All necessary approvals have been made label Aug 19, 2026
@gautham-kollu

Copy link
Copy Markdown
Contributor

/ok to test 6069334

@rapatel
rapatel added this pull request to the merge queue Aug 19, 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/32320591753

@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Aug 20, 2026
@rapatel
rapatel added this pull request to the merge queue Aug 20, 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/32345646452

Merged via the queue into NVIDIA:main with commit 93924d5 Aug 20, 2026
188 checks passed
@rapatel
rapatel deleted the ripatel/fsdp_chkpt_fix_main branch August 20, 2026 09:41
devnkong pushed a commit to devnkong/Megatron-LM that referenced this pull request Aug 22, 2026
Signed-off-by: Ritesh Patel <ripatel@nvidia.com>
Co-authored-by: gautham-kollu <gkollu@nvidia.com>
Signed-off-by: Kezhi Kong <kezhik@kezhik-mlt.client.nvidia.com>
ksivaman pushed a commit to ksivaman/Megatron-LM that referenced this pull request Sep 1, 2026
Signed-off-by: Ritesh Patel <ripatel@nvidia.com>
Co-authored-by: gautham-kollu <gkollu@nvidia.com>

This branch was previously deployed

2 inactive deployments
public — 60693345 Deployed Aug 19, 2026 by copy-pr-bot[bot] via finalize / publish-docs #6663
test — 60693345 Deployed Aug 19, 2026 by copy-pr-bot[bot] via cicd-wait-in-queue #19504
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 complexity: medium Run functional tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants