Skip to content

Add gradient accumulation for training (single-GPU/DDP/FSDP2)#1

Closed
eldarkurtic wants to merge 1 commit into
mainfrom
add-gradient-accumulation
Closed

Add gradient accumulation for training (single-GPU/DDP/FSDP2)#1
eldarkurtic wants to merge 1 commit into
mainfrom
add-gradient-accumulation

Conversation

@eldarkurtic

Copy link
Copy Markdown
Owner

Summary

Adds a --gradient-accumulation-steps / TrainerConfig.gradient_accumulation_steps knob (default 1) that accumulates gradients over N microbatches before each optimizer step, growing the effective batch size (per-step batch × N) without extra per-microbatch memory. Matches the convention used by peer speculative-decoding frameworks (DeepSpec, TorchSpec, SpecForge): loss scaled by 1/N, optimizer + LR scheduler stepped once per accumulation window, counting in optimizer (macro) steps.

Behavior

  • Macro-step counting: global_step, --log-freq, --checkpoint-freq (when < 1), and the LR scheduler total all advance once per accumulation window (an accum=N run does 1/N the scheduler steps per epoch).
  • Drop remainder: trailing microbatches that don't fill a full window are dropped each epoch. _optimizer_steps_per_epoch is the single source of truth for this policy (used by the scheduler math, the __init__ guard, and the loop boundary).
  • _maybe_no_sync: on DDP, skips the gradient all-reduce on non-boundary microbatches and syncs the accumulated gradient on the boundary. Single-GPU (raw module) and FSDP2 (fully_shard, DTensor params) fall back to a no-op context and stay correct because gradients accumulate additively into .grad.
  • Guardrail: Trainer raises early if N exceeds the batches-per-epoch (which would otherwise run zero optimizer steps).
  • accum=1 is behavior-identical to the previous one-step-per-batch loop (regression-safe).

Verification

Numerically confirmed equivalent to the mean of per-microbatch gradients (accumulated grad vs. reference) on real hardware:

Path Launch cadence (accum=1 / 4) max abs grad diff
Single-GPU python 13 / 3 3.7e-9
DDP torchrun ×8 13 / 3 9.3e-9
FSDP2 (fully_shard) torchrun ×4 13 / 3 2.3e-9

All three exercise the real Trainer.train_epoch → _accumulate_and_step → _maybe_no_sync path. DDP/FSDP2 use different data per rank, so cross-rank averaging composes with accumulation.

Unit tests added: scheduler-step math (accum, remainder, accum=1 regression), numerical grad-equivalence on a toy model, _maybe_no_sync behavior (single-process DDP), the accum-too-large guard, and CLI parsing/rejection.

Test plan

  • python -m pytest tests/unit/train/test_gradient_accumulation.py tests/unit/train/test_trainer_scheduler.py tests/unit/train/test_cli_args.py — passing.
  • make quality (ruff + format + mypy) clean on changed files.

🤖 Generated with Claude Code

Introduce a `--gradient-accumulation-steps` / `TrainerConfig.gradient_accumulation_steps`
knob (default 1) that accumulates gradients over N microbatches before each
optimizer step, growing the effective batch size without extra per-microbatch
memory.

- Loss is scaled by 1/N; the optimizer, gradient clip, and LR scheduler run once
  per accumulation window (macro-step counting for global_step, log_freq,
  checkpoint cadence, and scheduler total steps).
- Trailing microbatches that don't fill a full window are dropped each epoch;
  `_optimizer_steps_per_epoch` is the single source of truth for this policy.
- `_maybe_no_sync` skips the DDP all-reduce on non-boundary microbatches; the
  single-GPU and FSDP2 paths fall back to a no-op context and stay correct
  because gradients accumulate additively into `.grad`.
- Trainer raises early when N exceeds the batches-per-epoch (would never step).
- accum=1 is behavior-identical to the previous one-step-per-batch loop.

Verified numerically equivalent to the mean of per-microbatch gradients on
single-GPU, 8-GPU DDP, and 4-GPU FSDP2 (fully_shard), plus unit tests for the
scheduler math, no_sync behavior, cadence, and CLI wiring.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Eldar Kurtic <8884008+eldarkurtic@users.noreply.github.com>
@eldarkurtic

Copy link
Copy Markdown
Owner Author

Superseded by the upstream PR vllm-project#859.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant