Repository navigation
Conversation
…r state With optimizer-state CPU offload, step() moved every parameter's state to GPU at once, so peak memory still needed the full sharded optimizer state next to weights and gradients. For very large models this OOMs at the first optimizer step even when forward and backward fit. Add a step_chunk_bytes parameter (default 4 GiB, estimated as two fp32 moments per element) and step the parameters chunk by chunk with only that chunk's state on GPU. Each pass only exposes its chunk's gradients and the optimizer skips parameters without one, so every parameter is updated exactly once with the same element-wise math. step_chunk_bytes=None keeps the previous single-pass behavior. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ry rank Chunk sizes came from each rank's local shard and only parameters with a gradient, so uneven FSDP shards could give ranks different numbers of optimizer.step calls. Plan over all parameters with a per-rank estimate derived from the global shape and placements instead, and enable chunked stepping only for element-wise optimizers (AdamW, SignSGD); Muon runs collectives inside step and keeps a single pass. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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.
Reviewed by Cursor Bugbot for commit 11c8a0c. Configure here.
_planned_numel used ceil(numel / shards), which undercounts a tensor sharded on a dim that does not divide evenly: (3, 1000) on Shard(0) over 2 ranks holds 2000 elements on rank 0, not 1500. Round up per sharded dim instead, still from the global shape so every rank derives the same plan. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
garrett361
left a comment
There was a problem hiding this comment.
Can certainly see a use case for this (what was the model and setup where you found you needed this? would be good to have some measurements), but the implementation needs various fixes/improvements.
From the code, it looks like you intend to use this for AdamW and SignSGD, but only the former has any state (so it doesn't make sense to use for the latter). There's also some rank-aware chunking logic, which seems unnecessary since this only applies to AdamW which operates elementwise. The chunk size is also hard-coded and you can't turn it off as it. Also some probable cuda sync issues and other nits.
Claude generated below, for your clanker
The chunking is correct for AdamW: torch 2.13 AdamW skips parameters whose grad is None and returns early when a group has no grads, so stepping chunk by chunk gives the same result as one full step. Smaller fixes are in the inline comments; two larger points:
- The rank-identical chunk plan exists for Muon, which never chunks.
_step_chunkssizes chunks from global shapes so every rank makes the same number ofstepcalls, citing Muon's collectives (), butprime-rl/src/prime_rl/trainer/optim/state_offload.py
Lines 84 to 113 in ab15c01
setup_optimizerdisables chunking for Muon ( ) and AdamW'sstephas no collectives. Drop_planned_numel, the global-shape planning, and the two tests covering them (), or name the chunked optimizer that needs them. The check that a parameter without a gradient is left untouched is worth keeping.prime-rl/tests/unit/train/test_state_offload.py
Lines 134 to 171 in ab15c01
- Evidence. Please post peak allocated and peak reserved memory, and step time, before and after this change on the run that OOMed.
| """ | ||
|
|
||
| def __init__(self, optimizer: Optimizer, pin_memory: bool = True): | ||
| def __init__(self, optimizer: Optimizer, pin_memory: bool = True, step_chunk_bytes: int | None = 4 << 30): |
There was a problem hiding this comment.
Can this default to None, with the size coming from a config field that setup_optimizer passes through? 4 GiB is also hard-coded in optim/__init__.py:98, and with this default any caller that omits the argument gets chunking, Muon included.
| return CPUOffloadOptimizer(optimizer), None | ||
| # Chunked stepping calls optimizer.step once per chunk, which is only safe for element-wise | ||
| # optimizers; Muon issues collectives inside step and keeps a single pass. | ||
| step_chunk_bytes = 4 << 30 if config.type in ("adamw", "sign_sgd") else None |
There was a problem hiding this comment.
SignSGD keeps no optimizer state, so chunking it moves nothing and only adds step() calls. Chunk adamw only?
| for param in params: | ||
| param.grad = grads[id(param)] if id(param) in ids else None | ||
| self._move_states("cuda", chunk) | ||
| result = self.optimizer.step(closure) |
There was a problem hiding this comment.
Nit: nothing passes a closure today, but if something did, it would run once per chunk (loss and backward recomputed each time). Suggest assert closure is None on the chunked path.
| param.grad = grads[id(param)] if id(param) in ids else None | ||
| self._move_states("cuda", chunk) | ||
| result = self.optimizer.step(closure) | ||
| self._move_states("cpu", chunk) |
There was a problem hiding this comment.
_move_states("cpu", ...) ends with torch.cuda.synchronize(), so this is now one host sync per chunk instead of one per step. One sync after the last chunk should be enough, since the caching allocator is stream-ordered.
| # Upper bound on the optimizer state (estimated at two fp32 moments per element) brought | ||
| # to GPU per step pass. Moving all states at once needs the full sharded state on GPU next | ||
| # to weights and gradients, which can OOM for very large models. None steps everything in | ||
| # one pass. |
There was a problem hiding this comment.
Nit: this isn't strictly an upper bound. Chunks are built from whole parameters, so a parameter whose local shard alone exceeds step_chunk_bytes becomes its own chunk, above the limit.
|
|
||
|
|
||
| @pytest.mark.parametrize("step_chunk_bytes", [1, 64 * 128 * 8, None]) | ||
| def test_chunked_steps_match_plain_adamw(step_chunk_bytes: int | None) -> None: |
There was a problem hiding this comment.
These tests need CUDA, and the PR says they weren't run. Please run them and confirm they pass.

Summary
With optimizer-state CPU offload (
CPUOffloadOptimizer),step()moved every parameter's optimizer state to GPU at once. Peak GPU memory during the step therefore still included the full sharded optimizer state next to weights and gradients. On a very large model this OOMed at the first optimizer step even though forward and backward fit.This PR steps the parameters in chunks bounded by
step_chunk_bytesof optimizer state, so peak GPU memory during the step stays flat and the results are identical.Changes
CPUOffloadOptimizer.__init__gets astep_chunk_bytes: int | None = 4 << 30parameter. State size is estimated as two fp32 moments per local element.Nonekeeps the previous single-pass behavior._chunked_step: groups parameters that have gradients into chunks, and for each chunk exposes only that chunk's gradients, moves its state to GPU, callsoptimizer.step(), then moves the state back to CPU. The optimizer skips parameters without a gradient, so every parameter is updated exactly once with the same element-wise math. All gradients are restored afterwards, including on error._move_statestakes an optional parameter subset.test_chunked_steps_match_plain_adamwis parametrized over a tiny chunk size (one parameter per chunk), a mid-size chunk andNone. It checks that the parameters match plaintorch.optim.AdamWexactly after 3 steps, that states end up offloaded and that gradients are cleared.FullCPUOffloadOptimizer(full offload) is unchanged.Testing
ruff checkandruff format --checkon the changed files pass.tests/unit/train/test_state_offload.pyneeds CUDA and could not run on the CPU-only machine I used. Please run it on a GPU runner._move_statespatched to stay on CPU. It was bit-identical to plain AdamW for chunk sizes 1, 641288 andNoneover 3 steps.🤖 Generated with Claude Code
Note
Medium Risk
Changes distributed training optimizer stepping and memory behavior on large models; incorrect chunk planning or grad masking could desync ranks or alter updates, though tests target parity and rank agreement.
Overview
CPU-offloaded optimizer steps no longer pull the full optimizer state onto GPU at once, which was causing OOM on very large models during
step()even when forward/backward fit.CPUOffloadOptimizergainsstep_chunk_bytes(default 4 GiB of estimated state per pass). It chunks parameters, moves only that chunk’s state to GPU, masks gradients so eachoptimizer.step()updates one chunk, then offloads again—numerically matching a single full step for element-wise optimizers._move_statescan target a parameter subset; chunk sizing uses_planned_numelon globalDTensorshapes so all ranks issue the same number ofstepcalls (important for uneven shards and for optimizers like Muon that run collectives insidestep).The trainer wires
step_chunk_bytes=4<<30foradamwandsign_sgd, andNone(legacy single-pass) for other types such as Muon. Tests assert AdamW parity across chunk sizes, stable chunk plans when some params lack grads, and correct largest-shard sizing for unevenDTensorshards.Reviewed by Cursor Bugbot for commit ab15c01. Bugbot is set up for automated code reviews on this repo. Configure here.