Introduce multiple default settings files #297
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* Instead of src/, YAML files are in defaults/ * Instead of autogenerated_src/, JSON files are in defaults/json/ * Python scripts in MARBL_tools look for / create these files in correct location
To avoid over-using "default" in file / directory names, the most recent yaml
files are "settings_latest.yaml" and "diagnostics_latest.yaml". Note that this
also switches from [version]_{settings,diagnostics}.yaml to
{settings,diagnostics}_[version].yaml so all the settings files are listed
together.
These name changes are also reflected in the JSON.
Rather than explicitly providing a single settings file and single diagnostics file to convert to JSON, users can pass a list of YAML files with the --yaml_files option (default value is defaults/*.yaml).
Documentation now reflects changes to location of YAML files and improvements to yaml_to_json.py script.
|
Updated documentation is on my Read the Docs page. I made changes to three pages:
|
1. Error from a code consistency check now names the function reporting the error 2. Introduced a yaml_to_json() subroutine to make diffs easier to parse on github 3. Did some string manipulation to get human-readable results in default values when running "yaml_to_json.py -h" 4. Cleaned up docstring to present default values in clearer manner
1. No need to provide output from yaml_to_json.py -h 2. The example in "Adding a Diagnostic" should be a clean snippet of YAML, not something with comments, anchors, and / or references.
|
Commits 71dad02 and d465de0 address concerns raised in code review with @klindsay28 -- along with marbl_dev_levy_n192 I think this is ready to be put on That said, I'm going to wait until the CMIP6 tunings are in MARBL / POP before accepting this pull request so that I can deal with the inevitable conflicts. |
merging development onto this branch, default_settings.{yaml,json}
became setings_cesm2.0.{yaml,json} instead of settings_cesm2.1, and
settings_cesm2.1 remained the (old) settings file that didn't have all
the new parameters in it.
Default had inadvertently been set to .true. for the x1 resolution.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We want the CESM 2.1 release to use the CMIP6 tunings by default, but also be able to reproduce the CESM 2.0 results. Per #274, we are providing multiple default settings files and allowing the user to choose which one is used.
src/todefaults/autogenerated_src/todefaults/json/default_settings.{yaml,json}->settings_latest.{yaml,json}and also introducesettings_cesm2.0.{yaml,json}to reproduce old tuningsdefault_diagnostics.{yaml,json}->diagnostics_latest.{yaml,json}yaml_to_json.pyto account for new directories / file names (and to convert all yaml -> json instead of looking for specific files)default_{diagnostics,settings}.jsonto look for{diagnostics,settings}_latest.jsoninstead (to allow forSourceMods, these variables could already be changed by the user so nothing needed to be updated to allow users to specifysettings_cesm2.0.jsoninstead)