Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
48 changes: 48 additions & 0 deletions tests/test_sections.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,13 @@ def test_sections():
"reservation": "",
"qos": "regular",
"templateDir": "zppy/templates",
"ts_atm_grid": "180x360_aave",
"ts_atm_subsection": "",
"ts_grid": "180x360_aave",
"ts_land_grid": "180x360_aave",
"ts_land_subsection": "",
"ts_num_years": 5,
"ts_subsection": "",
"vars": "FSNTOA,FLUT,FSNT,FLNT,FSNS,FLNS,SHFLX,QFLX,TAUX,TAUY,PRECC,PRECL,PRECSC,PRECSL,TS,TREFHT,CLDTOT,CLDHGH,CLDMED,CLDLOW,U",
"walltime": "02:00:00",
"www": "WWWW",
Expand Down Expand Up @@ -149,7 +155,13 @@ def test_sections():
"subsection": None,
"templateDir": "zppy/templates",
"tpd": 1,
"ts_atm_grid": "180x360_aave",
"ts_atm_subsection": "",
"ts_grid": "180x360_aave",
"ts_land_grid": "180x360_aave",
"ts_land_subsection": "",
"ts_num_years": 5,
"ts_subsection": "",
"vars": "FSNTOA,FLUT,FSNT,FLNT,FSNS,FLNS,SHFLX,QFLX,PRECC,PRECL,PRECSC,PRECSL,TS,TREFHT",
"walltime": "02:00:00",
"www": "WWWW",
Expand Down Expand Up @@ -204,7 +216,13 @@ def test_sections():
"reservation": "",
"subsection": None,
"templateDir": "zppy/templates",
"ts_atm_grid": "180x360_aave",
"ts_atm_subsection": "",
"ts_grid": "180x360_aave",
"ts_land_grid": "180x360_aave",
"ts_land_subsection": "",
"ts_num_years": 5,
"ts_subsection": "",
"vars": "",
"walltime": "02:00:00",
"www": "WWWW",
Expand Down Expand Up @@ -259,7 +277,13 @@ def test_subsections():
"qos": "regular",
"reservation": "",
"templateDir": "zppy/templates",
"ts_atm_grid": "180x360_aave",
"ts_atm_subsection": "",
"ts_grid": "180x360_aave",
"ts_land_grid": "180x360_aave",
"ts_land_subsection": "",
"ts_num_years": 5,
"ts_subsection": "",
"vars": "FSNTOA,FLUT,FSNT,FLNT,FSNS,FLNS,SHFLX,QFLX,TAUX,TAUY,PRECC,PRECL,PRECSC,PRECSL,TS,TREFHT,CLDTOT,CLDHGH,CLDMED,CLDLOW,U",
"walltime": "02:00:00",
"www": "WWWW",
Expand Down Expand Up @@ -334,7 +358,13 @@ def test_subsections():
"subsection": "ts_grid1",
"templateDir": "zppy/templates",
"tpd": 1,
"ts_atm_grid": "180x360_aave",
"ts_atm_subsection": "",
"ts_grid": "180x360_aave",
"ts_land_grid": "180x360_aave",
"ts_land_subsection": "",
"ts_num_years": 5,
"ts_subsection": "",
"vars": "FSNTOA,FLUT,FSNT,FLNT,FSNS,FLNS,SHFLX,QFLX,PRECC,PRECL,PRECSC,PRECSL,TS,TREFHT",
"walltime": "02:00:00",
"www": "WWWW",
Expand Down Expand Up @@ -375,7 +405,13 @@ def test_subsections():
"subsection": "ts_grid2",
"templateDir": "zppy/templates",
"tpd": 1,
"ts_atm_grid": "180x360_aave",
"ts_atm_subsection": "",
"ts_grid": "180x360_aave",
"ts_land_grid": "180x360_aave",
"ts_land_subsection": "",
"ts_num_years": 5,
"ts_subsection": "",
"vars": "FSNTOA,FLUT,FSNT,FLNT,FSNS,FLNS,SHFLX,QFLX,PRECC,PRECL,PRECSC,PRECSL,TS,TREFHT",
"walltime": "02:00:00",
"www": "WWWW",
Expand Down Expand Up @@ -446,7 +482,13 @@ def test_subsections():
"reservation": "",
"subsection": "climo_grid1",
"templateDir": "zppy/templates",
"ts_atm_grid": "180x360_aave",
"ts_atm_subsection": "",
"ts_grid": "180x360_aave",
"ts_land_grid": "180x360_aave",
"ts_land_subsection": "",
"ts_num_years": 5,
"ts_subsection": "",
"vars": "",
"walltime": "02:00:00",
"www": "WWWW",
Expand Down Expand Up @@ -484,7 +526,13 @@ def test_subsections():
"qos": "regular",
"subsection": "climo_grid2",
"templateDir": "zppy/templates",
"ts_atm_grid": "180x360_aave",
"ts_atm_subsection": "",
"ts_grid": "180x360_aave",
"ts_land_grid": "180x360_aave",
"ts_land_subsection": "",
"ts_num_years": 5,
"ts_subsection": "",
"vars": "",
"walltime": "02:00:00",
"www": "WWWW",
Expand Down
128 changes: 128 additions & 0 deletions tests/test_zppy_e3sm_to_cmip.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
import pytest

