Fix/20256 early stopping reset#22358
Conversation
…ween runs When reusing an EarlyStopping callback across multiple model.fit() calls (e.g., in a for loop training different models), self.best was not reset in on_train_begin(). This caused subsequent runs to compare against the best metric from a previous run, triggering premature early stopping. Reset self.best to None in EarlyStopping.on_train_begin() and ReduceLROnPlateau._reset() so the first epoch of each new training run is always accepted as an improvement, matching the behavior of a fresh callback instance. Fixes keras-team#20256
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request resolves a critical issue where Keras callbacks like Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request correctly fixes a state reset issue in EarlyStopping and ReduceLROnPlateau callbacks when they are reused, by ensuring the best monitored value is reset at the start of each training run. The included regression test for EarlyStopping is a valuable addition. I've added a couple of suggestions to improve code consistency and documentation clarity.
| # Allow instances to be re-used | ||
| self.wait = 0 | ||
| self.stopped_epoch = 0 | ||
| self.best = None |
There was a problem hiding this comment.
While this change is correct, for consistency with other callbacks like ReduceLROnPlateau, consider extracting the state-resetting logic from on_train_begin (lines 89-93) into a private _reset() method. This would align with the Keras API design guideline of having similar APIs for objects that perform similar functions, improving code clarity and maintainability.
For example:
def on_train_begin(self, logs=None):
self._reset()
def _reset(self):
# Allow instances to be re-used
self.wait = 0
self.stopped_epoch = 0
self.best = None
self.best_weights = None
self.best_epoch = 0References
- Objects that do interchangeable things should have identical or very close APIs. Both
EarlyStoppingandReduceLROnPlateauare callbacks that monitor metrics and reset state. Using a_resetmethod in both would improve consistency. (link)
| @@ -74,6 +74,7 @@ def __init__( | |||
|
|
|||
| def _reset(self): | |||
| """Resets wait counter and cooldown counter.""" | |||
There was a problem hiding this comment.
With the addition of resetting self.best, the docstring for the _reset method is now incomplete. Please update it to reflect that best is also reset.
| """Resets wait counter and cooldown counter.""" | |
| """Resets wait counter, cooldown counter, and best value.""" |
References
- Docstrings should be kept up-to-date and accurately describe what the code does. The docstring for
_resetshould be updated to include the fact thatself.bestis also being reset. (link)
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #22358 +/- ##
=======================================
Coverage 82.95% 82.95%
=======================================
Files 595 595
Lines 66040 66042 +2
Branches 10305 10305
=======================================
+ Hits 54785 54787 +2
Misses 8639 8639
Partials 2616 2616
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
3 participants
This pull request addresses issues related to the reuse of callback instances in Keras, specifically ensuring that stateful attributes are properly reset when callbacks like
EarlyStoppingandReduceLROnPlateauare used multiple times. It also adds a regression test to prevent future issues with callback reuse across different models.Callback state reset improvements:
keras/src/callbacks/early_stopping.py: Ensuresself.bestis reset toNoneinon_train_beginto avoid comparing against stale best values when reusing the sameEarlyStoppinginstance across multiple training runs.keras/src/callbacks/reduce_lr_on_plateau.py: Resetsself.besttoNonein the_resetmethod, ensuring correct behavior when theReduceLROnPlateaucallback is reused.Testing and regression coverage:
keras/src/callbacks/early_stopping_test.py: Adds a regression test to verify thatEarlyStoppingcan be reused across multiple models without retaining stale state, addressing a previously reported issue.Fixes: Early-stopping does not work properly in Keras 3 when used in a for loop #20256Problem
When the same
EarlyStopping(orReduceLROnPlateau) callback instance is reused across multiplemodel.fit()calls (a common pattern in hyperparameter loops),self.bestis never cleared in_reset(). This means the secondfit()starts with abestvalue from the previous run: if the first model had a very low loss, the second model may stop immediately on epoch 1 simply because its loss hasn't beaten a stale threshold from an entirely different model.Root Cause
EarlyStopping._reset()andReduceLROnPlateau._reset()resetwait,stopped_epoch, andbest_weights, but forgot to resetbesttoNone. TheMonitorCallbackbase initializesbestin__init__, but_reset()is what runs at the start of eachfit()— so after the first run,bestis permanently stale.Fix
Add
self.best = Noneto bothEarlyStopping._reset()andReduceLROnPlateau._reset(). ANonesentinel is already handled by the comparison logic in both callbacks (it forces acceptance of the first epoch's value).Files Changed
keras/src/callbacks/early_stopping.py— addself.best = Nonein_reset()keras/src/callbacks/reduce_lr_on_plateau.py— addself.best = Nonein_reset()keras/src/callbacks/early_stopping_test.py— regression test: shared callback reused across 3 models in a loop, each run must last > patience epochs