-
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
Added changeable extension variable for model checkpoints #4977
Conversation
Hello @janhenriklambrechts! Thanks for updating this PR.
Comment last updated at 2020-12-06 12:39:27 UTC |
Codecov Report
@@ Coverage Diff @@
## master #4977 +/- ##
======================================
Coverage 93% 93%
======================================
Files 129 129
Lines 9371 9372 +1
======================================
+ Hits 8688 8689 +1
Misses 683 683 |
@janhenriklambrechts can you add a test for this? |
@rohitgr7 sure, Ill write a test |
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.
I think that the .ckpt
is used also in other places...
@@ -140,6 +140,7 @@ class ModelCheckpoint(Callback): | |||
|
|||
CHECKPOINT_JOIN_CHAR = "-" | |||
CHECKPOINT_NAME_LAST = "last" | |||
FILE_EXTENSION = "ckpt" |
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.
FILE_EXTENSION = "ckpt" | |
FILE_EXTENSION = ".ckpt" |
lets keep the .
as os.path.extsplit(...)
holds it too
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
…nstead of epoch for integration test
* Added changeable extension variable for model checkpoints * Removed whitespace * Removed the last bit of whitespace * Wrote tests for FILE_EXTENSION * Fixed formatting issues * More formatting issues * Simplify test by just using defaults * Formatting to PEP8 * Added dummy class that inherits ModelCheckpoint; run only one batch instead of epoch for integration test * Fixed too much whitespace formatting * some changes Co-authored-by: rohitgr7 <rohitgr1998@gmail.com>
What does this PR do?
This PR adds a variable
FILE_EXTENSION
toModelCheckpoint
that allows the user to change the file extension of their model checkpoints as per #4963Before 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 🙃