Skip to content

Fix async trainers resuming past the epoch target - #7036

Open
DaoyuanLi2816 wants to merge 1 commit into
huggingface:mainfrom
DaoyuanLi2816:fix/async-resume-epoch-stop
Open

Fix async trainers resuming past the epoch target#7036
DaoyuanLi2816 wants to merge 1 commit into
huggingface:mainfrom
DaoyuanLi2816:fix/async-resume-epoch-stop

Conversation

@DaoyuanLi2816

@DaoyuanLi2816 DaoyuanLi2816 commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes #7035.

When an async trainer resumes from a checkpoint whose saved prompt position already reaches the configured epoch target, the epoch-stop callback currently runs only after the first optimizer step. This PR applies the follow-up recommended in #6823 (comment) to both async trainers:

  • check restored prompt/group progress at train begin;
  • run that check before cold weight sync and rollout-worker startup;
  • avoid entering the first epoch batch when training is already stopped;
  • keep the existing post-step target check for normal training.

Regression coverage verifies that both AsyncGRPOTrainer and AsyncDistillationTrainer stop before delegating to the base epoch loop when checkpoint progress already meets the target.

Validation:

  • python -m pytest tests/experimental/test_async_grpo_trainer.py::TestEpochStopOnResume tests/experimental/test_async_grpo_trainer.py::TestRolloutStateCheckpoint tests/experimental/test_async_distillation_trainer.py::TestEpochStop tests/experimental/test_async_distillation_trainer.py::TestRolloutStateCheckpoint -q --no-cov — 16 passed
  • pre-commit run ruff-check on the four changed files — passed
  • pre-commit run ruff-format on the four changed files — passed
  • git diff --check — passed

The existing GPU end-to-end GRPO epoch test was not runnable in this local environment because Transformers requires the optional compatible kernels package for its Flash Attention implementation.

Before submitting

AI writing disclosure

  • No AI usage: the PR was written entirely by a human.
  • AI-assisted: some parts were suggested or improved by AI, but the PR was written and reviewed by a human.
  • AI-generated: the PR was mostly or fully generated by an AI tool.

Who can review?

Anyone in the community is free to review the PR once the tests have passed. Feel free to tag members/contributors who may be interested in your PR.


Note

Medium Risk
Touches training-loop control flow and rollout-worker startup on resume for both async trainers; behavior change is narrow (already-at-target checkpoints) but affects when vLLM sync and rollouts run.

Overview
Fixes async GRPO and distillation trainers continuing (and starting the rollout worker) after resume when checkpoint progress already satisfies num_train_epochs.

_EpochStopCallback now runs the same target check at on_train_begin as at step end (via shared _check_target), using resumed _groups_before_resume / _prompts_before_resume. The callback is registered before _TrainBeginCallback so the stop flag is set before cold weight sync and worker startup; _TrainBeginCallback no-ops when should_training_stop is already true. _run_epoch returns immediately without entering the first batch, because the base trainer only reads the stop flag after the epoch loop returns.

Regression tests assert both trainers set should_training_stop at train begin and never delegate to the base _run_epoch when resume count already equals the epoch target.

Reviewed by Cursor Bugbot for commit 6be8d40. Bugbot is set up for automated code reviews on this repo. Configure here.

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.

Async trainers take an extra optimizer step when resuming at the epoch target

1 participant