search

USGS-R / river-dl

Deep learning model for predicting environmental variables on river systems
Creative Commons Zero v1.0 Universal
21 stars 15 forks source link

rename `train_model.py`? #111

Closed jsadler2 closed 3 years ago

jsadler2 commented 3 years ago

As is, it's not clear how train_model.py differs from train.py. It would be clearer to rename it to maybe train_model_cli.py (credit @aappling-usgs for the name) so that it's clearer that this script is to be used as a command line interface (cli).

The only time I've had to use (and forsee using) this functionality is when I have to issue a direct command to Slurm when I want to use the Tallgrass GPUs https://github.com/USGS-R/river-dl/blob/9d2d340dd1f95bb09eb00a04cad5e73327fdb65e/Snakefile#L62

Summary: proposing renaming train_model.py ---> train_model_cli.py

Any "yays" or "nays" here, @SimonTopp, @jzwart, @janetrbarclay on renaming? Is there a better name?

janetrbarclay commented 3 years ago

That works for me. I did need to look at it a few times to understand the difference between them (the comments in the snakefile are super helpful for that), but then it made sense and was clear. I wouldn't know without the explanation what "cli" means, so maybe include that as a note in the top of the train_model_cli.py file and the snakefile?

jzwart commented 3 years ago

I think train_model_cli.py is good, and agree with @janetrbarclay for adding the explanation of cli