Closed bettbra closed 4 years ago
This pull request is being automatically deployed with Vercel (learn more). To see the status of your deployment, click below or on the icon next to each commit.
🔍 Inspect: https://vercel.com/covid19-scenarios/covid19-scenarios/r6ukndfu4 ✅ Preview: https://covid19-scenarios-git-fork-bettbra-cli.covid19-scenarios.now.sh
Code Climate has analyzed commit 0bc426ba and detected 0 issues on this pull request.
View more on Code Climate.
@bettbra Hi Bradley, Thanks! I should probably note that at this point we prioritize the app and we don't officially support the CLI. Personally, I never tried to run it and don't know if it runs correctly. Theoretically it should :) That being said, it wouldn't hurt to have a CLI if community wants it.
However, before merging, I need to ask the folks at GitHub/Microsoft who originally contributed it in #689. @adityasharad Could you please review the changes and see if it is okay for your use-case? I'll be happy to merge then.
@bettbra Could you please tell us more about your project, so that we could take this information into account when planning the improvements of the algorithm?
Any additional improvements are welcome, including any of the items listed in https://github.com/neherlab/covid19_scenarios/issues/663
Hi Ivan,
Thanks for your reply. Right, I should have checked to try to infer if another team was also using the CLI. I suspect the changes here won't work for them (e.g., @adityasharad) as they already have dependencies on the existing form, but maybe we can coordinate. For us it's convenient to be able to control all parameters---scenario, age, severity---at the command line, meaning not only the first set. So really the change here is focused on adding the last two (age, severity) and using yargs for argument parsing purely for convenience.
I also see a bunch of eslint warnings that I'll fix; forgive me, didn't realize those were getting run. I'll eliminate all the warnings to adhere to the project conventions.
As for what we're using the project for, we're trying to vary R_0 using mobility data, very similar in spirit to #715. We execute large numbers of runs as we assess sensitivities to various input parameters, doing this on the command line in a large gridded compute environment. The UI is fantastic and we use it for quick sanity checks, but our heavy lifting is done via the CLI.
Apologies for the long reply. If the community would favor making the CLI a supported interface we'd be enthusiastic supporters and commit to helping maintain it. Thanks again @ivan-aksamentov. I'll await additional overall feedback and at least fix those warnings as mentioned.
--Brad
@bettbra
For us it's convenient to be able to control all parameters---scenario, age, severity---at the command line
Yes. That is probably the most useful interface for the community.
Feel free to propose the improvements to the interface that are useful for you. For example we could make every parameter to be exposed from both files and command line, and command line would take precedence. This way one could have a base config and vary a given parameter. We may also read json from stdin, so that users could pipe the autogenerated configs through the shell or exec.
I also see a bunch of eslint warnings
Note that the current eslint config is mostly tailored for the React app, so some of these warning may not necessarily make much sense for a Node script. In this case, feel free to either (1) add an eslint-ignore comment for the file or (2) add an item in the override
section of the eslint config file or (3) .eslintignore
the cli file/directory and create a separate config. Whichever suits your needs the best.
As for what we're using the project
Wow, that sounds cool! Do you have a link?
helping maintain it
That would be fantastic! Feel free to pick any if the items in https://github.com/neherlab/covid19_scenarios/issues/663 and add yours too. We are looking forward to any feedback and contributions.
I guess currently you have to pull the project from GitHub in order to use it, maybe as a submodule or similar. Would it be useful if we create and NPM package with a CLI and programmatic Node.js interface? We could automate the build and publishing as well. One advantage is more convenient versioning and reduction of breakage, because we are currently not tracking how our changes affect the CLI at all.
this does look like a very useful direction and we might consider ditching our parallel implementation in python for model fitting.
@adityasharad Thanks for reviewing.
@adityasharad @bettbra I will merge and then fix the kinks on master. Let me know if there's anything to ammend or what other improvements you have in mind.
Thanks all for the input. I fixed the eslint kinks on my branch without changing the eslint settings. I can open another pull request for it if you'd like; apologies for the delay. Thanks @adityasharad and @ivan-aksamentov for your feedback.
Oh, and I should have commented @rneher: We're grateful for any direction you want to take the project that allows command line running, or more broadly programmatic modification of parameters. If this does the trick for the community, super, but happy to switch to any other language or methodology.
Thanks again for all your hard work on this.
Add a yarn target for running the model via the command line.
Description
These changes allow the model to be run via the command line via a
yarn cli
call. Two arguments are required:--scenario
and--out
.Impacted Areas in the application
The changes are all pretty trivial. Some command line parsing is done via
yargs
insrc/algorithms/cli.ts
(and so a dependency is introduced on the yargs package inpackage.json
).Testing
docs/comand_line.md
into a filescenario.json
.yarn cli --help
to see the usage message.yarn cli --scenario=scenario.json --out=output.json
.output.json
was created by the run and has reasonable-looking output.Additional Notes