model_checkpoint to save all models (where validation occurs) #1160
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1160 +/- ##
======================================
Coverage 93% 93%
======================================
Files 61 61
Lines 2670 2671 +1
======================================
+ Hits 2470 2471 +1
Misses 200 200 |
|
this failing shall be fixed in #1163 |
|
@jamesjjcondon could you pls update from master? we have fixed the failier in #1163 |
There was a problem hiding this comment.
hey @jamesjjcondon thanks for your contribution! just one question
| @@ -77,7 +77,7 @@ def __init__(self, filepath, monitor: str = 'val_loss', verbose: bool = False, | |||
| save_top_k: int = 1, save_weights_only: bool = False, | |||
| mode: str = 'auto', period: int = 1, prefix: str = ''): | |||
| super().__init__() | |||
| if save_top_k and os.path.isdir(filepath) and len(os.listdir(filepath)) > 0: | |||
| if save_top_k > 0 and os.path.isdir(filepath) and len(os.listdir(filepath)) > 0: | |||
There was a problem hiding this comment.
what's the motivation for dropping the delete warning if save_top_k == -1?
There was a problem hiding this comment.
Looks to me that we only call _del_model when save_top_k > 0 (lines 225, 226), so I think this is fine currently. We could clear it once if we're saving every epoch though, not sure if that's preferable.
|
Seems I can't push to this PR (permission denied) to merge in master. @jamesjjcondon If you could merge it in that would be great. |
|
@jamesjjcondon how is it going? could we get this done for next release next week? |
|
Finishing in #1359 |
Before submitting
What does this PR do?
Improves some use-cases where saving all models is desired. See #596 .
Removes deleting of models on existing save filepath and drops delete warning if save_top_k == -1.
Enables saving of all epochs where validation occurs.
Does not address saving all epochs regardless of check_val_every_n_epochs > 1
Not tested with NVIDIA/apex
PR review
Anyone in the community is free to review the PR once the tests have passed.
If we didn't discuss your PR in Github issues there's a high chance it will not be merged.
Did you have fun?
Yes, but 2nd ever contribution (be gentle : )