from zppy.e3sm_to_cmip import check_and_define_parameters, check_parameters_for_bash
from zppy.utils import ParameterNotProvidedError


def test_check_parameters_for_bash():
c = {"ts_grid": "grid"}
check_parameters_for_bash(c)

c = {"ts_grid": ""}
with pytest.raises(ParameterNotProvidedError):
check_parameters_for_bash(c)

c = {"component": "atm", "ts_atm_grid": "atm_grid"}
check_parameters_for_bash(c)
c = {"component": "lnd", "ts_land_grid": "land_grid"}
check_parameters_for_bash(c)

c = {"component": "atm", "ts_land_grid": "land_grid"}
with pytest.raises(ParameterNotProvidedError):
check_parameters_for_bash(c)
c = {"component": "lnd", "ts_atm_grid": "atm_grid"}
with pytest.raises(ParameterNotProvidedError):
check_parameters_for_bash(c)

c = {"ts_atm_grid": "atm_grid"}
with pytest.raises(ParameterNotProvidedError):
check_parameters_for_bash(c)
c = {"ts_land_grid": "land_grid"}
with pytest.raises(ParameterNotProvidedError):
check_parameters_for_bash(c)


def test_check_and_define_parameters():
sub = "name_of_this_subsection"

# Guess the subsection
c = {"ts_subsection": "subsection", "guess_section_parameters": True}
check_and_define_parameters(c, sub)
assert c["ts_subsection"] == "subsection"

c = {"ts_subsection": "", "guess_section_parameters": True}
check_and_define_parameters(c, sub)
assert c["ts_subsection"] == "name_of_this_subsection"

c = {
"component": "atm",
"ts_atm_subsection": "atm_subsection",
"guess_section_parameters": True,
}
check_and_define_parameters(c, sub)
assert c["ts_subsection"] == "atm_subsection"
c = {
"component": "lnd",
"ts_land_subsection": "land_subsection",
"guess_section_parameters": True,
}
check_and_define_parameters(c, sub)
assert c["ts_subsection"] == "land_subsection"

c = {
"component": "atm",
"ts_land_subsection": "land_subsection",
"guess_section_parameters": True,
}
check_and_define_parameters(c, sub)
assert c["ts_subsection"] == "name_of_this_subsection"
c = {
"component": "lnd",
"ts_atm_subsection": "atm_subsection",
"guess_section_parameters": True,
}
check_and_define_parameters(c, sub)
assert c["ts_subsection"] == "name_of_this_subsection"

c = {"ts_atm_subsection": "atm_subsection", "guess_section_parameters": True}
check_and_define_parameters(c, sub)
assert c["ts_subsection"] == "name_of_this_subsection"
c = {"ts_land_subsection": "land_subsection", "guess_section_parameters": True}
check_and_define_parameters(c, sub)
assert c["ts_subsection"] == "name_of_this_subsection"

# Don't guess the subsection
c = {"ts_subsection": "subsection", "guess_section_parameters": False}
check_and_define_parameters(c, sub)
assert c["ts_subsection"] == "subsection"

c = {"ts_subsection": "", "guess_section_parameters": False}
with pytest.raises(ParameterNotProvidedError):
check_and_define_parameters(c, sub)

