Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions docs/cli/train.md
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,8 @@ torchrun --standalone --nproc_per_node=4 scripts/train.py \

- **`--lr`** (float, default: `1e-4`) Learning rate.

- **`--gradient-accumulation-steps`** (int, default: `1`) Accumulate gradients over N microbatches before each optimizer step, giving an effective batch size of `per-step batch × N` without extra per-microbatch memory. Must be ≥ 1. Notes: step-based counters (`global_step`, `--scheduler-total-steps`, `--log-freq`, and `--checkpoint-freq` when `< 1`) are counted in optimizer steps, so an accumulated run performs `1/N` as many scheduler steps per epoch; trailing microbatches that don't fill a full window are dropped each epoch; and the value must not change across a resume. On multi-GPU DDP, the gradient all-reduce is skipped on non-boundary microbatches (`no_sync`).

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the note about 'step-based counters' is a bit confusing. Is it trying to draw a distinction between 'optimizer steps' and 'scheduler steps'? In particular, it's not clear to me what 'counted in optimizer steps' means.

To be clear, I think this makes sense after reading the code, but that in isolation within the documentation it is a bit confusing.


- **`--train-data-ratio`** (float, default: `0.9`) Ratio of data to use for training, the rest of the provided data will be used for validation.

- **`--no-resume-from-checkpoint`** (flag) Disable automatic checkpoint resumption. Without this flag, this script will automatically load the latest checkpoint in `{save-path}` if one exists.
Expand Down
1 change: 1 addition & 0 deletions scripts/train.py
Original file line number Diff line number Diff line change
Expand Up @@ -675,6 +675,7 @@ def main(cfg: TrainConfig): # noqa: C901
hidden_states_dtype=hidden_states_dtype,
log_freq=args.log_freq,
fsdp_shard=args.fsdp_shard,
gradient_accumulation_steps=args.gradient_accumulation_steps,
)
trainer = Trainer(draft_model, trainer_config, train_loader, val_loader)

Expand Down
8 changes: 8 additions & 0 deletions src/speculators/train/config/schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -400,6 +400,14 @@ class TrainerArgs(_Group):
"parameters are fully replicated (DDP-like). Enable when the model does not "
"fit in a single GPU's memory.",
)
gradient_accumulation_steps: int = Field(
default=1,
ge=1,
description="Accumulate gradients over N microbatches before each optimizer "
"step (effective batch = per-step batch * N). The trailing partial window is "
"dropped each epoch; step-based counters (global_step, scheduler total, "
"log/checkpoint cadence) are measured in optimizer steps.",
)

@field_validator("checkpoint_freq")
@classmethod
Expand Down
157 changes: 133 additions & 24 deletions src/speculators/train/trainer.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import contextlib
import json
import logging
import time
Expand Down Expand Up @@ -98,6 +99,7 @@ class TrainerConfig(NamedTuple):
lr: float
num_epochs: int
save_path: str
gradient_accumulation_steps: int = 1
resume_from_checkpoint: bool = False
train_call_kwargs: dict | None = None
val_call_kwargs: dict | None = None
Expand All @@ -121,6 +123,16 @@ class TrainerConfig(NamedTuple):
max_steps: int | None = None


def _optimizer_steps_per_epoch(num_batches: int, accum: int) -> int:
"""Number of optimizer steps taken in one epoch under gradient accumulation.

Each optimizer step consumes ``accum`` microbatches; the trailing partial
window (``num_batches % accum`` microbatches) is dropped. Single source of
truth for the drop-remainder policy.
"""
return num_batches // accum
Comment thread
coderabbitai[bot] marked this conversation as resolved.


