Currently the input.yaml is not validated. Users can set additional fields that are not read by APCEMM (typos / deprecated fields) without knowing that these have no effect. For the case of the typos, given that we have input file overlays, it means that the parameter the user meant to set is silently patched over by the default value and they do not know this is happening. To fix this I think we need a couple of things:
- Validation of user inputs: the
defaults/input.yaml serves as truth of what parameters are read and used by APCEMM. Values found in user inputs that are not found in the default should raise an error.
- Actually check which inputs APCEMM reads and uses. APCEMM does not read certain inputs: great you thought you could set something but you don't. Worse, APCEMM sometimes reads a value and does nothing with it. We need to update the
defaults/input.yaml.
- With the input overlays, it is not always clear what parameters were used after the merging of input files. APCEMM should dump in the output folder a new
.yaml that corresponds to the final merged input that was then used in the simulation.
Something that makes this more complicated than it should be is APCEMM support for parametric sweeps / Monte Carlo within the input file. I think we should drop support for this:
- As far as I know, no one currently uses this feature
- The performance benefit of not restarting APCEMM with a new input file is minimal, and users that do want to do Monte Carlo / parameter sweeps can just run APCEMM multiple times with different input files
- It adds a lot of complexity in the parameter parsing at the start with the case generation
- It adds the need for custom string parsing functions instead of using STL or
yaml-cpp options
- It makes the task of dumping the exact APCEMM inputs used more complicated than it should be
If we do move forward with this plan and drop the sweeps/MC, this will be an annoyance for all users as their current input files will break. While this is annoying, I think the added safety of validation of the inputs is worth it, especially as we move to deprecate more inputs. I am not sure how we want to roll this out: first add validation and then slowly deprecate things, or rip the bandaid and deprecrate a lot of inputs + validate in one go?
Any opinions @sdeastham ?
Currently the
input.yamlis not validated. Users can set additional fields that are not read by APCEMM (typos / deprecated fields) without knowing that these have no effect. For the case of the typos, given that we have input file overlays, it means that the parameter the user meant to set is silently patched over by the default value and they do not know this is happening. To fix this I think we need a couple of things:defaults/input.yamlserves as truth of what parameters are read and used by APCEMM. Values found in user inputs that are not found in the default should raise an error.defaults/input.yaml..yamlthat corresponds to the final merged input that was then used in the simulation.Something that makes this more complicated than it should be is APCEMM support for parametric sweeps / Monte Carlo within the input file. I think we should drop support for this:
yaml-cppoptionsIf we do move forward with this plan and drop the sweeps/MC, this will be an annoyance for all users as their current input files will break. While this is annoying, I think the added safety of validation of the inputs is worth it, especially as we move to deprecate more inputs. I am not sure how we want to roll this out: first add validation and then slowly deprecate things, or rip the bandaid and deprecrate a lot of inputs + validate in one go?
Any opinions @sdeastham ?