Repository navigation
perf(optim): overlap optimizer state offload copies with the AdamW step - #4008
faresobeid wants to merge 11 commits into
Conversation
# Conflicts: # tests/unit/train/test_state_offload.py
|
Measured on GPU: Qwen3-30B-A3B on 8×H200, FSDP 8 / EP 8, seq 16384, full AC, compile, 25 steps, mean tok/s over the last 10. Losses agree across arms (≈1.30–1.31 at step 25).
Even with this PR, optimizer offload is still ~44% below no-offload (step ≈ 4.9–5.5 s vs 2.26 s). A follow-up investigation of where the rest goes (copies, CPU step, NUMA/PCIe bandwidth, overlap) is underway. Any improvement will go on a separate branch on top of this PR. |
garrett361
left a comment
There was a problem hiding this comment.
Looks ok to me, but maybe wait on Sami as there's apparently a history here that I don't know about.
Inline comments are mostly agent-written. Generated below, for your agent:
Approve with comments. With model.offload = "optimizer" and AdamW, every step after the first now streams states through two GPU staging buffers chunk by chunk (H2D, update, D2H on three streams) and keeps the step counters on CPU, instead of moving every state to the GPU and back on the compute stream.
- The multi-chunk path has no test coverage: every test model fits in one chunk.
CHUNK_BYTESis 1 GiB and the test model is a few hundred KiB, so CI never runs chunk boundaries, staging-buffer offsets, or the reuse of a buffer slot by chunki+2.Fix: monkeypatchprime-rl/src/prime_rl/trainer/optim/state_offload.py
Lines 149 to 176 in 1a8052f
state_offload.CHUNK_BYTESto0in thetrained_pairfixture, which puts each of the model's four parameters in its own chunk and runs the existing bitwise parity, pinned-buffer and round-trip tests through that path. No new test is needed.prime-rl/tests/unit/train/test_state_offload.py
Lines 69 to 79 in 1a8052f
| def _local(value: torch.Tensor) -> torch.Tensor: | ||
| return value._local_tensor if isinstance(value, DTensor) else value | ||
|
|
||
|
|
||
| def _with_local(value: torch.Tensor, local: torch.Tensor) -> torch.Tensor: | ||
| if not isinstance(value, DTensor): | ||
| return local | ||
| new_dtensor = copy.copy(value) | ||
| new_dtensor._local_tensor = local | ||
| return new_dtensor |
There was a problem hiding this comment.
Optional: _move_states still spells out this DTensor unwrap and rewrap inline, so using these helpers there too would write it once.
prime-rl/src/prime_rl/trainer/optim/state_offload.py
Lines 55 to 68 in 1a8052f
local = _local(value)
new_local = self._offload_to_cpu(param, key, local) if device == "cpu" else local.to(device, non_blocking=True)
state[key] = _with_local(value, new_local)…erlap Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…se the DTensor local helpers in _move_states Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Overlaps the optimizer-state CPU offload copies with the AdamW step (
model.offload = "optimizer",CPUOffloadOptimizer).On main, the step moves all AdamW states to the GPU, steps, and moves them back, one direction at a time on the compute stream, and the GPU holds every state at once.
Change
stepcounters stay on CPU, as in plain torch AdamW, so there is no per-parameter device sync.CPUOffloadOptimizerno longer takespin_memory(its only caller usedTrue).Numbers (H200)
DeepSeek-V4-Flash (1 node, 8 layers, 65k, cp2 ep8): 17.0-18.1 s -> 16.6-17.7 s. Entropy and mismatch KL identical per step.
Validation
tests/unit/train/test_state_offload.py(includes a parameter that gets its first grad on step 2). The parity, pinned-buffer and round-trip tests setCHUNK_BYTES = 0, so each parameter is its own chunk and they cross chunk boundaries and reuse both staging slots; all pass on 1 H200.pytest -m "not gpu" tests/unit/orchestrator tests/unit/test_configs.py tests/unit/train,ruff check,ruff format --check,uv lock --checkon the branch merged with current main.🤖 Generated with Claude Code
Note
Medium Risk
Changes the hot-path AdamW training step with multi-stream memory management and param-group mutation during chunked steps; correctness is covered by GPU tests but regressions could affect peak memory or numerical parity under FSDP/DTensor.
Overview
AdamW optimizer steps through
CPUOffloadOptimizernow use a chunked, stream-overlapped path instead of moving all state to GPU, stepping, then moving everything back on the compute stream.Whole parameters are grouped into ~1 GiB chunks with aligned views in two GPU staging buffers. While chunk i runs on the default stream, the next chunk H2D loads on one side stream and the previous chunk D2H writes to pinned CPU on another, shrinking peak GPU state during the step and overlapping PCIe traffic with compute. AdamW
stepscalars stay on CPU; DTensor handling is centralized in_local/_with_local. The legacy full-move path remains for the first step, Muon/non-AdamW, and any step where a parameter has a grad but no optimizer state yet. The wrapper drops thepin_memoryconstructor flag (pinned CPU buffers are always used).Unit tests force one-parameter-per-chunk via
CHUNK_BYTES=0and add coverage for parameters that only receive gradients after the first step.Reviewed by Cursor Bugbot for commit bfb7dc4. Bugbot is set up for automated code reviews on this repo. Configure here.