def _resolve_scheduler_steps(
config: TrainerConfig,
train_loader_len: int,
Expand All @@ -132,7 +144,10 @@ def _resolve_scheduler_steps(
default of 1% of the resolved total steps. ``scheduler_total_steps`` defaults
to ``num_epochs * train_loader_len``.
"""
default_total_steps = config.num_epochs * train_loader_len
steps_per_epoch = _optimizer_steps_per_epoch(
train_loader_len, config.gradient_accumulation_steps
)
default_total_steps = config.num_epochs * steps_per_epoch

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If a caller passes --scheduler-total-steps explicitly then scheduler_total_steps gets used as-is with no adjustment for accum - here the schedule will end up running accum times longer than intended relative to real optimizer steps, with no error or warning. Should an explicit scheduler_total_steps also get divided by accum, or at least raise/warn when accum > 1 and an explicit value is set?

scheduler_total_steps = (
config.scheduler_total_steps
if config.scheduler_total_steps is not None
Expand Down Expand Up @@ -173,6 +188,16 @@ def __init__(
self.rank = get_rank()
self.train_loader = train_loader
self.val_loader = val_loader

accum = config.gradient_accumulation_steps
if accum < 1:
raise ValueError(f"gradient_accumulation_steps must be >= 1, got {accum}.")
if _optimizer_steps_per_epoch(len(train_loader), accum) == 0:
raise ValueError(
f"gradient_accumulation_steps={accum} exceeds the number of "
f"batches per epoch ({len(train_loader)}); no optimizer step would "
"ever run. Lower gradient_accumulation_steps or add more data."
)
Comment thread
coderabbitai[bot] marked this conversation as resolved.
self.is_distributed = is_distributed()
self.resume_from_checkpoint = config.resume_from_checkpoint
acc = torch.accelerator.current_accelerator()
Expand All @@ -197,6 +222,7 @@ def _save_training_state(self, epoch: int, local_step: int) -> None:
"epoch": epoch,
"local_step": local_step,
"global_step": self.global_step,
"gradient_accumulation_steps": self.config.gradient_accumulation_steps,
}
p = self._training_state_path(epoch)
p.parent.mkdir(parents=True, exist_ok=True)
Expand All @@ -222,6 +248,16 @@ def setup_trainer(self):
# Check if this was a mid-epoch checkpoint — if so, resume
# from within that epoch rather than jumping to the next one.
state = self._load_training_state()
# Accumulation windows are aligned to the saved microbatch index, so
# resuming with a different accum would misalign them; reject it.
saved_accum = state.get("gradient_accumulation_steps", 1)
if state and saved_accum != self.config.gradient_accumulation_steps:
raise ValueError(
"Cannot resume: checkpoint was trained with "
f"gradient_accumulation_steps={saved_accum}, but this run "
f"uses {self.config.gradient_accumulation_steps}. "
"They must match to resume."
)
is_mid_epoch = (
state
and state.get("epoch") == self.checkpointer.previous_epoch
Expand Down Expand Up @@ -371,6 +407,76 @@ def make_scheduler(opt: torch.optim.Optimizer):
if self.resume_from_checkpoint and self.checkpointer.previous_epoch != -1:
self.checkpointer.load_scheduler_state_dict(self.schedulers)

def _maybe_no_sync(self, is_boundary: bool):
"""Skip DDP gradient all-reduce on non-boundary accumulation micro-steps.

Returns ``model.no_sync()`` only for a real ``DistributedDataParallel``
model on a non-boundary micro-step; otherwise a no-op context. Single-GPU
(raw module) and FSDP2 (``fully_shard``) fall through to ``nullcontext`` and
remain correct because gradients accumulate additively into ``.grad``.
"""
if isinstance(self.model, DistributedDataParallel) and not is_boundary:
return self.model.no_sync()

@shubhra shubhra Aug 11, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FSDP2 has its own no_sync equivalent, set_requires_gradient_sync(False). Was that considered for the non-boundary microbatches here? If it was skipped deliberately (so as to avoid holding the full unsharded gradient across the accumulation window), might be worth a short note in the docstring so it doesn't look like an oversight.

return contextlib.nullcontext()

def _accumulate_and_step(
self,
loss: torch.Tensor,
accum: int,
is_window_start: bool,
is_boundary: bool,
timer: "_StepTimer",
) -> dict[str, float]:
"""Run one accumulation micro-step; step the optimizer only at a boundary.

Gradients are zeroed at the start of each accumulation window and the loss is
scaled by ``accum`` so the accumulated gradient equals the mean over the
effective (window) batch. On a boundary micro-step the gradient is clipped,
the optimizer and schedulers step. LRs are captured before the scheduler step
to match the pre-step logging semantics. Returns those LRs by optimizer name.
"""
if is_window_start:
self._optimizers_zero_grad()
with self._maybe_no_sync(is_boundary):
(loss / accum).backward()

current_lrs = {
type(opt).__name__: opt.param_groups[0]["lr"] for opt in self.optimizers
}
if is_boundary:
torch.nn.utils.clip_grad_norm_(self.model.parameters(), 1.0)
timer.mark("bwd")
self._optimizers_step()
self._schedulers_step()
timer.mark("opt")
return current_lrs

def _maybe_save_step_checkpoint(
self,
epoch: int,
local_step: int,
accum: int,
opt_steps_per_epoch: int,
step_interval: int | None,
is_boundary: bool,
) -> None:
"""Save a mid-epoch checkpoint at the configured optimizer-step cadence.

Cadence is measured in optimizer steps so it aligns with accumulation-window
boundaries; saves too close to the end of the epoch are skipped. ``local_step``
(a microbatch index on a window boundary) is what gets persisted, so resume
lands on a fresh window start.
"""
if not is_boundary or step_interval is None or self.config.save_best:
return
opt_step = local_step // accum
if (
opt_step % step_interval == 0
and opt_steps_per_epoch - opt_step >= step_interval * MIN_STEP_PCT
# Avoid saving back to back at the end of each epoch
):
self.maybe_save_checkpoint(epoch, local_step=local_step)

def _optimizers_zero_grad(self):
for opt in self.optimizers:
opt.zero_grad()
Expand Down Expand Up @@ -432,17 +538,28 @@ def train_epoch(self, epoch: int):
if self.rank == 0:
train_loader = tqdm(train_loader, desc=f"Epoch {epoch}") # type: ignore[assignment]

accum = self.config.gradient_accumulation_steps
opt_steps_per_epoch = _optimizer_steps_per_epoch(num_steps, accum)
# Sub-epoch checkpoint cadence is measured in optimizer steps so it aligns
# with accumulation-window boundaries (microbatch counts would rarely match).
step_interval = (
max(1, round(num_steps * self.config.checkpoint_freq))
max(1, round(opt_steps_per_epoch * self.config.checkpoint_freq))
if self.config.checkpoint_freq < 1
else None
)
# Last microbatch index that completes a full accumulation window; any
# trailing microbatches that can't fill a window are dropped.
last_boundary = opt_steps_per_epoch * accum
t_before_fetch = time.perf_counter()
timer = _StepTimer()
for local_step_rel, batch in enumerate(train_loader, 1):
# local_step is 1-based index into the *full* epoch (not the slice).
local_step = local_step_rel + skip_steps
timer.reset(self.global_step % self.config.log_freq == 0)
if local_step > last_boundary:
break
is_window_start = (local_step - 1) % accum == 0
is_boundary = local_step % accum == 0
timer.reset(is_boundary and self.global_step % self.config.log_freq == 0)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When accum > 1, the timer is only enabled on the boundary microbatch, so the profiled step_ms captures roughly 1/accum of the real wall-clock time per optimizer step (one microbatch's forward/backward + clip + optimizer/scheduler, but not the preceding accum - 1 microbatches). Similarly, tokens_per_s reflects per-microbatch throughput rather than effective per-optimizer-step throughput. Since global_step counts optimizer steps, this creates a mismatch: a user comparing step_ms across runs with different accum values would see similar numbers despite very different actual step durations.

Is this intentional (profiling the per-microbatch pipeline by design)? If so, consider including gradient_accumulation_steps in the logged dict so users can derive the per-window metrics --- e.g. adding "gradient_accumulation_steps": accum to the metric_logger.info(...) payload when accum > 1.


timer.mark_value("start", t_before_fetch)
gpu_batch = {
Expand All @@ -461,18 +578,9 @@ def train_epoch(self, epoch: int):
)

timer.mark("fwd")
self._optimizers_zero_grad()
loss.backward()
torch.nn.utils.clip_grad_norm_(self.model.parameters(), 1.0)

timer.mark("bwd")
self._optimizers_step()

current_lrs = {
type(opt).__name__: opt.param_groups[0]["lr"] for opt in self.optimizers
}
self._schedulers_step()
timer.mark("opt")
current_lrs = self._accumulate_and_step(
loss, accum, is_window_start, is_boundary, timer
)
t_before_fetch = timer.now() or time.perf_counter()

profile = None
Expand Down Expand Up @@ -501,22 +609,23 @@ def train_epoch(self, epoch: int):
},
extra={"step": self.global_step},
)
self.global_step += 1
if is_boundary:
self.global_step += 1

if (
self.config.max_steps is not None
and self.global_step >= self.config.max_steps
):
break

if (
step_interval is not None
and not self.config.save_best
and local_step % step_interval == 0
and num_steps - local_step >= step_interval * MIN_STEP_PCT
# Avoid saving back to back ay the end of each epoch
):
self.maybe_save_checkpoint(epoch, local_step=local_step)
self._maybe_save_step_checkpoint(
epoch,
local_step,
accum,
opt_steps_per_epoch,
step_interval,
is_boundary,
)

@torch.no_grad()
def val_epoch(self, epoch: int) -> dict[str, float] | None:
Expand Down
22 changes: 22 additions & 0 deletions tests/unit/train/test_cli_args.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

import argparse

import pytest

from speculators.models.dflash.core import DFlashDraftModel
from speculators.models.dspark.core import DSparkDraftModel
from speculators.models.eagle3.core import Eagle3DraftModel
Expand Down Expand Up @@ -169,3 +171,23 @@ def test_no_norm_before_fc_flag(monkeypatch):
def test_no_norm_output_flag(monkeypatch):
args = _parse(monkeypatch, ["--no-norm-output"])
assert args.norm_output is False


# ---------------------------------------------------------------------------
# Gradient accumulation
# ---------------------------------------------------------------------------


def test_gradient_accumulation_steps_default(monkeypatch):
args = _parse(monkeypatch, [])
assert args.gradient_accumulation_steps == 1


def test_gradient_accumulation_steps_explicit(monkeypatch):
args = _parse(monkeypatch, ["--gradient-accumulation-steps", "8"])
assert args.gradient_accumulation_steps == 8


def test_gradient_accumulation_steps_rejects_zero(monkeypatch):
with pytest.raises(SystemExit):
_parse(monkeypatch, ["--gradient-accumulation-steps", "0"])
Loading
Loading