-
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
Fix saved filename in ModelCheckpoint if it already exists #4861
Conversation
Hello @rohitgr7! Thanks for updating this PR. There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻 Comment last updated at 2020-12-14 17:27:54 UTC |
Codecov Report
@@ Coverage Diff @@
## master #4861 +/- ##
======================================
- Coverage 93% 93% -0%
======================================
Files 134 134
Lines 9907 9905 -2
======================================
- Hits 9206 9204 -2
Misses 701 701 |
11952d6
to
9464b16
Compare
Thanks @carmocca for the review :) |
Unrelated to the PR but, is there a reason for version starting at 0?
instead of:
|
there is no special reason :] |
8687939
to
7485092
Compare
@carmocca wanna give this a shot? #4861 (comment) |
Sure, I will. Most likely this weekend |
* disable version if not required * disable version if not required * pep * chlog * improve test * improve test * parametrize test and update del_list * Update pytorch_lightning/callbacks/model_checkpoint.py Co-authored-by: Carlos Mocholí <[email protected]> * try appending version to already saved ckpt_file * Revert "try appending version to already saved ckpt_file" This reverts commit 710e05e. * add more assertions * use BoringModel Co-authored-by: Carlos Mocholí <[email protected]> Co-authored-by: chaton <[email protected]> Co-authored-by: Roger Shieh <[email protected]>
* disable version if not required * disable version if not required * pep * chlog * improve test * improve test * parametrize test and update del_list * Update pytorch_lightning/callbacks/model_checkpoint.py Co-authored-by: Carlos Mocholí <[email protected]> * try appending version to already saved ckpt_file * Revert "try appending version to already saved ckpt_file" This reverts commit 710e05e. * add more assertions * use BoringModel Co-authored-by: Carlos Mocholí <[email protected]> Co-authored-by: chaton <[email protected]> Co-authored-by: Roger Shieh <[email protected]>
* disable version if not required * disable version if not required * pep * chlog * improve test * improve test * parametrize test and update del_list * Update pytorch_lightning/callbacks/model_checkpoint.py Co-authored-by: Carlos Mocholí <[email protected]> * try appending version to already saved ckpt_file * Revert "try appending version to already saved ckpt_file" This reverts commit 710e05e. * add more assertions * use BoringModel Co-authored-by: Carlos Mocholí <[email protected]> Co-authored-by: chaton <[email protected]> Co-authored-by: Roger Shieh <[email protected]>
* disable version if not required * disable version if not required * pep * chlog * improve test * improve test * parametrize test and update del_list * Update pytorch_lightning/callbacks/model_checkpoint.py Co-authored-by: Carlos Mocholí <[email protected]> * try appending version to already saved ckpt_file * Revert "try appending version to already saved ckpt_file" This reverts commit 710e05e. * add more assertions * use BoringModel Co-authored-by: Carlos Mocholí <[email protected]> Co-authored-by: chaton <[email protected]> Co-authored-by: Roger Shieh <[email protected]>
* disable version if not required * disable version if not required * pep * chlog * improve test * improve test * parametrize test and update del_list * Update pytorch_lightning/callbacks/model_checkpoint.py Co-authored-by: Carlos Mocholí <[email protected]> * try appending version to already saved ckpt_file * Revert "try appending version to already saved ckpt_file" This reverts commit 710e05e. * add more assertions * use BoringModel Co-authored-by: Carlos Mocholí <[email protected]> Co-authored-by: chaton <[email protected]> Co-authored-by: Roger Shieh <[email protected]>
* disable version if not required * disable version if not required * pep * chlog * improve test * improve test * parametrize test and update del_list * Update pytorch_lightning/callbacks/model_checkpoint.py Co-authored-by: Carlos Mocholí <[email protected]> * try appending version to already saved ckpt_file * Revert "try appending version to already saved ckpt_file" This reverts commit 710e05e. * add more assertions * use BoringModel Co-authored-by: Carlos Mocholí <[email protected]> Co-authored-by: chaton <[email protected]> Co-authored-by: Roger Shieh <[email protected]>
What does this PR do?
Fixes the saved filename if it's already present. It will override the file if it's already going to be deleted. Previously if
save_top_k=1
and filename is static for eg:some_file
, it will create a file with the version for eg: filename=some_file-v2.ckpt
ifmax_epoch > 1
but it should besome_file.ckpt
only.The test fails on master.
Before submitting
PR review
Anyone in the community is free to review the PR once the tests have passed.
Before you start reviewing make sure you have read Review guidelines. In short, see the following bullet-list:
Did you have fun?
Make sure you had fun coding 🙃