Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve parameter inference #682

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 77 additions & 0 deletions docs/source/parameters.rst
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,80 @@ See `this release's parameter defaults <https://github.com/E3SM-Project/zppy/blo
on GitHub for a complete list of parameters and their default values.
You can also view the most up-to-date,
`unreleased parameter defaults <https://github.com/E3SM-Project/zppy/blob/main/zppy/templates/default.ini>`_.

Deprecated parameters
=====================

The following parameters no longer perform any function:
::

"e3sm_to_cmip_environment_commands"
"ts_fmt"
"scratch"
"atmosphere_only"
"plot_names"

Parameter checking & inferring -- for users
===========================================

There are two types of inferences, each with their own parameter in ``default.ini``:

* ``infer_path_parameters``: infer paths that are not explicitly provided in the configuraiton file. Default is ``True``.
* ``infer_section_parameters``: infer subtask dependency names that are not explicitly provided in the configuration file. Default is ``True``.

**Section inferences**

For the ``climo``, ``ts``, ``e3sm_to_cmip``, and ``e3sm_diags`` tasks:

* If ``subsection`` (the name of the subtask) is undefined, just use the value of ``grid``.

For the ``e3sm_to_cmip`` task:

* If ``ts_subsection`` (the name of the ``ts`` subtask that this ``e3sm_to_cmip`` subtask is dependent on) is undefined, assume it had the same name as this ``e3sm_to_cmip`` subtask.

For the ``ilamb`` task:

* If ``ts_land_subsection`` (the name of the ``ts`` land-specific subtask that this ``ilamb`` task is dependent on), assume it is ``land_monthly``.
* If ``e3sm_to_cmip_land_subsection`` (the name of the ``e3sm_to_cmip`` land-specific subtask that this ``ilamb`` task is dependent on), again assume it is ``land_monthly``.
* If we are not doing a ``land_only`` run and ``ts_atm_subsection`` (the name of the ``ts`` atm-specific subtask that this ``ilamb`` task is dependent on), assume it is ``atm_monthly_180x360_aave``.
* If we are not doing a ``land_only`` run and ``e3sm_to_cmip_atm_subsection`` (the name of the ``e3sm_to_cmip`` atm-specific subtask that this ``ilamb`` task is dependent on), again assume it is ``atm_monthly_180x360_aave``.


**Path inferences**

For the ``e3sm_diags`` task:

* If ``reference_data_path`` (the path to the reference data) is undefined, assume it is the ``diagnostics_base_path`` from Mache plus ``/observations/Atm/climatology/``. (So, it is important to change this for model-vs-model runs).


For the ``ilamb`` task:

* If ``ilamb_obs`` (the path to observation data for ``ilamb``) is undefined, assume it is the ``diagnostics_base_path`` from Mache plus ``/ilamb_data``.

**Required parameters, by e3sm_diags set**

TODO: Insert main table from Confluence

These parameters are required for all model-vs-model runs: ``diff_title``, ``ref_name``, ``short_ref_name``.

TODO: Insert model-vs-model table from Confluence
Comment on lines +70 to +74
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://acme-climate.atlassian.net/wiki/spaces/IPD/pages/4984209586/zppy+parameters+for+e3sm_diags

It's really difficult to create tables in rst, so for a big table, Confluence seemed like the better fit. For actually including it in the docs, I suppose we could add in screenshots?


Parameter checking & inferring -- for developers
================================================

There are many parameter-handling functions.

In ``utils.py``:

* ``get_value_from_parameter``: check if parameter is in the configuration dictionary. If not, if inference is turned on (the default), then just use the value of ``second_choice_parameter``. If inferenceis turned off, raise a ``ParameterNotProvidedError``. Use this function if the backup option
* ``set_value_of_parameter_if_undefined``: check if parameter is in the configuration dictionary. If not, if inferenceis turned on (the default), then just set the parameter's value to the ``backup_option``. If inferenceis turned off, raise a ``ParameterNotProvidedError``.

In ``e3sm_diags.py``:

