Add functionality for generating parameter file for one-at-the-time tests #13
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for one-at-a-time (OAT) sensitivity analysis as an alternative to Latin Hypercube sampling. The changes refactor the parameter file generation logic into modular functions and introduce new configuration options.
- Refactored monolithic
generate_paramfileinto separate functions for different sampling strategies - Added
one_at_the_timeflag to switch between Latin Hypercube and OAT sampling - Added
perturbed_chem_mechflag to control chemistry mechanism perturbations - Reorganized CLI to make parameter ranges file optional with a default value
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| tinkertool/scripts/generate_paramfile/generate_paramfile.py | Refactored into modular functions; added OAT sampling logic and conditional dataset attributes |
| tinkertool/scripts/generate_paramfile/config.py | Added one_at_the_time and perturbed_chem_mech configuration fields with validation |
| tinkertool/scripts/generate_paramfile/cli.py | Reorganized CLI arguments and added flags for new features |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| help="Whether to exclude the default parameter value in the output file in nmb_sim=0. Using this flag will skip nmb_sim=0. Default is to include default value.", | ||
| ) | ||
| parser.add_argument( | ||
| "--perturbe-chem-mech", |
There was a problem hiding this comment.
Corrected spelling of 'perturbe' to 'perturb' in CLI argument name.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Johannesfjeldsaa
left a comment
Johannesfjeldsaa
left a comment
There was a problem hiding this comment.
Only a comment on what the functionality on how exclude_default should be
| # Fill variation slots | ||
| idx = 1 # start after default | ||
| if config.exclude_default == False: | ||
| idx = 1 # start after default |
There was a problem hiding this comment.
Default behavior is that default is included i.e. with index 0, if you exclude_default you should start at 1? so the other way around?
| "help": "List of parameters to be sampled, have to be defined in param_ranges_inpath. If unspecified all parameters in param_ranges_inpath will be used" | ||
| }, | ||
| ) | ||
| one_at_the_time: bool = field( |
There was a problem hiding this comment.
Thinking that it might be more future proof to use method field instead. Then use a valid_methods = ['one-at-the-time', 'latin-hypercube'] for checking. In this way it is easier to introduce more methods later
| ) | ||
| # exclude_default | ||
|
|
||
| check_type(self.one_at_the_time, bool) |
There was a problem hiding this comment.
This ties into the comment above: instead on treating one_at_the_time perturbations as a special case it can be a different "sampling_class", same with lhypc. If they have the same methods code become more general.
…ndling - Changes are to the best of my ability brought in from general_purpose_4_noresm3 - TODO take suggestions from johannes
2 participants
Often before pushing go on a large perturbed parameter ensemble it is useful to test the ranges and parameters beforehand by doing a one-at-the-time at the min and max of the range for each parameter.
To add this new functionality the generate_paramfile.py has be refactored and latin hypercube sampling have been put in its own function similar to the one-at-the-time routines
generate-paramfilegives expected output