-
Notifications
You must be signed in to change notification settings - Fork 3.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Disable {save,check}_on_train_epoch_end
with check_val_every_n_epoch>1
#9156
Conversation
{save,check}_on_train_epoch_end
with check_val_every_n_epoch
{save,check}_on_train_epoch_end
with check_val_every_n_epoch>1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@carmocca do you think it'd be better to have the user explicitly opt into this without providing a default?
Codecov Report
@@ Coverage Diff @@
## master #9156 +/- ##
======================================
Coverage 92% 92%
======================================
Files 176 178 +2
Lines 14804 14850 +46
======================================
+ Hits 13651 13701 +50
+ Misses 1153 1149 -4 |
No, the problem is that users don't notice something is wrong until the epoch is over and a callback tries to get a non-logged metric key. We could print a warning before the epoch starts asking to set this flag when |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM !
What does this PR do?
Fixes #9151, #9163
Does your PR introduce any breaking changes? If yes, please list them.
None
Before submitting
PR review