Set greater_is_better to False if metric_for_best_model ends with "loss" #31142
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
|
Hey! I was wondering if there were questions/concerns with this small refactor/fix? |
muellerzr
approved these changes
Jun 3, 2024
There was a problem hiding this comment.
Thanks! I think overall this change makes sense, since we need to be able to support any metric name.
cc @amyeroberts for final approval!
amyeroberts
approved these changes
Jun 3, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Prior to this PR greater_is_better (used for determining if the best checkpoint, for example) would be set to False if users didn't set it explicitly and either metric_for_best_model is unset or it's one of "loss" or "eval_loss".
This PR simplifies the logic to simply check if metric_for_best_model ends with loss.
Background: Despite having read the documentation a while back I forgot about this behavior and updated my script to evaluate against multiple datasets, for each one I compute the "eval_dataset-name_loss" metric. You can imagine my surprise that after 7h of training I realized that the worst model was saved and all work was lost :)
@ArthurZucker @muellerzr