search

libqueso / queso

QUESO is a C++ library for doing uncertainty quantification. QUESO stands for Quantification of Uncertainty for Estimation, Simulation and Optimization.
Other
58 stars 39 forks source link

Mixed input file and C++ API options handling #569

Open briadam opened 7 years ago

briadam commented 7 years ago

Enhance other (Environment, Metropolis Hastings, inverse problem) constructors with options setting flexibility similar to GPMSA, so one can default construct, set via C++ API, or parse from input file in flexible order. In particular, it's not possible to set through C++, then override with power user options from an input file.

See discussion with @roystgnr https://groups.google.com/forum/#!topic/queso-users/S58W-IzRRtI

Related, there is a potential chicken and egg problem with the options for constructing an Environment, as parsing Environment options from a file requires an Environment...

dmcdougall commented 7 years ago

If the user passes an options object, the input file never gets parsed. Probably want to mess around with that logic.

briadam commented 7 years ago

@dmcdougall @roystgnr For the Environment (and other) constructors that accept both input file name and EnvOptionsValues object, what do you feel the precedence should be?

On the Dakota side, we're planning to have the C++ options object apply first, followed by the parsed input file options as this lets a user write a C++ program with their preferred defaults possibly overriding QUESO class defaults, but then lets power users override them at run-time through a file. So our thinking is (1) class default values, possibly overridden by (2) explicitly set options in C++ options object, possibly overridden by (3) user input file.

briadam commented 7 years ago

Per @dmcdougall, also need to consider where in this workflow a client might programmaticly set options as well, e.g., to override defaults through C++ code.

roystgnr commented 7 years ago

Although the goal is to give the user as much control over the ordering as possible, there's no way to do that when you have two options sources going into the same constructor, and "C++ object gets used first, but input file can override anything in it" is clearly the way to go. We can make sure there's accessors available if anyone wants to then override an input file option from C++ even later.

briadam commented 7 years ago

Where's the right place in the code base to put a convenience function for converting sets of scalars to a string as I am doing in EnvironmentOptions for default management?
Edit: Never mind, I went with Miscellaneous.h/C

briadam commented 7 years ago

Should I leave SequenceStatisticalOptions in place or remove it? Or at least remove its use in other Options classes? I see it's deprecated and it's complicating MetropolisHastingsSGOptions among other Options classes...

briadam commented 7 years ago

Related, should other deprecated Options classes like MetropolisHastingsSGOptions be left in place or removed for this refactor?

roystgnr commented 7 years ago

Miscellaneous should be fine.

SequenceStatisticalOptions was deprecated in the 0.55, 0.56, and 0.57 releases, so IMHO it's ready to remove. I'd do that in a separate PR though; we ought to be able to merge it quickly. Likewise for MetropolisHastingsSGOptions, or anything else that's been deprecated for a couple releases.

briadam commented 7 years ago

To close out this issue, need to take one more cleanup pass and review: