Skip to content

Fix epoch indexing for experimental schedules - #6811

Open
guptaishaan wants to merge 1 commit into
huggingface:mainfrom
guptaishaan:fix-6806
Open

Fix epoch indexing for experimental schedules#6811
guptaishaan wants to merge 1 commit into
huggingface:mainfrom
guptaishaan:fix-6806

Conversation

@guptaishaan

@guptaishaan guptaishaan commented Aug 19, 2026

Copy link
Copy Markdown

What does this PR do?

The schedule properties indexed Python lists with TrainerState.epoch, which is None before training and a float during training. This converts the value to an integer epoch, defaulting to epoch zero, in OnlineDPO, XPO, and Nash-MD.

Regression tests cover pre-training access, fractional epochs, epoch boundaries, and schedules shorter than the training run. The three affected test modules passed on Linux using CPU execution, with 41 passed and 13 skipped. Ruff lint and format checks passed.

GPU execution was not verified because the installed PyTorch CUDA runtime required a newer driver. The combined pre-commit entry point could not run because the node's Git lacks git ls-files --deduplicate; its configured Ruff checks passed directly.

Thanks to @mmjerge for reporting the issue and identifying the affected trainers.

Fixes #6806

Before submitting

  • This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
  • Did you read the contributor guideline, Pull Request section?
  • Was this discussed/approved via a GitHub issue? Please add a link to it if that's the case.
  • Did you make sure to update the documentation with your changes?
  • Did you write any new necessary tests?

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.


Note

Low Risk
Narrow bugfix to epoch indexing for optional list schedules in three experimental trainers; behavior change is intentional (fixes crashes/wrong indices) with regression tests.

Overview
Fixes list-based hyperparameter schedules in Online DPO, XPO, and Nash-MD when TrainerState.epoch is None (before training) or a float during training.

Schedule properties (beta, alpha, mixture_coef) now use int(self.state.epoch or 0) instead of indexing with self.state.epoch directly, so pre-training access uses epoch 0, fractional epochs truncate to the current integer epoch, and out-of-range epochs still clamp to the last schedule entry.

Adds parametrized unit tests in the three experimental trainer test modules for None, fractional, and boundary epoch values.

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

Convert TrainerState.epoch to an integer before indexing list-valued
beta, alpha, and mixture coefficient schedules. Treat an unset epoch as
epoch zero so the properties are also safe before training starts.

Add regression coverage for all three experimental trainers.
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.

OnlineDPO/XPO/Nash-MD: list-valued beta/alpha/mixture_coef epoch schedules crash (list indexed with float state.epoch)

1 participant