-
Notifications
You must be signed in to change notification settings - Fork 429
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
#Minor Fixes final checkpoint save for wsj0-mix DeepClustering example. #398
Conversation
98c298b
to
38db9f9
Compare
Does this apply to other recipes as well? |
38db9f9
to
4b0068d
Compare
@jonashaag I compared to But the lines:
are present in other egs, and what happens (with pytorch_lightning version '1.1.2' anyway) is that |
egs/wsj0-mix/DeepClustering/train.py
Outdated
@@ -80,7 +80,7 @@ def main(conf): | |||
with open(os.path.join(exp_dir, "best_k_models.json"), "w") as f: | |||
json.dump(best_k, f, indent=0) | |||
# Save last model for convenience | |||
torch.save(system.model.state_dict(), os.path.join(exp_dir, "checkpoints/final.pth")) | |||
torch.save(system.model.state_dict(), os.path.join(exp_dir, "best_model.pth")) |
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.
Can you change in the kinect eg as well?
Yeah that behaviour changed in PL 1. Would you mind sending a fix for all the egs to use |
Thanks a lot for the PR @ilyakava, let's fix this! Do we have to fix the model loading in the eval file as well? |
Hey @ilyakava, do you have updates about this? |
Makes
wsj0-mix/DeepClustering
final save likewham/DPTNet
.PyTorch Lightning's
pytorch_lightning.callbacks.ModelCheckpoint
assumesdirpath + filename
not adirpath
as its first argument, but line 83 assumes acheckpoints
folder exists.So when this example runs this error occurs:
since the directory structure looks like:
I'm using pytorch_lightning version '1.1.2' with pytorch 1.8.0a0+1606899.