* ``check_parameter_defined``: check if parameter is in the configuration dictionary, and if not raise a ``ParameterNotProvidedError``.
* ``check_set_specific_parameter``: if any requested ``e3sm_diags`` sets require this parameter, make sure it is present. If not, raise a ``ParameterNotProvidedError``.
* ``check_parameters_for_bash``: use ``check_set_specific_parameter`` to check the existence of parameters that aren't used until the bash script.
* ``check_mvm_only_parameters_for_bash``: similar, but these are specifically parameters used for model-vs-model runs. Uses ``check_parameter_defined`` in addition to ``check_set_specific_parameter``.
* ``check_and_define_parameters``: make sure all parameters are defined, using ``utils.py get_value_from_parameter``, ``utils.py set_value_of_parameter_if_undefined``, and ``check_mvm_only_parameters_for_bash``.

``check_parameters_for_bash`` can be run immediately for each subtask because it has very few conditions. Other checks are included in ``check_and_define_parameters`` later on in the code.
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ constraint = ""
dry_run = True # Exlcusively testing with dry_run
environment_commands = ""
fail_on_dependency_skip = True
guess_path_parameters = False
guess_section_parameters = False
infer_path_parameters = False
infer_section_parameters = False
input = /lcrc/group/e3sm2/ac.wlin//E3SMv3/v3.LR.historical_0051
input_subdir = archive/atm/hist
mapping_file = "map_ne30pg2_to_cmip6_180x360_aave.20200201.nc"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ constraint = ""
dry_run = "False"
environment_commands = ""
fail_on_dependency_skip = True
guess_path_parameters = False
guess_section_parameters = False
infer_path_parameters = False
infer_section_parameters = False
input = /lcrc/group/e3sm2/ac.wlin//E3SMv3/v3.LR.historical_0051
input_subdir = archive/atm/hist
mapping_file = "map_ne30pg2_to_cmip6_180x360_aave.20200201.nc"
Expand Down
4 changes: 2 additions & 2 deletions tests/integration/generated/test_min_case_nco_chrysalis.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ constraint = ""
dry_run = "False"
environment_commands = ""
fail_on_dependency_skip = True
guess_path_parameters = False
guess_section_parameters = False
infer_path_parameters = False
infer_section_parameters = False
input = /lcrc/group/e3sm2/ac.wlin//E3SMv3/v3.LR.historical_0051
input_subdir = archive/atm/hist
mapping_file = "map_ne30pg2_to_cmip6_180x360_aave.20200201.nc"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ constraint = ""
dry_run = "False"
environment_commands = ""
fail_on_dependency_skip = True
guess_path_parameters = False
guess_section_parameters = False
infer_path_parameters = False
infer_section_parameters = False
input = /lcrc/group/e3sm2/ac.wlin//E3SMv3/v3.LR.historical_0051
input_subdir = archive/atm/hist
mapping_file = "map_ne30pg2_to_cmip6_180x360_aave.20200201.nc"
Expand Down Expand Up @@ -144,7 +144,7 @@ tc_obs = "/lcrc/group/e3sm/diagnostics/observations/Atm/tc-analysis/"
# Reference paths
reference_data_path = "/lcrc/group/e3sm/diagnostics/observations/Atm/climatology/"
# mvo diurnal_cycle only
# NOTE: This is NOT the guess zppy would have made!
# NOTE: This is NOT the inference zppy would have made!
dc_obs_climo = '/lcrc/group/e3sm/public_html/e3sm_diags_test_data/unit_test_complete_run/obs/climatology'
# mvo streamflow only
streamflow_obs_ts = "/lcrc/group/e3sm/diagnostics/observations/Atm/time-series/"
Expand Down
4 changes: 2 additions & 2 deletions tests/integration/template_min_case_deprecated_parameters.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ constraint = "#expand constraint#"
dry_run = True # Exlcusively testing with dry_run
environment_commands = "#expand environment_commands#"
fail_on_dependency_skip = True
guess_path_parameters = False
guess_section_parameters = False
infer_path_parameters = False
infer_section_parameters = False
input = #expand user_input_v3#/E3SMv3/#expand case_name#
input_subdir = archive/atm/hist
mapping_file = "map_ne30pg2_to_cmip6_180x360_aave.20200201.nc"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ constraint = "#expand constraint#"
dry_run = "#expand dry_run#"
environment_commands = "#expand environment_commands#"
fail_on_dependency_skip = True
guess_path_parameters = False
guess_section_parameters = False
infer_path_parameters = False
infer_section_parameters = False
input = #expand user_input_v3#/E3SMv3/#expand case_name#
input_subdir = archive/atm/hist
mapping_file = "map_ne30pg2_to_cmip6_180x360_aave.20200201.nc"
Expand Down
4 changes: 2 additions & 2 deletions tests/integration/template_min_case_nco.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ constraint = "#expand constraint#"
dry_run = "#expand dry_run#"
environment_commands = "#expand environment_commands#"
fail_on_dependency_skip = True
guess_path_parameters = False
guess_section_parameters = False
infer_path_parameters = False
infer_section_parameters = False
input = #expand user_input_v3#/E3SMv3/#expand case_name#
input_subdir = archive/atm/hist
mapping_file = "map_ne30pg2_to_cmip6_180x360_aave.20200201.nc"
Expand Down
6 changes: 3 additions & 3 deletions tests/integration/template_weekly_comprehensive_v3.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ constraint = "#expand constraint#"
dry_run = "#expand dry_run#"
environment_commands = "#expand environment_commands#"
fail_on_dependency_skip = True
guess_path_parameters = False
guess_section_parameters = False
infer_path_parameters = False
infer_section_parameters = False
input = #expand user_input_v3#/E3SMv3/#expand case_name#
input_subdir = archive/atm/hist
mapping_file = "map_ne30pg2_to_cmip6_180x360_aave.20200201.nc"
Expand Down Expand Up @@ -144,7 +144,7 @@ tc_obs = "#expand diagnostics_base_path#/observations/Atm/tc-analysis/"
# Reference paths
reference_data_path = "#expand diagnostics_base_path#/observations/Atm/climatology/"
# mvo diurnal_cycle only
# NOTE: This is NOT the guess zppy would have made!
# NOTE: This is NOT the inference zppy would have made!
dc_obs_climo = '/lcrc/group/e3sm/public_html/e3sm_diags_test_data/unit_test_complete_run/obs/climatology'
# mvo streamflow only
streamflow_obs_ts = "#expand diagnostics_base_path#/observations/Atm/time-series/"
Expand Down
32 changes: 16 additions & 16 deletions tests/test_sections.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,8 @@ def test_sections():
"fail_on_dependency_skip": False,
"frequency": "monthly",
"grid": "",
"guess_section_parameters": True,
"guess_path_parameters": True,
"infer_section_parameters": True,
"infer_path_parameters": True,
"input": "INPUT",
"input_files": "eam.h0",
"input_subdir": "INPUT_SUBDIR",
Expand Down Expand Up @@ -132,8 +132,8 @@ def test_sections():
"fail_on_dependency_skip": False,
"frequency": "monthly",
"grid": "",
"guess_section_parameters": True,
"guess_path_parameters": True,
"infer_section_parameters": True,
"infer_path_parameters": True,
"input": "INPUT",
"input_component": "",
"input_files": "eam.h0",
Expand Down Expand Up @@ -188,8 +188,8 @@ def test_sections():
"fail_on_dependency_skip": False,
"frequency": "monthly",
"grid": "",
"guess_section_parameters": True,
"guess_path_parameters": True,
"infer_section_parameters": True,
"infer_path_parameters": True,
"input": "INPUT",
"input_component": "",
"input_files": "eam.h0",
Expand Down Expand Up @@ -245,8 +245,8 @@ def test_subsections():
"fail_on_dependency_skip": False,
"frequency": "monthly",
"grid": "",
"guess_section_parameters": True,
"guess_path_parameters": True,
"infer_section_parameters": True,
"infer_path_parameters": True,
"input": "INPUT",
"input_files": "eam.h0",
"input_subdir": "INPUT_SUBDIR",
Expand Down Expand Up @@ -317,8 +317,8 @@ def test_subsections():
"fail_on_dependency_skip": False,
"frequency": "monthly",
"grid": "",
"guess_section_parameters": True,
"guess_path_parameters": True,
"infer_section_parameters": True,
"infer_path_parameters": True,
"input": "INPUT",
"input_component": "",
"input_files": "eam.h0",
Expand Down Expand Up @@ -358,8 +358,8 @@ def test_subsections():
"fail_on_dependency_skip": False,
"frequency": "monthly",
"grid": "",
"guess_section_parameters": True,
"guess_path_parameters": True,
"infer_section_parameters": True,
"infer_path_parameters": True,
"input": "INPUT",
"input_component": "",
"input_files": "eam.h0",
Expand Down Expand Up @@ -430,8 +430,8 @@ def test_subsections():
"fail_on_dependency_skip": False,
"frequency": "monthly",
"grid": "",
"guess_section_parameters": True,
"guess_path_parameters": True,
"infer_section_parameters": True,
"infer_path_parameters": True,
"input": "INPUT",
"input_component": "",
"input_files": "eam.h0",
Expand Down Expand Up @@ -468,8 +468,8 @@ def test_subsections():
"fail_on_dependency_skip": False,
"frequency": "monthly",
"grid": "",
"guess_section_parameters": True,
"guess_path_parameters": True,
"infer_section_parameters": True,
"infer_path_parameters": True,
"input": "INPUT",
"input_component": "",
"input_files": "eam.h0",
Expand Down
35 changes: 32 additions & 3 deletions tests/test_zppy_e3sm_diags.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,40 @@
add_ts_dependencies,
check_and_define_parameters,
check_mvm_only_parameters_for_bash,
check_parameter_defined,
check_parameters_for_bash,
check_set_specific_parameter,
)
from zppy.utils import ParameterNotProvidedError


