Enable optimized Adam backend for MuonWithAuxAdam optimizer - #8278
Enable optimized Adam backend for MuonWithAuxAdam optimizer#8278jinyouzhi wants to merge 9 commits into
Conversation
Signed-off-by: iLeGend <824040212@qq.com>
Signed-off-by: iLeGend <824040212@qq.com>
Signed-off-by: iLeGend <824040212@qq.com>
|
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) |
There was a problem hiding this comment.
Why not this?
| 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) |
There was a problem hiding this comment.
Yes, it's strange. Thank you for your suggestions.
There was a problem hiding this comment.
@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).
…n method in test Signed-off-by: iLeGend <824040212@qq.com>
|
@PKUWZP FYI |
Signed-off-by: iLeGend <824040212@qq.com>
Signed-off-by: iLeGend <824040212@qq.com>
Signed-off-by: iLeGend <824040212@qq.com>
pengdurice
left a comment
There was a problem hiding this comment.
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! DeepSpeed/deepspeed/__init__.py Line 84 in aa3914d 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! |
|
Hi @jinyouzhi, 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. |
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
_select_adam_optimizermethod, improving modularity and allowing consistent backend selection for both Adam and Muon optimizers.CPUAdamandZenflowTests
tested on RTX 5090: 128 Tensors 16,384 FP32 elements, warmup 30, steps 200.