Run Muon once per optimizer step under ZeRO-3, not once per micro-batch - #8600
Conversation
ZeRO-3 applied Muon inside the gradient reduce, which runs every micro-batch. With gradient_accumulation_steps n, the momentum advanced n times per step and Newton-Schulz orthogonalized partial gradients, so training with accumulation diverged from the same batch taken in one micro-batch. ZeRO-1/2 were correct. Without optimizer offload, the reduce path now leaves Muon out and the partitions accumulate the raw gradient. At the step, after the overflow check and before the norm, each Muon sub-group's accumulated gradients are gathered in bounded chunks, orthogonalized once through the same round-robin update, and written back to the partitions. The per-subgroup update is factored out of _apply_distributed_muon_update so both paths share it. The offload path is unchanged. Fixes deepspeedai#8443 Signed-off-by: alanhuangyoo <alanhuangyoo@gmail.com>
| @@ -0,0 +1,100 @@ | |||
| # Copyright (c) Microsoft Corporation. | |||
There was a problem hiding this comment.
should remove Microsoft copyright head.
There was a problem hiding this comment.
Removed, thanks.
| m, | ||
| beta=self.muon_beta, | ||
| ns_method=getattr(self, 'muon_ns_method', 'gram'), | ||
| num_heads=getattr(param, 'muon_num_heads', None)) |
There was a problem hiding this comment.
param is no longer defined, will always get a None here.
There was a problem hiding this comment.
Good catch, that was mine: pulling the grad from the list dropped the param = params[base_i + rank] line. Restored in 31f5e29. Added a test that takes one step with and without per-head under ZeRO-3 and checks it moves q and k but not the MLP. It fails without the fix.
Taking the gradient from the list dropped the line that bound `param` to the parameter being updated, so `muon_num_heads` was read from whichever parameter the momentum loop above left behind. Per-head Muon under ZeRO-3 then split every matrix in the bucket by one parameter's head count. The new test takes one step with and without per-head from the same start: per-head has to change q and k and leave the untagged MLP alone. With the binding missing, the MLP picks up a head count and the test fails. Also drops the Microsoft copyright line from the new test file. Signed-off-by: alanhuangyoo <alanhuangyoo@gmail.com>
31f5e29 to
f494efb
Compare
| numel += group[end].ds_numel | ||
| end += 1 | ||
| params, chunk = group[start:end], partitions[start:end] | ||
| full_grads = self._partitioned_buffers_all_gather(params, chunk, chunk[0].dtype) |
There was a problem hiding this comment.
This all-gather should use self.communication_data_type instead of chunk[0].dtype — otherwise a user-configured fp16 comm dtype is silently overridden and the gather's communication volume and buffer footprint double.
There was a problem hiding this comment.
Fixed in 952cc44, both gathers now use self.communication_data_type. Muon suite still passes (261 on 2 GPUs).
| full_grads = self._partitioned_buffers_all_gather(params, chunk, chunk[0].dtype) | ||
| group_items = [(param, self.grad_position[self.get_param_id(param)][1], full_grad) | ||
| for param, full_grad in zip(params, full_grads)] | ||
| self._muon_update_sub_group(i, group_items, chunk[0].dtype) |
There was a problem hiding this comment.
Same for the momentum gather inside _muon_update_sub_group: the momentum buffer itself is stored in communication_data_type (_create_momentum_buffer), so gathering it in fp32 gains no precision — it only doubles the traffic. Please pass self.communication_data_type so both paths match the storage dtype.
|
Hi @alanhuangyoo , the updated PR looks overall good for me. The code review revealed a fix needed for communication data type, can you take a look at the comments? Thanks! |
The step-time gather passed the partitions' own dtype, so a configured fp16/bf16 communication dtype was ignored and the gradient and momentum gathers moved twice the bytes. The momentum buffer is already stored in the communication dtype, so gathering it wider gained nothing. Signed-off-by: alanhuangyoo <alanhuangyoo@gmail.com>
The reduce path now calls _apply_distributed_muon_update unconditionally, as it does on master, and the function returns early without optimizer offload. Reading offload_optimizer at the call site broke test_zero3_autoep_contiguous_grads_average_by_global_dp, which builds the optimizer without __init__ and stubs that method out. Signed-off-by: alanhuangyoo <alanhuangyoo@gmail.com>
Head branch was pushed to by a user without write access
|
@delock The cpu-torch-latest failure was mine: |
Fixes #8443.
The problem
ZeRO-3 applies Muon inside the gradient reduce (
_apply_distributed_muon_update, called from__avg_scatter_contiguous_grads), and that runs every micro-batch. Withgradient_accumulation_steps: n, the momentum advancesntimes per optimizer step, and Newton-Schulz orthogonalizes each micro-batch's partial gradient instead of the accumulated one. ZeRO-1/2 apply Muon at the accumulation boundary and are correct.On 2 GPUs, fp32, with the same 8 samples per step either way (one micro-batch of 8 at
gas=1, four of 2 atgas=4), three steps, relative difference in the weights:gas=1vsgas=4gas=1vsgas=4gas=4ZeRO-3 now lands on exactly ZeRO-2's figure.
The change
This is option 1 from the discussion in #8443. It is scoped to ZeRO-3 without optimizer offload.
step()calls_apply_muon_to_accumulated_grads()after the overflow check and before the gradient norm. For each Muon sub-group, it:reduce_bucket_sizeas the reduce buckets were;_apply_distributed_muon_updateis moved into_muon_update_sub_group. It takes the full-shape gradients explicitly, because at step time the parameters are partitioned andparam.gradcan't hold them. The reduce path calls it withparam.gradas before.gas=nthat isntimes fewer.The optimizer-offload path is unchanged; #8464 is working on it. jinyouzhi added a pointer to this shape in #8464, and the overlap is limited to
_apply_distributed_muon_update.Testing
On 2×H20:
tests/unit/v1/ops/muon/test_muon_zero3_grad_accum.py. Both tests fail on master and pass here.test_newton_schulz_runs_once_per_matrix_per_step: Newton-Schulz calls summed over ranks come to 2 × steps, not 2 × steps × gas.test_gradient_accumulation_matches_one_large_micro_batch[2, 3]:gas=1andgas=4agree to within half-precision Newton-Schulz noise at both stages.tests/unit/v1/ops/muon/plustests/unit/runtime/zero/test_per_head_muon.py: 260 passed.