Centralize argument parsing #948
Conversation
matthiasdiener
force-pushed
the
argparse-new
branch
2 times, most recently
from
362031b to
03e03b4
Compare
matthiasdiener
force-pushed
the
argparse-new
branch
from
28c2772 to
c3d0f26
Compare
| @@ -733,25 +733,14 @@ def my_post_step(step, t, dt, state): | |||
|
|
|||
|
|
|||
| if __name__ == "__main__": | |||
| from mirgecom.simutil import add_general_args | |||
There was a problem hiding this comment.
add_predefined_cmd_line_options?
| dest="restart_file", nargs="?", action="store", | ||
| help="simulation restart file") | ||
| parser.add_argument("--casename", help="casename to use for i/o") | ||
| add_general_args(parser, overintegration=False) |
There was a problem hiding this comment.
I think I'd prefer if this was a little more customizable, and if the options were opt-in rather than opt-out. Otherwise a driver might end up accepting options that it doesn't actually support. Maybe something like:
add_predefined_cmd_line_options(parser, [
"lazy",
"profiling",
"log",
"leap",
"numpy",
"restart_file",
"casename"])?
There was a problem hiding this comment.
I see your point, but wouldn't that lead to similar issues we have right now? E.g., adding a new option, like --numpy or --esdg, requires changes in every driver?
A driver can also choose to ignore certain options (by not accessing the corresponding args.foo option, or throwing an error).
There was a problem hiding this comment.
Sure, but the driver would have to be modified in order to actually use the new option anyway. Doing the above would at least reduce the amount of boilerplate.
There was a problem hiding this comment.
A driver can also choose to ignore certain options (by not accessing the corresponding
args.foooption, or throwing an error).
The ignoring thing scares me a little... if a particular driver doesn't support an option, IMO it shouldn't accept it.
There was a problem hiding this comment.
As someone who is constantly writing and maintaining drivers, I would like it very much if the options for actx creation, the MPI entry point, and any other non-physics options (even logpyle init) could be parsed by the "front-matter" code, and the rest of the "application-specific" or "simulation-specific" args could be handled by the specific physics-oriented driver.
casename, restart file specification, etc, all belong to the physics driver domain, and not the "front-matter" stuff. We can easily write generalized components to handle common sets of physics or simulation control args.
There is a built-in argparse mechanism for doing this parse_known_args that lets us parse command line args in stages, by different components.
There was a problem hiding this comment.
A driver can also choose to ignore certain options (by not accessing the corresponding
args.foooption, or throwing an error).The ignoring thing scares me a little... if a particular driver doesn't support an option, IMO it shouldn't accept it.
This should be up to the driver developer imo. parse_known_args will continue to accumulate the un-parsed args, and at the end if there are any unknown args, let the driver toss an error if the driver developer deems that necessary, or give a warning or ignore.
(of course we can require that our examples toss an error, but don't force everyone to do it)
There was a problem hiding this comment.
This should be up to the driver developer imo.
parse_known_argswill continue to accumulate the un-parsed args, and at the end if there are any unknown args, let the driver toss an error if the driver developer deems that necessary, or give a warning or ignore.(of course we can require that our examples toss an error, but don't force everyone to do it)
Makes sense. Could have the "front matter" call parse_known_args and then the examples can call parse_args on whatever's left I guess?
There was a problem hiding this comment.
That's along the lines of what I was thinking, then we can make like 2 or 3 more drivers (to rule them all) which drive fluid-only or coupled problems and parse all the common args there, and so on. This would probably be sufficient for all the examples and reduce our maintenance task to just those few drivers.
Each component can use the standard built-in argparse facility for defining the args and the help strings, etc, and we can just gather up all the common ones into convenient blocks.
Questions for the review: