search

dattalab / moseq2-model

Python scripts for linking kinect-extraction to pybasicbayes
Other
1 stars 2 forks source link

CLI module only checks output filename extension after modeling #79

Closed jonahpearl closed 1 year ago

jonahpearl commented 3 years ago

Issue: I accidentally ran "moseq2-model learn-model _pca/pca_scores.h5 model.py -c 8 --robust" instead of "moseq2-model learn-model _pca/pca_scores.h5 model.p -c 8 --robust". Moseq should catch this error upfront, instead it went through all the modeling and then failed because it didn't recognize the extension.

Current behavior: error is raised in utils/save_dict() [After modeling] File "/home/jop9552/miniconda3/envs/moseq2-app/lib/python3.7/site-packages/moseq2_model/cli.py", line 89, in learn_model learn_model_wrapper(input_file, dest_file, config_data) File "/home/jop9552/miniconda3/envs/moseq2-app/lib/python3.7/site-packages/moseq2_model/helpers/wrappers.py", line 164, in learn_model_wrapper save_dict(filename=dest_file, obj_to_save=export_dict) File "/home/jop9552/miniconda3/envs/moseq2-app/lib/python3.7/site-packages/moseq2_model/util.py", line 242, in save_dict raise ValueError('Did not understand filetype') ValueError: Did not understand filetype

Expected behavior: file extension should be parsed up front in helpers/wrappers/learn_model_wrapper.py: assert [output extension] in [list of accepted output extensions]

versey-sherry commented 1 year ago

Closing this issue because users should pay attention to the commands before running them.

jonahpearl commented 1 year ago

It's been a while so I don't remember, but if you re-run with the right extension, does it finish quickly because it was checkpointing the whole time? Or does this error mean you have to start all over again? In general I agree users should check commands before running them, but this is clearly a typo that might be hard to catch, not a misuse of an argument or something that could be gleaned by reading the docs. I think if it wastes a few hours of the user's time, why not just catch it?

versey-sherry commented 1 year ago

Yes, if you turn on the checkpoint, then the model would load the checkpoint, so if there is checkpoint saved a few iterations away from the finish line, that checkpoint will be loaded and the model will train from there.