covid-sim
covid-sim copied to clipboard
Need help with figuring out which parameters are optional
I just took a look at: https://github.com/mrc-ide/covid-sim/blob/master/docs/inputs-and-outputs.md
But I don't see any description of the parameters like which one are mandatory or not...
I especially want to know if those in brackets are.
Halp pls! :-)
I finally figured it out by myself.
Finally it would be a good thing to explain which parameters are mandatory... I'm having a hard time figuring this one out from the code only.
Especially this part because in the documentation the admin file in not enclosed in brackets but in the param loop theres no check for it https://github.com/mrc-ide/covid-sim/blob/01010f52a3a4e2cc8eaa09d144ad21bd1ea88c22/src/CovidSim.cpp#L164
There are no mention of the kernel strenght in the documentation also https://github.com/mrc-ide/covid-sim/blob/01010f52a3a4e2cc8eaa09d144ad21bd1ea88c22/src/CovidSim.cpp#L184
It would be good to understand how kernels are used and set. Maybe that's another issue to raise.
Finally it would be a good thing to explain which parameters are mandatory... I'm having a hard time figuring this one out from the code only.
I attempted to document which command-line parameters were mandatory/optional in #164, at least from the perspective of the main()
function. This was merged yesterday, so I would advise taking a second look at the inputs-and-outputs.md
file on Master now to see if this documentation helps
Also, I will be making a pull request later this afternoon which refactors the if/else parameter parsing into a much more, I hope, easier-to-read series of switch cases in addition to a '/H' option for a detailed help similar to the inputs-and-outputs.md
file.
I will say that even though the main()
parameter parsing code seems to only require the /O
and /P
parameters in addition to the RNG seeds, it appears that there are a lot of hidden requirements in the /P
file that are usually provided in the Pre-Param file. I hope to eventually make some headway into documenting that.
@ozmorph Do you think I can do a pull request of the program_options code I did to replace the very complex parsing at the beginning of the main? When it will be done of course... Since I moved a lot of stuff around in my fork I might reclone it and include only this part. It efficiently clears up all the parsing stuff.
@Feynstein I actually just submitted my PR #228 that did a lot of work on the main() CLI argument parsing. Can you send me a link to your branch so that I can compare/integrate with it?
@ozmorph Of course! https://github.com/Feynstein/covid-sim https://github.com/Feynstein/covid-sim/blob/master/CovidSim.cpp
What you want to look at is my main... Its been changed a lot and I upgraded to C++17 in order to use the std::any but I think that you could easily copy-paste the program options part and grow from it to what is necessary. I would like to be quoted as a collaborator if you do though :-) This is a good thing for a scientific dev like me :P
Oh and in my CMake Lists I added an automatic out-of-source -git-branch-related-debug/release build, so that when you get in there you only have to do "cmake ."
And I already sent an email from my university account to @weshinsley in order to see if I could be an official scientific contributor...
Thanks, I will take a look at it. If I am asked to make any changes in #228, then I will consult your code to see if there is a middle ground for both implementations and adjust accordingly.
And of course, I will give you acknowledgement of collaborating if I do integrate your changes.
@ozmorph ohhh there something I forgot when I looked at #228. There would be a very easy way to verify if the files exist using boost::filesystem or std::filesystem (I think this one is in it). I did not do it now, but it would be easy to add to the checks done on the arguments themselves.
In order to use program_options, which I dont know if you know, you simply add your arguments like: --preParamF=/some/file
and for the multiple files you reuse the same call: --interventionsF=/some/file1 --interventionsF=/some/file2
@Feynstein @ozmorph Yohan no you can't do that for #228. The build is C++11, std::filesystem is C++17. git submodules seem to be ignored by the CI system so you won't get boost in either.
@zebmason Well... It seems to me that this could be added as a pressing issue in order to be able to make any significant improvement on the original code... boost is used almost everywhere in the scientific code I use... And probably in most other code that I don't know about.
@Feynstein Yohan there is a backport of std::filesystem that I've used in the past and I have a approved commit for C++14 (#207) and may get submodules sorted due to #213
@zebmason I already filed an issue #229 about this because in itself is a very pressing issue imo... and also since its not really related to this one (definitions of parameters) you can comment there for a better issue modularity. What you just said basically.
Thanks for your time on this. Many parameters have defaults, which isn’t quite the same as being optional. We will try to generate some “minimal” parameter files in the coming days and weeks though. It is possible to run the code as a simple non-spatial stochastic SEIR model with no households or places with a tiny number of parameters. But we will also try to generate some more functional minimalist parameter files.
@NeilFerguson Thanks for your answer, really appreciated. I will try to continue my work on this. I've started a pretty big refactoring of the code that I might present as a parallel, object oriented version of this. But for now I can't because I'm sick and my day job reopened. I don't know if I will be able to completely refactor this thing but I think that I brought a few fresh ideas that might help the future scientific development of this simulation. In the best of worlds everything should be modular so that new grad students or scientists that are unfamiliar with C++ can create his own sub-library and call it from the relevant place in the code, without risking to damage it. That's what I will try to improve next. Thanks and keep up the good work!