def test_check_parameter_defined():
c = {"a": 1, "b": 2, "c": ""}
check_parameter_defined(c, "a")
with pytest.raises(ParameterNotProvidedError):
check_parameter_defined(c, "c")
with pytest.raises(ParameterNotProvidedError):
check_parameter_defined(c, "d")


def test_check_set_specific_parameter():
# Parameter is required
# a, b need parameter p, and we want sets a, b, c
c = {"sets": ["a", "b", "c"], "p": "exists"}
check_set_specific_parameter(c, set(["a", "b"]), "p")

# Parameter isn't required based on the sets we want
# z needs parameter p, but we only want sets a, b, c
c = {"sets": ["a", "b", "c"], "p": ""}
check_set_specific_parameter(c, set(["z"]), "p")

# Parameter is required
# a, b need parameter p, and we want sets a, b, c
c = {"sets": ["a", "b", "c"], "p": ""}
with pytest.raises(ParameterNotProvidedError):
check_set_specific_parameter(c, set(["a", "b"]), "p")


def test_check_parameters_for_bash():
# diurnal_cycle
c = {"sets": ["diurnal_cycle"], "climo_diurnal_frequency": "diurnal_8xdaily"}
Expand Down Expand Up @@ -191,9 +220,9 @@ def test_check_mvm_only_parameters_for_bash():


