Repository navigation
perf(optim): stream Muon optimizer state per batch in CPU offload - #4116
faresobeid wants to merge 1 commit into
Conversation
e379b54 to
4091e72
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 4091e72. Configure here.
| compute.wait_event(loaded) | ||
| for group, step in zip(self.param_groups, group_steps): | ||
| if step is not None: | ||
| group["step"] = step |
There was a problem hiding this comment.
Packed params can be updated twice
High Severity
Chunk units come from create_param_batches with _matrix_partitions, so one packed parameter can land in several same-shape matrix batches. Planning and _step_params still key on the physical tensor, so each chunk that contains that tensor runs a full optimizer.step() on it. Fused QKV or gate-up weights can be updated more than once per optimizer step once those batches no longer fit in a single CHUNK_BYTES window.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 4091e72. Configure here.


Stacked on #4008.
With
model.offload = "optimizer", Muon now uses #4008's chunked state offload instead of moving all of its momentum to the GPU at once. Each chunk loads on a side stream while the previous chunk is updated, and the chunk before that is written back.create_param_batchesgives batches of same-shape matrices, one per rank of the group's mesh. The AdamW group inside Muon is one unit, so it stays a single foreach update. Chunks are cut on the mean shard size (numel / mesh size), not on the local size. So every rank cuts at the same batches even with uneven shards, and Muon's all-to-alls match across ranks.group["step"]once perstep()call, and its AdamW bias correction reads it. Each chunk therefore starts from the same count, so the count advances by one per optimizer step, as before.Checks
offload = "optimizer", optimizer step only (random grads), steady state, max over ranks:tests/unit/train/test_state_offload.py: 6 passed on an H200. The new test checks that Muon (matrices plus an AdamW group) with one unit per chunk is bitwise equal to plain Muon after 4 steps.ruff check,ruff format --check.Note
Medium Risk
Touches distributed optimizer stepping and chunk synchronization for Muon collectives; incorrect batching or step counters could change training numerics, though a parity unit test mitigates this.
Overview
Extends CPU offload chunked stepping (previously AdamW-only) to dion
Muon, so optimizer state is streamed in batches instead of loading all momentum onto GPU each step.Chunking now groups step units rather than single parameters: Muon matrix groups are split with
create_param_batches(same-shape batches per mesh rank); embedded AdamW groups stay one unit. Chunk boundaries use mean shard bytes (numel / mesh size) so every rank splits at the same units despite uneven local shards, keeping Muon's collectives aligned.Before each chunk update,
group["step"]is reset to the value at the start of the fullstep()call so dion's per-group counter still advances once per optimizer step across multiple chunks.Adds
test_chunked_muon_matches_plain_muon(forces tinyCHUNK_BYTES) to verify bitwise parity vs plain Muon for matrix + AdamW bias groups.Reviewed by Cursor Bugbot for commit 4091e72. Bugbot is set up for automated code reviews on this repo. Configure here.