c = {
"component": "atm",
"ts_atm_subsection": "atm_subsection",
"guess_section_parameters": False,
}
with pytest.raises(ParameterNotProvidedError):
check_and_define_parameters(c, sub)
c = {
"component": "lnd",
"ts_land_subsection": "land_subsection",
"guess_section_parameters": False,
}
with pytest.raises(ParameterNotProvidedError):
check_and_define_parameters(c, sub)

c = {
"component": "atm",
"ts_land_subsection": "land_subsection",
"guess_section_parameters": False,
}
with pytest.raises(ParameterNotProvidedError):
check_and_define_parameters(c, sub)
c = {
"component": "lnd",
"ts_atm_subsection": "atm_subsection",
"guess_section_parameters": False,
}
with pytest.raises(ParameterNotProvidedError):
check_and_define_parameters(c, sub)

c = {"ts_atm_subsection": "atm_subsection", "guess_section_parameters": False}
with pytest.raises(ParameterNotProvidedError):
check_and_define_parameters(c, sub)
c = {"ts_land_subsection": "land_subsection", "guess_section_parameters": False}
with pytest.raises(ParameterNotProvidedError):
check_and_define_parameters(c, sub)
34 changes: 20 additions & 14 deletions zppy/defaults/default.ini
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,28 @@ plugins = force_list(default=list())
qos = string(default="regular")
# Reservation -- if you have access to a node reservation, specify it with this parameter.
reservation = string(default="")
# Use for e3sm_to_cmip and/or ilamb tasks.
# Name of the grid used by the relevant `[ts]` atm subtask
ts_atm_grid = string(default="180x360_aave")
# Use for e3sm_to_cmip and/or ilamb tasks.
# Name of the `[ts]` atm subtask to depend on
ts_atm_subsection = string(default="")
# Use for e3sm_to_cmip task (but NOT the ilamb task) -- you can either set this, or
# both ts_atm_grid and ts_land_grid
# Name of the grid used by the relevant `[ts]` task
ts_grid = string(default="180x360_aave")
# Use for e3sm_to_cmip and/or ilamb tasks.
# Name of the grid used by the relevant `[ts]` land subtask
ts_land_grid = string(default="180x360_aave")
# Use for e3sm_to_cmip and/or ilamb tasks.
# Name of the `[ts]` land subtask to depend on
ts_land_subsection = string(default="")
Comment on lines +60 to +75
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

From what I understand, you're moving these parameters up a level in the configuration hierarchy so that they can be reused across each e3sm_to_cmip subtask, rather than configuring ts_grid differently for each subtask. The intention is to eliminate redundant code/configuration.

Is this correct? If so, it sounds reasonable to me.

# The years increment from `years` in `[ts]`
ts_num_years = integer(default=5)
# Use for e3sm_to_cmip task (but NOT the ilamb task) -- you can either set this, or
# both ts_atm_subsection and ts_land_subsection
# Name of the `[ts]` subtask to depend on
ts_subsection = string(default="")
# scriptDir -- NOTE: this parameter is created internally
# templateDir -- NOTE: this parameter is created internally
# The variables to process
Expand Down Expand Up @@ -123,17 +143,11 @@ cmip_metadata = string(default="inclusions/e3sm_to_cmip/default_metadata.json")
cmip_vars = string(default="")
# Model component having generated input files (eam, eamxx, elm, mosart, ...)
input_component = string(default="")
# Name of the grid used by the relevant `[ts]` task
ts_grid = string(default="180x360_aave")
# Name of the `[ts]` subtask to depend on
ts_subsection = string(default="")

[[__many__]]
cmip_metadata = string(default=None)
cmip_vars = string(default=None)
input_component = string(default=None)
ts_grid = string(default=None)
ts_subsection = string(default=None)