def test_check_and_define_parameters():
# test_zppy_utils.py tests the guessing functionality turned off.
# test_zppy_utils.py tests the inference functionality turned off.
# So, we'll only test it turned on here.
guesses = {"guess_path_parameters": True, "guess_section_parameters": True}
inferences = {"infer_path_parameters": True, "infer_section_parameters": True}
prefix_requirements = {
"subsection": "sub",
"tag": "tag",
Expand All @@ -203,7 +232,7 @@ def test_check_and_define_parameters():
"ref_year2": 1990,
}
base: Dict[str, Any] = {"diagnostics_base_path": "diags/post"}
base.update(guesses)
base.update(inferences)
base.update(prefix_requirements)

mvm_base = dict()
Expand Down
6 changes: 3 additions & 3 deletions tests/test_zppy_ilamb.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ def test_determine_and_add_dependencies():
]
assert dependencies == expected

# Have zppy guess the subsection names
# Have zppy infer the subsection names
c = {
"e3sm_to_cmip_atm_subsection": "",
"e3sm_to_cmip_land_subsection": "",
Expand All @@ -32,8 +32,8 @@ def test_determine_and_add_dependencies():
"year1": 1980,
"year2": 1990,
"ts_num_years": 5,
"guess_path_parameters": True,
"guess_section_parameters": True,
"infer_path_parameters": True,
"infer_section_parameters": True,
}
dependencies = []
determine_and_add_dependencies(c, dependencies, "script_dir")
Expand Down
Loading
Loading