ENH: early stopping in models & example with RUMnet - #294
Conversation
Summary of ChangesHello @VincentAuriau, 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 enhances the model training capabilities by integrating an early stopping mechanism into the base model, which helps prevent overfitting and optimizes training duration. It also refines the logging of validation metrics for clarity and provides a practical example of early stopping in action within the RUMnet model's notebook. Highlights
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
|
There was a problem hiding this comment.
Code Review
This pull request introduces early stopping functionality to the base model and adds a corresponding example in the RUMnet notebook. The implementation in base_model.py correctly uses the self.stop_training flag set by Keras callbacks.
My review identifies a couple of issues:
- There's a bug where the early stopping check is missing for the training path where
sample_weightis not used, which would cause early stopping to fail. - The messages printed when early stopping is triggered are unconditional and should respect the
verboseparameter.
The notebook changes are good, with a clear example of how to use the new early stopping feature and a fix to use val_loss consistently. After addressing the feedback, this will be a solid enhancement.
| if self.stop_training: | ||
| print("Training stopped with early stopping taking effect") | ||
| break |
There was a problem hiding this comment.
This early stopping check is great, but it's missing from the training loop in the else block below (when sample_weight is None), which will prevent early stopping from working in that case. Please add a similar check there.
Also, this print statement is unconditional. It should only be executed if verbose > 0 to avoid unexpected output when verbose is set to 0.
| if self.stop_training: | |
| print("Training stopped with early stopping taking effect") | |
| break | |
| if self.stop_training: | |
| if verbose > 0: | |
| print("Training stopped with early stopping taking effect") | |
| break |
| if self.stop_training: | ||
| print("Early Stopping taking effect") | ||
| print("Training stopped with early stopping taking effect") | ||
| break |
There was a problem hiding this comment.
Similar to the check within the batch loop, this print statement is unconditional. It should be wrapped in an if verbose > 0: check to respect the verbosity level set for the fit method.
| if self.stop_training: | |
| print("Early Stopping taking effect") | |
| print("Training stopped with early stopping taking effect") | |
| break | |
| if self.stop_training: | |
| if verbose > 0: | |
| print("Training stopped with early stopping taking effect") | |
| break |
Coverage Report for Python 3.10
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Coverage Report for Python 3.11
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Coverage Report for Python 3.12
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
No description provided.