priyappillai
commented
3 years ago My only concern has to do with renaming the model paths, which removed the model hashes. Was there a good reason for this? If we update the models (e.g., new architecture or hyperparameters) users might still want to be able to reproduce their work on the old models. Likewise, for reproducibility, I can see it being helpful if it logs some information about the specific model that's being used (even if just a hash). I'm not saying the particular hashing/path structure that was previously there is the best way to go, but just something to keep in mind in case models are updated.
I did want to change the model paths so that if we introduce models for activity other than Cas13a, the path structure would be consistent and easy to tell apart. I can add another subdirectory of the version (with "current" for the current version)?
Add '--predict-cas13a-activity-model' as an argument so that users use the model even in packaged form. Rename folders containing Cas13a models to Cas13a for readability and generalizability. Move models/ to adapt/models/ so that they can be included as package_data. Modify README to account for these changes and explain FASTA file download. Fix version.py to not reference CATCH and setup.py to include the models folder in the package. General code clean up.