Skip to content

Enable optimized Adam backend for MuonWithAuxAdam optimizer - #8278

Open
jinyouzhi wants to merge 9 commits into
deepspeedai:masterfrom
jinyouzhi:muonauxadam
Open

Enable optimized Adam backend for MuonWithAuxAdam optimizer#8278
jinyouzhi wants to merge 9 commits into
deepspeedai:masterfrom
jinyouzhi:muonauxadam

Conversation

@jinyouzhi

Copy link
Copy Markdown
Contributor

Motivation

This pull request aim to extend the Muon optimizer auxiliary Adam(w) optimizer to support more built-in optimized implementation such as FusedAdam.

Changes

  • Extracted Adam/AdamW backend selection logic into a new _select_adam_optimizer method, improving modularity and allowing consistent backend selection for both Adam and Muon optimizers.
  • Keep original naive implementation as fallback path and extend to support weight-decay (AdamW)
  • Support CPUAdam and Zenflow

Tests

pytest tests/unit/ops/muon/test_muon_partial_training.py
图片

tested on RTX 5090: 128 Tensors 16,384 FP32 elements, warmup 30, steps 200.

Signed-off-by: iLeGend <824040212@qq.com>
Signed-off-by: iLeGend <824040212@qq.com>
Signed-off-by: iLeGend <824040212@qq.com>
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.

],
)
def test_adam_backend_selection(engine, parameters, adam_w_mode, expected_class, expected_kwargs):
optimizer_class, optimizer_kwargs = DeepSpeedEngine._select_adam_optimizer(engine, parameters, adam_w_mode)

@sfc-gh-truwase sfc-gh-truwase Aug 19, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why not this?

Suggested change
optimizer_class, optimizer_kwargs = DeepSpeedEngine._select_adam_optimizer(engine, parameters, adam_w_mode)
optimizer_class, optimizer_kwargs = engine._select_adam_optimizer(parameters, adam_w_mode)

@jinyouzhi jinyouzhi Aug 19, 2026

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.

Yes, it's strange. Thank you for your suggestions.

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.

Fixed.

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.

@sfc-gh-truwase Sorry, I need to revert this modification. engine here is an instance of the local _AdamSelectionEngine test double, not a fully constructed DeepSpeedEngine. Because of that, we need to call DeepSpeedEngine's method directly (DeepSpeedEngine.get_optimizer_configuration(engine, ...)) rather than as a bound method on engine.

Fully instantiating a real DeepSpeedEngine (via deepspeed.initialize()) would require a model, config, and distributed backend — overhead that's disproportionate to what this test is actually checking, and it would distract from the test's real purpose (verifying Adam-backend selection logic).

Comment thread tests/unit/ops/muon/test_muon_partial_training.py
@sfc-gh-truwase
sfc-gh-truwase requested a review from PKUWZP August 19, 2026 16:03
@sfc-gh-truwase

Copy link
Copy Markdown
Collaborator

@PKUWZP FYI

Comment thread tests/unit/ops/muon/test_muon_partial_training.py
Signed-off-by: iLeGend <824040212@qq.com>
Signed-off-by: iLeGend <824040212@qq.com>
Signed-off-by: iLeGend <824040212@qq.com>

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

Thank you for the great PR. I think this changes warrants an end to end test with loss value and curves reported on a realistic training (e.g. a 128m model for 100 steps) between baseline and this change. LMK what you think.

@jinyouzhi

Copy link
Copy Markdown
Contributor Author

Thank you for the great PR. I think this changes warrants an end to end test with loss value and curves reported on a realistic training (e.g. a 128m model for 100 steps) between baseline and this change. LMK what you think.

Thank you for your suggestion. I agree that an end-to-end training loss comparison would be necessary. I’ll set up a training-loss curve check focusing on the auxiliary Adam backend with Muon. Do you have a particular model or configuration you would recommend?

@pengdurice

Copy link
Copy Markdown
Contributor

Thank you for the great PR. I think this changes warrants an end to end test with loss value and curves reported on a realistic training (e.g. a 128m model for 100 steps) between baseline and this change. LMK what you think.

Thank you for your suggestion. I agree that an end-to-end training loss comparison would be necessary. I’ll set up a training-loss curve check focusing on the auxiliary Adam backend with Muon. Do you have a particular model or configuration you would recommend?

Thank you!
Let's maybe use a small model e.g. 128mb ish? and make sure the naming of the parameters are recognizable by how we split between muon and Adam (

def set_optimizer_flags(config_class: DeepSpeedConfig, model: torch.nn.Module) -> None:
)
test baseline and the possible arms after the PR (fused Adam, cpu Adam etc)
Also, make sure the lr are set appropriately (not sure if the grad clipping issue is solved or not), the seed is set so for each arm same data example is used by different arms at the same step.
LMK what you think, thank you!

@tohtana

tohtana commented Aug 20, 2026

Copy link
Copy Markdown
Collaborator

Hi @jinyouzhi,
Thank you for submitting this PR! This is a significant improvement. The change looks okay to me, but I want to give you a heads-up.

Before this PR, non-Muon groups under the Muon optimizer use the inline Adam path. After this PR is merged, the same Muon configuration automatically selects FusedAdam. A pre-PR checkpoint therefore loads under the same user-facing optimizer and configuration, but its first resumed step fails because the new backend expects fields absent from the saved state (e.g. KeyError: 'bias_correction').
Is there a good way to avoid it?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants