Skip to content

Merge emerging-optimizers change from dev to main - #4060

Closed
skyw wants to merge 13 commits into
NVIDIA:mainfrom
skyw:merge-optimizer-dev-to-main
Closed

Merge emerging-optimizers change from dev to main#4060
skyw wants to merge 13 commits into
NVIDIA:mainfrom
skyw:merge-optimizer-dev-to-main

Conversation

@skyw

@skyw skyw commented Mar 30, 2026

Copy link
Copy Markdown
Contributor

What does this PR do ?

  • Emerging optimizer registry: New emerging_optimizers.py with registry pattern for Muon, AdaptiveMuon, SOAP, Lion, and future
    optimizers. Muon code consolidated from standalone muon.py (now a backward-compat shim) into the registry framework.
  • Layer-wise optimizer improvements: Simplified LayerWiseDistributedOptimizer init (removed async_allgather param, uses config
    directly), improved error messages for pre-wrapped optimizers.
  • Config updates: New fields (use_layer_wise_distributed_optimizer, SOAP/AdaptiveMuon params), muon_use_nesterov renamed to
    muon_nesterov, AdamOptimizerConfig/SGDOptimizerConfig simplified to aliases.
  • CLI args: Expanded --optimizer choices (soap, adaptive_muon), --muon-nesterov rename, dist_muon deprecation with auto-redirect to
    muon + layer-wise.
  • ChainedOptimizer fix: _split_state_dict now uses model chunk identity mapping instead of sequential offset, fixing state dict
    splitting for mixed optimizer types.

related:

⚠️ 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 the @mcore-oncall.

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.

skyw and others added 4 commits March 30, 2026 09:22
Bring over optimizer improvements from the dev branch including:
- Emerging optimizers registry and refactoring (Lion, AdaptiveMuon, etc.)
- Optimizer state and master weight CPU offloading
- Layer-wise optimizer improvements with --overlap-param-gather support
- Muon optimizer cleanup and consolidation into emerging optimizers framework
- Updated unit tests (new: emerging_optimizers, lion, state_offloading; removed: muon_optimizer)
- Updated functional test golden values

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…rad_buffer.py

- arguments.py: Add CLI args for new optimizers (soap, adaptive_muon),
  --offload-optimizer-states flag, --muon-nesterov rename, dist_muon
  deprecation handling, use_layer_wise_distributed_optimizer logic,
  and emerging optimizer validation checks
- param_and_grad_buffer.py: Add grad mode tracking for cached param
  views to avoid stale views when grad mode changes

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Reverts: OptimizerStateOffloader, distrib_optimizer quantized tensor
changes, offload_optimizer_states config/CLI arg, and offloading test.
These will be merged separately.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@copy-pr-bot

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

skyw and others added 2 commits March 30, 2026 09:52
Lion is part of the emerging_optimizers package, so a single
HAVE_EMERGING_OPTIMIZERS check is sufficient.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Hao Wu <skyw@nvidia.com>
@skyw
skyw marked this pull request as ready for review March 30, 2026 16:54
@skyw
skyw requested review from a team as code owners March 30, 2026 16:54
@svcnvidia-nemo-ci
svcnvidia-nemo-ci requested a review from a team March 30, 2026 16:54
@svcnvidia-nemo-ci svcnvidia-nemo-ci added Final Review PR is in the "final review" stage complexity: high labels Mar 30, 2026
@skyw

skyw commented Mar 30, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test 04e0d35

@svcnvidia-nemo-ci svcnvidia-nemo-ci added this to the Core 0.16 milestone Mar 30, 2026
@skyw

skyw commented Mar 30, 2026

Copy link
Copy Markdown
Contributor Author

@FDecaYed plz take a look. Pay attention to the muon-scalar-optimizer logic which could be breaking.

And some uv settings may be out of sync. CI returned strange missing package errors.

@skyw
skyw requested a review from FDecaYed March 30, 2026 17:02
@skyw
skyw marked this pull request as draft March 30, 2026 17:06
skyw and others added 2 commits March 30, 2026 10:19
@skyw
skyw marked this pull request as ready for review March 30, 2026 17:19
@skyw

skyw commented Mar 30, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test 0765b35

@skyw
skyw marked this pull request as draft March 30, 2026 17:24
@skyw skyw removed the Final Review PR is in the "final review" stage label Mar 30, 2026
@skyw
skyw marked this pull request as ready for review March 31, 2026 01:23
@svcnvidia-nemo-ci svcnvidia-nemo-ci added Final Review PR is in the "final review" stage and removed complexity: high labels Mar 31, 2026
skyw added 2 commits March 30, 2026 18:36
Signed-off-by: Hao Wu <skyw@nvidia.com>
Signed-off-by: Hao Wu <skyw@nvidia.com>
@skyw

skyw commented Mar 31, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test 2a21bba

@skyw
skyw marked this pull request as draft March 31, 2026 01:41
Signed-off-by: Hao Wu <skyw@nvidia.com>
@skyw
skyw marked this pull request as ready for review March 31, 2026 03:41
@skyw

skyw commented Mar 31, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test ce7174e

@skyw
skyw marked this pull request as draft March 31, 2026 03:42
@skyw skyw removed the Final Review PR is in the "final review" stage label Mar 31, 2026

@FDecaYed FDecaYed 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.

some minor issues. otherwise looks good to me

Comment thread megatron/core/distributed/param_and_grad_buffer.py
if state_dict is not None:
if len(self.model_chunks) == 1:
state_dicts[0] = state_dict
# When there is only one global model chunk, all sub-optimizers

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.

changes from #3676
@kunlunl also agree to merge here in one go

Comment thread megatron/core/safe_globals.py
# Muon optimizer check
if 'muon' in args.optimizer:
# emerging optimizer check
if not hasattr(args, 'use_layer_wise_distributed_optimizer'):

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.

maybe just add this as a usual parameter below, and set to default false there? it might help allow downstream properly set it(bridge for example). because sometime they just create args without running code logics here.
not a hard request though

@skyw
skyw marked this pull request as ready for review March 31, 2026 15:27
@svcnvidia-nemo-ci svcnvidia-nemo-ci added Final Review PR is in the "final review" stage complexity: high and removed complexity: high labels Mar 31, 2026
…o-main

# Conflicts:
#	pyproject.toml
#	uv.lock
@skyw skyw added core_r0.17.0 Auto-cherrypick to release branch. Apply before merge; cherrypick happens after merge. and removed core_r0.17.0 Auto-cherrypick to release branch. Apply before merge; cherrypick happens after merge. labels Mar 31, 2026
@skyw

skyw commented Mar 31, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test f4d9301

@skyw

skyw commented Mar 31, 2026

Copy link
Copy Markdown
Contributor Author

Closing.
In favor of small PRs instead of combined ones.

@skyw skyw closed this Mar 31, 2026
@skyw
skyw deleted the merge-optimizer-dev-to-main branch May 6, 2026 20:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

complexity: high Final Review PR is in the "final review" stage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants