-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
Checkpoint naming broken #775
Comments
that’s already possible. https://pytorch-lightning.readthedocs.io/en/latest/callbacks.html.
but as you mentioned, if that’s broken then it’s better to submit a PR to fix? :) |
@neelr11 did this work for you? is this still a bug? |
@williamFalcon this is still a bug. I can submit a PR this weekend. |
I suggest following pytorch-ignite and just add the monitored name and value to the saved checkpoint. |
@neelr11 are you still interested in sending a PR? |
As neerlr11 mentioned, 0.6.0-modelcallback didn't work as documented: supporting dynamic checkpoint string like |
🐛 Bug
I would like to be able to save checkpoints with custom names that include the value of my
val_loss
, ie.path/epoch_2-val_loss_0.2.hdf5
. The documentation forModelCheckpoint
suggests that this is possible using thefilepath
argument. This does not appear to be the case, since the source code callsos.mkdirs(filepath)
. I have also tried using theprefix
argument, but it doesn't seem to be possible to pass it a format string containing a variable.Expected behavior
The documentation claims that
filepath='{epoch:02d}-{val_loss:.2f}.hdf5'
will save a checkpoint at/path/epoch_2-val_loss_0.2.hdf5
. Instead, it saves a checkpoint at{epoch:02d}-{val_loss:.2f}.hdf5/_ckpt_epoch_1.ckpt
.The issues in the documentation are two-fold:
-- It suggests that
filepath
can contain the directory + name of the checkpoint, when it seems like it should only contain the directory specifying where to save.-- It suggests that it can 'contain named formatting options to be auto-filled', which also doesn't seem to be the case.
Is it possible to achieve this functionality with the
prefix
argument instead? If so, how?The text was updated successfully, but these errors were encountered: