V2: consolidate --checkpoint CLI
#731
Conversation
|
Moving to v2.1 because this is more related to uncertainty estimation |
|
Can you check if |
|
@KnathanM I have realized that all these arguments should have been overridden by the |
Co-authored-by: Nathan Morgan <nate.k.morgan@gmail.com>
|
@KnathanM Thanks, fixed! |
chemprop/cli/predict.py
Outdated
| for model_path in model_paths: | ||
| if model_path.suffix in [".ckpt", ".pt"]: | ||
| collected_model_paths.append(model_path) | ||
| elif model_path.is_dir(): | ||
| collected_model_paths.extend( | ||
| list(model_path.rglob("*.ckpt")) + list(model_path.rglob("*.pt")) | ||
| ) |
There was a problem hiding this comment.
I don't think it's a good idea to iterate through all the .ckpt and .pt files in a directory. Maybe we should only use the .pt file. When you train a model, you would have a best.pt and two .ckpt files in the checkpoint directory.
There was a problem hiding this comment.
Hmm, that is a good point. I think we shouldn't look for .ckpt files in a directory, but can still accept it if it is passed as a file path. The checkpoint files are primarily intended for restarting training, so it is reasonable to require checkpoint files to be explicitly passed for prediction (--model-path mymodel.ckpt) if that is intended instead of finding them implicitly in a directory. This way a user can do both:
SAVE_DIR=mydir
chemprop train -o $SAVE_DIR -i data
chemprop predict --model-path $SAVE_DIR -i data
(which will find all the "best.pt" files) and
chemprop predict -i data --model-path *.ckpt
(which will find all the ckpt files in a directory, but note that they all passed as file paths and not the path to a directory).
There was a problem hiding this comment.
That's a good point. I have modified to only search for .pt files if a directory is provided.
My PR at #667 got closed automatically when I changed my forked source, and I couldn't reopen it. So I open this one to continue the PR.