Skip to content

fix(training): continue training past epoch boundary on skipped optimizer step - #15190

Open
nrenvo wants to merge 1 commit into
huggingface:mainfrom
nrenvo:fix/epoch-boundary-skipped-step-v2
Open

nrenvo wants to merge 1 commit into
huggingface:mainfrom
nrenvo:fix/epoch-boundary-skipped-step-v2

Conversation

@nrenvo

@nrenvo nrenvo commented Nov 5, 2026 •

Copy link
Copy Markdown

When AMP skips an optimizer step at the last batch of an epoch, global_step is not incremented and the for epoch in range loop exits before reaching max_train_steps.

Replace the for-loop with while global_step < args.max_train_steps so training always completes the requested number of steps regardless of where in an epoch the overflow happens.

Adds test_text_to_image_ema_skipped_step_at_epoch_boundary to pin this behaviour.

Splits out from #15173 as requested.

Fixes #15112

@github-actions

github-actions Bot commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

Hi @nrenvo, thanks for the PR! It does not appear to link an issue it fixes. If this PR addresses an existing issue, please add a closing keyword (e.g. Fixes #1234) to the PR description so the issue is linked. See the contribution guide for more details. If this PR intentionally does not fix a tracked issue, a maintainer can add the no-issue-needed label to silence this reminder.

Please note that PRs without a linked issue are likely to be automatically closed 10 days after this notice.

Once the PR links an issue (or gets the no-issue-needed label), you can ignore this message — it stays here as a comment, but it no longer applies.

@SeverinVisionary

Copy link
Copy Markdown

I ran test_text_to_image_ema_skipped_step_at_epoch_boundary on CPU at 68b9429. It failed with assert 2 == 3: the skipped second attempt advanced global_step, so training stopped early. The same test passed after applying #15189’s train_text_to_image.py changes. Could you base this PR on #15189 so the test runs with the guard in place?

AI-assisted testing and write-up.

…izer step

When AMP skips an optimizer step at the last batch of an epoch, the
static num_train_epochs ceiling causes the outer epoch loop to exit
with global_step < max_train_steps.

Replace the for-epoch loop with while global_step < max_train_steps
so training always completes the requested number of successful steps
regardless of where in an epoch the overflow happens.
@nrenvo
nrenvo force-pushed the fix/epoch-boundary-skipped-step-v2 branch from 68b9429 to b4f1cc8 Compare November 10, 2026 11:12
@github-actions github-actions Bot added fixes-issue size/S PR with diff < 50 LOC and removed size/M PR with diff < 200 LOC labels Nov 10, 2026

This branch has not been deployed

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Text-to-image example updates EMA when AMP skips the optimizer step

2 participants