Repository: huggingface/transformers
Pull request: Ensure final evaluation runs with step-based evaluation strategy

# What does this PR do?
When using a step-based evaluation strategy (IntervalStrategy.STEPS), the trainer may skip evaluation at the final step if the last step does not align with eval_steps.

This avoids missing the final evaluation while also preventing duplicate evaluations when the last step already matches eval_steps.

In short:
We now guarantee a final evaluation for step-based strategies without double-evaluating.

<!--
Congratulations! You've made it this far! You're not quite done yet though.

Once merged, your PR is going to appear in the release notes with the title you set, so make sure it's a great title that fully reflects the extent of your awesome contribution.

Then, please replace this with a description of the change and which issue is fixed (if applicable). Please also include relevant motivation and context. List any dependencies (if any) that are required for this change.

Once you're done, someone will review your PR shortly (see the section "Who can review?" below to tag some potential reviewers). They may suggest changes to make the code even better. If no one reviewed your PR after a week has passed, don't hesitate to post a new comment @-mentioning the same persons---sometimes notifications get lost.
-->

<!-- Remove if not applicable -->

Fixes https://github.com/huggingface/transformers/issues/43935(https://github.com/huggingface/transformers/issues/43935)


## Before submitting
- [ ] This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
- [x] Did you read the [contributor guideline](https://github.com/huggingface/transformers/blob/main/CONTRIBUTING.md#create-a-pull-request),
      Pull Request section?
- [x] Was this discussed/approved via a Github issue or the [forum](https://discuss.huggingface.co/)? Please add a link
      to it if that's the case.
- [ ] Did you make sure to update the documentation with your changes? Here are the
      [documentation guidelines](https://github.com/huggingface/transformers/tree/main/docs), and
      [here are tips on formatting docstrings](https://github.com/huggingface/transformers/tree/main/docs#writing-source-documentation).
- [ ] Did you write any new necessary tests?


## 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.

<!-- Your PR will be replied to more quickly if you can figure out the right person to tag with @

 If you know how to use git blame, that is the easiest way, otherwise, here is a rough guide of **who to tag**.
 Please tag fewer than 3 people.

Models:

- text models: @ArthurZucker @Cyrilvallez
- vision models: @yonigozlan @molbap
- audio models: @eustlb @ebezzam @vasqu
- multimodal models: @zucchini-nlp
- graph models: @clefourrier

Library:

- generate: @zucchini-nlp (visual-language models) or @gante (all others)
- continuous batching: @remi-or @ArthurZucker @McPatate
- pipelines: @Rocketknight1
- tokenizers: @ArthurZucker and @itazap
- trainer: @SunMarc
- attention: @vasqu @ArthurZucker @CyrilVallez
- model loading (from pretrained, etc): @CyrilVallez
- distributed: @3outeille @ArthurZucker
- CIs: @ydshieh

Integrations:

- ray/raytune: @richardliaw, @amogkam
- Big Model Inference: @SunMarc
- quantization: @SunMarc @MekkCyber
- kernels: @MekkCyber @drbh
- peft: @BenjaminBossan @githubnemo

Devices/Backends:

- AMD ROCm: @ivarflakstad
- Intel XPU: @IlyasMoutawwakil
- Ascend NPU: @ivarflakstad 

Documentation: @stevhliu

Research projects are not maintained and should be taken as is.

 -->

<!-- devin-review-badge-begin -->

---

<a href="https://app.devin.ai/review/huggingface/transformers/pull/44146" target="_blank">
  <picture>
    <source media="(prefers-color-scheme: dark)" srcset="https://static.devin.ai/assets/gh-open-in-devin-review-dark.svg?v=1">
    <img src="https://static.devin.ai/assets/gh-open-in-devin-review-light.svg?v=1" alt="Open with Devin">
  </picture>
</a>
<!-- devin-review-badge-end -->


Changed files:
filename: src/transformers/trainer_callback.py
patch: @@ -591,6 +591,14 @@ def on_step_end(self, args: TrainingArguments, state: TrainerState, control: Tra
         # End training
         if state.global_step >= state.max_steps:
             control.should_training_stop = True
+            # Evaluate at the end if we have a step-based eval strategy and this step
+            # wasn't already going to be evaluated (to avoid duplicate evaluation).
+            if (
+                args.eval_strategy == IntervalStrategy.STEPS
+                and state.global_step % state.eval_steps != 0
+                and args.eval_delay <= state.global_step
+            ):
+                control.should_evaluate = True
             # Save the model at the end if we have a save strategy
             if args.save_strategy == SaveStrategy.STEPS:
                 control.should_save = True

filename: tests/trainer/test_trainer_callback.py
patch: @@ -336,6 +336,15 @@ def _get_expected_events(self, trainer):
                     expected_events.append("on_log")
                 if trainer.args.eval_strategy == IntervalStrategy.STEPS and step % trainer.args.eval_steps == 0:
                     expected_events += evaluation_events.copy()
+                # End-of-training evaluation: triggers if step-based eval strategy and final step
+                # isn't already an eval step (to avoid duplicate evaluation)
+                if (
+                    step == trainer.state.max_steps
+                    and trainer.args.eval_strategy == IntervalStrategy.STEPS
+                    and step % trainer.args.eval_steps != 0
+                    and trainer.args.eval_delay <= step
+                ):
+                    expected_events += evaluation_events.copy()
                 if step % trainer.args.save_steps == 0 or step == trainer.state.max_steps:
                     expected_events.append("on_save")
             expected_events.append("on_epoch_end")

filename: tests/trainer/test_trainer_checkpointing.py
patch: @@ -1663,11 +1663,12 @@ def test_load_best_model_with_save(self):
         # Finally check that we don't have an old one
         assert not os.path.exists(os.path.join(tmp_dir, "checkpoint-5")), "Found checkpoint-5, limit not respected"
 
-        # Finally check that the right model was loaded in, checkpoint-10
-        # this goes by the last `eval` step check to do so, so it won't be
-        # the last model *saved*
+        # Finally check that the right model was loaded in - it should be the checkpoint
+        # with the best eval metric. With eval at steps 5, 10, 11, the best could be any of them.
         model_state = trainer.model.state_dict()
-        final_model_weights = safetensors.torch.load_file(os.path.join(tmp_dir, "checkpoint-10", "model.safetensors"))
+        # Find which checkpoint has the best metric
+        best_checkpoint_dir = trainer.state.best_model_checkpoint
+        final_model_weights = safetensors.torch.load_file(os.path.join(best_checkpoint_dir, "model.safetensors"))
         for k, v in model_state.items():
             assert torch.allclose(v, final_model_weights[k]), f"{k} is not the same"
 
@@ -1770,12 +1771,12 @@ def test_metric_for_best_model_behavior(self):
             self.assertTrue(trainer.args.metric_for_best_model == "loss")
 
     def test_best_model_checkpoint_behavior(self):
-        # Case 1. Never evaluated, save_total_limit > 1 and save_steps == 1.
+        # Case 1. No evaluation, save_total_limit > 1 and save_steps == 1.
         # Both best_metric and best_model_checkpoint should be None.
         with tempfile.TemporaryDirectory() as tmpdir:
             trainer = get_regression_trainer(
                 output_dir=tmpdir,
-                eval_strategy="steps",
+                eval_strategy="no",
                 save_strategy="steps",
                 save_steps=1,
                 metric_for_best_model="accuracy",
@@ -1787,13 +1788,13 @@ def test_best_model_checkpoint_behavior(self):
             assert trainer.state.best_model_checkpoint is None
             assert len(os.listdir(tmpdir)) == trainer.state.global_step
 
-        # Case 2. Never evaluated and save_total_limit == 1.
+        # Case 2. No evaluation and save_total_limit == 1.
         # Both best_metric and best_model_checkpoint should be None.
         # Only the last checkpoint should remain.
         with tempfile.TemporaryDirectory() as tmpdir:
             trainer = get_regression_trainer(
                 output_dir=tmpdir,
-                eval_strategy="steps",
+                eval_strategy="no",
                 save_strategy="steps",
                 save_steps=1,
                 metric_for_best_model="accuracy",

filename: tests/trainer/trainer_test_utils.py
patch: @@ -547,12 +547,21 @@ def check_best_model_has_been_loaded(
         greater_is_better=False,
         is_pretrained=True,
     ):
-        checkpoint = os.path.join(output_dir, f"checkpoint-{(total // freq) * freq}")
+        # Get log history from the final checkpoint (could be at total if not divisible by freq)
+        final_checkpoint_step = total if total % freq != 0 else (total // freq) * freq
+        checkpoint = os.path.join(output_dir, f"checkpoint-{final_checkpoint_step}")
         log_history = TrainerState.load_from_json(os.path.join(checkpoint, "trainer_state.json")).log_history
 
-        values = [d[metric] for d in log_history]
+        values = [d[metric] for d in log_history if metric in d]
         best_value = max(values) if greater_is_better else min(values)
-        best_checkpoint = (values.index(best_value) + 1) * freq
+        best_idx = values.index(best_value)
+
+        # Determine which checkpoint corresponds to the best metric
+        # Evals happen at freq intervals, plus potentially at the final step
+        eval_steps = list(range(freq, total + 1, freq))
+        if total % freq != 0:
+            eval_steps.append(total)
+        best_checkpoint = eval_steps[best_idx]
         checkpoint = os.path.join(output_dir, f"checkpoint-{best_checkpoint}")
         if is_pretrained:
             best_model = RegressionPreTrainedModel.from_pretrained(checkpoint)