[tc_analysis]
# NOTE: always overrides value in [default]
Expand Down Expand Up @@ -339,11 +353,3 @@ e3sm_to_cmip_land_subsection = string(default="")
ilamb_obs = string(default="")
# for land_only run
land_only = boolean(default=False)
# Name of the grid used by the relevant `[ts]` atm subtask
ts_atm_grid = string(default="180x360_aave")
# Name of the `[ts]` atm subtask to depend on
ts_atm_subsection = string(default="")
# Name of the grid used by the relevant `[ts]` land subtask
ts_land_grid = string(default="180x360_aave")
# Name of the `[ts]` land subtask to depend on
ts_land_subsection = string(default="")
42 changes: 39 additions & 3 deletions zppy/e3sm_to_cmip.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,12 @@
from zppy.bundle import handle_bundles
from zppy.utils import (
ParameterGuessType,
ParameterNotProvidedError,
add_dependencies,
check_status,
define_or_guess,
define_or_guess2,
get_file_names,
get_guess_type_parameter,
get_tasks,
get_years,
initialize_template,
Expand All @@ -34,6 +35,7 @@ def e3sm_to_cmip(config: ConfigObj, script_dir: str, existing_bundles, job_ids_f
for c in tasks:
dependencies: List[str] = []
set_component_and_prc_typ(c)
check_parameters_for_bash(c)
c["cmor_tables_prefix"] = c["diagnostics_base_path"]
year_sets: List[Tuple[int, int]] = get_years(c["years"])
# Loop over year sets
Expand Down Expand Up @@ -68,8 +70,7 @@ def e3sm_to_cmip(config: ConfigObj, script_dir: str, existing_bundles, job_ids_f
with open(bash_file, "w") as f:
f.write(template.render(**c))
make_executable(bash_file)
# Default to the name of this task if ts_subsection is not defined
define_or_guess2(c, "ts_subsection", sub, ParameterGuessType.SECTION_GUESS)
check_and_define_parameters(c, sub)
add_dependencies(
dependencies,
script_dir,
Expand Down Expand Up @@ -106,3 +107,38 @@ def e3sm_to_cmip(config: ConfigObj, script_dir: str, existing_bundles, job_ids_f
print(f" environment_commands={c['environment_commands']}")

return existing_bundles


def check_parameters_for_bash(c: Dict[str, Any]) -> None:
# Check parameters that aren't used until e3sm_diags.bash is run
parameter = "ts_grid"
if (parameter not in c.keys()) or (c[parameter] == ""):
if "component" in c.keys():
if (c["component"] == "atm") and ("ts_atm_grid" in c.keys()):
c[parameter] = c["ts_atm_grid"]
elif (c["component"] == "lnd") and ("ts_land_grid" in c.keys()):
c[parameter] = c["ts_land_grid"]
else:
raise ParameterNotProvidedError(parameter)
else:
raise ParameterNotProvidedError(parameter)


def check_and_define_parameters(c: Dict[str, Any], sub: str) -> None:
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

The reasons for the different function are:

  1. ts_grid is used in the bash file whereas ts_subsection is used in the python file.
  2. We will only try to guess ts_subsection if the guess parameter is on. The reason for this is somewhat historical -- when I did the parameter refactoring, I created parameters to turn on/off guessing for file paths or zppy cfg sections.
class ParameterGuessType(Enum):
    PATH_GUESS = 1
    SECTION_GUESS = 2

grid was always used in the bash files using exactly what the user set it as, or else it was computed from the mapping file. This PR appears to be the first time we're allowing a grid parameter to be set by an alternative set of parameters.

def set_grid(c: Dict[str, Any]) -> None:
    # Grid name (if not explicitly defined)
    #   'native' if no remapping
    #   or extracted from mapping filename
    if c["grid"] == "":
        if c["mapping_file"] == "":
            c["grid"] = "native"
        elif c["mapping_file"] == "glb":
            c["grid"] = "glb"
        else:
            tmp = os.path.basename(c["mapping_file"])
            # FIXME: W605 invalid escape sequence '\.'
            tmp = re.sub("\.[^.]*\.nc$", "", tmp)  # noqa: W605
            tmp = tmp.split("_")
            if tmp[0] == "map":
                c["grid"] = f"{tmp[-2]}_{tmp[-1]}"
            else:
                raise ValueError(
                    f"Cannot extract target grid name from mapping file {c['mapping_file']}"
                )
    # If grid is defined, just use that

parameter = "ts_subsection"
if (parameter not in c.keys()) or (c[parameter] == ""):
guess_type_parameter: str = get_guess_type_parameter(
ParameterGuessType.SECTION_GUESS
)
if c[guess_type_parameter]:
if "component" in c.keys():
if (c["component"] == "atm") and ("ts_atm_subsection" in c.keys()):
c[parameter] = c["ts_atm_subsection"]
elif (c["component"] == "lnd") and ("ts_land_subsection" in c.keys()):
c[parameter] = c["ts_land_subsection"]
else:
c[parameter] = sub
else:
c[parameter] = sub
else:
raise ParameterNotProvidedError(parameter)