From eb104e7c28eef514e055cb60d3c699db1c26c417 Mon Sep 17 00:00:00 2001 From: Paul Madden Date: Wed, 30 Oct 2024 23:42:08 +0000 Subject: [PATCH 01/10] Require control_file in UPP config --- src/uwtools/resources/jsonschema/upp.jsonschema | 4 ++++ src/uwtools/tests/drivers/test_upp.py | 1 + src/uwtools/tests/test_schemas.py | 9 ++++++++- 3 files changed, 13 insertions(+), 1 deletion(-) diff --git a/src/uwtools/resources/jsonschema/upp.jsonschema b/src/uwtools/resources/jsonschema/upp.jsonschema index ff2ea2a67..811fd8ea0 100644 --- a/src/uwtools/resources/jsonschema/upp.jsonschema +++ b/src/uwtools/resources/jsonschema/upp.jsonschema @@ -3,6 +3,9 @@ "upp": { "additionalProperties": false, "properties": { + "control_file": { + "type": "string" + }, "execution": { "$ref": "urn:uwtools:execution-parallel" }, @@ -187,6 +190,7 @@ } }, "required": [ + "control_file", "execution", "namelist", "rundir" diff --git a/src/uwtools/tests/drivers/test_upp.py b/src/uwtools/tests/drivers/test_upp.py index c906ffbd4..85dcc8728 100644 --- a/src/uwtools/tests/drivers/test_upp.py +++ b/src/uwtools/tests/drivers/test_upp.py @@ -25,6 +25,7 @@ def config(tmp_path): return { "upp": { + "control_file": "/path/to/postxconfig-NT.txt", "execution": { "batchargs": { "cores": 1, diff --git a/src/uwtools/tests/test_schemas.py b/src/uwtools/tests/test_schemas.py index 951d6447d..0e07eb559 100644 --- a/src/uwtools/tests/test_schemas.py +++ b/src/uwtools/tests/test_schemas.py @@ -1952,6 +1952,7 @@ def test_schema_ungrib_rundir(ungrib_prop): def test_schema_upp(): config = { + "control_file": "/path/to/postxconfig-NT.txt", "execution": { "batchargs": { "cores": 1, @@ -1977,7 +1978,7 @@ def test_schema_upp(): # Basic correctness: assert not errors(config) # Some top-level keys are required: - for key in ("execution", "namelist", "rundir"): + for key in ("control_file", "execution", "namelist", "rundir"): assert f"'{key}' is a required property" in errors(with_del(config, key)) # Other top-level keys are optional: assert not errors({**config, "files_to_copy": {"dst": "src"}}) @@ -1986,6 +1987,12 @@ def test_schema_upp(): assert "Additional properties are not allowed" in errors({**config, "foo": "bar"}) +def test_schema_upp_control_file(upp_prop): + errors = upp_prop("control_file") + # A string value is required: + assert "is not of type 'string'" in errors(None) + + def test_schema_upp_namelist(upp_prop): maxpathlen = 256 errors = upp_prop("namelist") From 857100d9a92c3ca4ccd0888a03fcec2a387c3d10 Mon Sep 17 00:00:00 2001 From: Paul Madden Date: Wed, 30 Oct 2024 23:43:15 +0000 Subject: [PATCH 02/10] Update upp.yaml --- docs/shared/upp.yaml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/docs/shared/upp.yaml b/docs/shared/upp.yaml index 769212bad..96ad01083 100644 --- a/docs/shared/upp.yaml +++ b/docs/shared/upp.yaml @@ -1,4 +1,5 @@ upp: + control_file: /path/to/postxconfig-NT.txt execution: batchargs: export: NONE @@ -12,8 +13,6 @@ upp: mpiargs: - "--ntasks $SLURM_CPUS_ON_NODE" mpicmd: srun - files_to_copy: - postxconfig-NT.txt: /path/to/postxconfig-NT.txt files_to_link: eta_micro_lookup.dat: /path/to/nam_micro_lookup.dat params_grib2_tbl_new: /path/to/params_grib2_tbl_new From 1ff2561e8e9c508e0499c0cb1879d9fde1ea5bef Mon Sep 17 00:00:00 2001 From: Paul Madden Date: Wed, 30 Oct 2024 23:47:48 +0000 Subject: [PATCH 03/10] Doc update --- docs/sections/user_guide/cli/drivers/upp/show-schema.out | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/sections/user_guide/cli/drivers/upp/show-schema.out b/docs/sections/user_guide/cli/drivers/upp/show-schema.out index 697354a77..67517e149 100644 --- a/docs/sections/user_guide/cli/drivers/upp/show-schema.out +++ b/docs/sections/user_guide/cli/drivers/upp/show-schema.out @@ -3,6 +3,9 @@ "upp": { "additionalProperties": false, "properties": { + "control_file": { + "type": "string" + }, "execution": { "additionalProperties": false, "properties": { @@ -15,6 +18,3 @@ "debug": { "type": "boolean" }, - "exclusive": { - "type": "boolean" - }, From 7d2fd9a925006fbce46686945f2848daa1fcdde9 Mon Sep 17 00:00:00 2001 From: Paul Madden Date: Wed, 30 Oct 2024 23:54:30 +0000 Subject: [PATCH 04/10] Add control_file @tasks method --- src/uwtools/drivers/upp.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/uwtools/drivers/upp.py b/src/uwtools/drivers/upp.py index 60ff373af..c16808182 100644 --- a/src/uwtools/drivers/upp.py +++ b/src/uwtools/drivers/upp.py @@ -20,6 +20,16 @@ class UPP(DriverCycleLeadtimeBased): # Workflow tasks + @tasks + def control_file(self): + """ + The GRIB control file. + """ + yield self.taskname("GRIB control file") + yield filecopy( + src=Path(self.config["control_file"]), dst=self.rundir / "postxconfig-NT.txt" + ) + @tasks def files_copied(self): """ @@ -66,6 +76,7 @@ def provisioned_rundir(self): """ yield self.taskname("provisioned run directory") yield [ + self.control_file(), self.files_copied(), self.files_linked(), self.namelist_file(), From 2acf2dddd035d1c83a9cc1550d08a3b4eb869266 Mon Sep 17 00:00:00 2001 From: Paul Madden Date: Thu, 31 Oct 2024 16:53:59 +0000 Subject: [PATCH 05/10] Work on output() implementation --- src/uwtools/drivers/driver.py | 4 +++- src/uwtools/drivers/upp.py | 38 ++++++++++++++++++++++++++++++++++- 2 files changed, 40 insertions(+), 2 deletions(-) diff --git a/src/uwtools/drivers/driver.py b/src/uwtools/drivers/driver.py index 756479b74..107fd800a 100644 --- a/src/uwtools/drivers/driver.py +++ b/src/uwtools/drivers/driver.py @@ -33,6 +33,8 @@ from uwtools.utils.file import writable from uwtools.utils.processing import run_shell_cmd +OutputT = dict[str, Union[str, list[str]]] + # NB: Class docstrings are programmatically defined. @@ -428,7 +430,7 @@ def _run_via_local_execution(self): # Public methods @property - def output(self) -> dict[str, Union[str, list[str]]]: + def output(self) -> OutputT: """ Returns a description of the file(s) created when this component runs. """ diff --git a/src/uwtools/drivers/upp.py b/src/uwtools/drivers/upp.py index c16808182..770c957e4 100644 --- a/src/uwtools/drivers/upp.py +++ b/src/uwtools/drivers/upp.py @@ -2,13 +2,15 @@ A driver for UPP. """ +import math from pathlib import Path from iotaa import asset, task, tasks from uwtools.config.formats.nml import NMLConfig -from uwtools.drivers.driver import DriverCycleLeadtimeBased +from uwtools.drivers.driver import DriverCycleLeadtimeBased, OutputT from uwtools.drivers.support import set_driver_docstring +from uwtools.exceptions import UWConfigError from uwtools.strings import STR from uwtools.utils.tasks import file, filecopy, symlink @@ -92,6 +94,40 @@ def driver_name(cls) -> str: """ return STR.upp + @property + def output(self) -> OutputT: + """ + Returns a description of the file(s) created when this component runs. + """ + # Define facts specific to the supported UPP version. + FIELDS_PER_BLOCK = 16 + GEN_PROC_TYPE_IDX = 8 + PARAMS_PER_VAR = 42 + # Derive values from the current driver config. GRIB output filename suffixes include the + # forecast leadtime, zero-padded to at least 2 digits (more if necessary). Avoid taking the + # log of zero. + cf = self.config["control_file"] + leadtime = int(self.leadtime.total_seconds() / 3600) + suffix = ".GrbF%0{}d".format(max(2, int(math.log10(leadtime or 1)) + 1)) % leadtime + # Read the control file into an array of lines. Get the number of blocks (one per output + # GRIB file) and the number of variables per block. For each block, construct a filename + # from the block's identifier and the suffix defined above. + try: + with open(cf, "r", encoding="utf-8") as f: + cfg = f.read().split("\n") + except (FileNotFoundError, PermissionError) as e: + raise UWConfigError(f"Could not open UPP control file {cf}") from e + nblocks, cfg = int(cfg[0]), cfg[1:] + nvars, cfg = list(map(int, cfg[:nblocks])), cfg[nblocks:] + paths = [] + for _ in range(nblocks): + identifier = cfg[0] + paths.append(str(self.rundir / (identifier + suffix))) + fields, cfg = cfg[:FIELDS_PER_BLOCK], cfg[FIELDS_PER_BLOCK:] + _, cfg = (cfg[0], cfg[1:]) if fields[GEN_PROC_TYPE_IDX] == "ens_fcst" else (None, cfg) + cfg = cfg[PARAMS_PER_VAR * nvars.pop() :] + return {"gribfiles": paths} + # Private helper methods @property From c5b9c056b9df464c3fd88b00b988bc1376321036 Mon Sep 17 00:00:00 2001 From: Paul Madden Date: Thu, 31 Oct 2024 18:27:24 +0000 Subject: [PATCH 06/10] Unit tests @ 100% --- src/uwtools/drivers/upp.py | 22 ++++++++++-------- src/uwtools/tests/drivers/test_upp.py | 33 +++++++++++++++++++++++---- 2 files changed, 41 insertions(+), 14 deletions(-) diff --git a/src/uwtools/drivers/upp.py b/src/uwtools/drivers/upp.py index 770c957e4..268d92bc2 100644 --- a/src/uwtools/drivers/upp.py +++ b/src/uwtools/drivers/upp.py @@ -2,7 +2,7 @@ A driver for UPP. """ -import math +from math import log10 from pathlib import Path from iotaa import asset, task, tasks @@ -20,6 +20,12 @@ class UPP(DriverCycleLeadtimeBased): A driver for UPP. """ + # Facts specific to the supported UPP version: + + FIELDS_PER_BLOCK = 16 + GEN_PROC_TYPE_IDX = 8 + PARAMS_PER_VAR = 42 + # Workflow tasks @tasks @@ -99,16 +105,12 @@ def output(self) -> OutputT: """ Returns a description of the file(s) created when this component runs. """ - # Define facts specific to the supported UPP version. - FIELDS_PER_BLOCK = 16 - GEN_PROC_TYPE_IDX = 8 - PARAMS_PER_VAR = 42 # Derive values from the current driver config. GRIB output filename suffixes include the # forecast leadtime, zero-padded to at least 2 digits (more if necessary). Avoid taking the # log of zero. cf = self.config["control_file"] leadtime = int(self.leadtime.total_seconds() / 3600) - suffix = ".GrbF%0{}d".format(max(2, int(math.log10(leadtime or 1)) + 1)) % leadtime + suffix = ".GrbF%0{}d".format(max(2, int(log10(leadtime or 1)) + 1)) % leadtime # Read the control file into an array of lines. Get the number of blocks (one per output # GRIB file) and the number of variables per block. For each block, construct a filename # from the block's identifier and the suffix defined above. @@ -123,9 +125,11 @@ def output(self) -> OutputT: for _ in range(nblocks): identifier = cfg[0] paths.append(str(self.rundir / (identifier + suffix))) - fields, cfg = cfg[:FIELDS_PER_BLOCK], cfg[FIELDS_PER_BLOCK:] - _, cfg = (cfg[0], cfg[1:]) if fields[GEN_PROC_TYPE_IDX] == "ens_fcst" else (None, cfg) - cfg = cfg[PARAMS_PER_VAR * nvars.pop() :] + fields, cfg = cfg[: self.FIELDS_PER_BLOCK], cfg[self.FIELDS_PER_BLOCK :] + _, cfg = ( + (cfg[0], cfg[1:]) if fields[self.GEN_PROC_TYPE_IDX] == "ens_fcst" else (None, cfg) + ) + cfg = cfg[self.PARAMS_PER_VAR * nvars.pop() :] return {"gribfiles": paths} # Private helper methods diff --git a/src/uwtools/tests/drivers/test_upp.py b/src/uwtools/tests/drivers/test_upp.py index 85dcc8728..5afade73e 100644 --- a/src/uwtools/tests/drivers/test_upp.py +++ b/src/uwtools/tests/drivers/test_upp.py @@ -14,7 +14,7 @@ from uwtools.drivers.driver import Driver from uwtools.drivers.upp import UPP -from uwtools.exceptions import UWNotImplementedError +from uwtools.exceptions import UWConfigError from uwtools.logging import log from uwtools.tests.support import logged, regex_logged @@ -91,7 +91,6 @@ def leadtime(): "_scheduler", "_validate", "_write_runscript", - "output", "run", "runscript", ], @@ -161,10 +160,34 @@ def test_UPP_namelist_file_missing_base_file(caplog, driverobj): assert regex_logged(caplog, "missing.nml: State: Not Ready (external asset)") -def test_UPP_output(driverobj): - with raises(UWNotImplementedError) as e: +def test_UPP_output(driverobj, tmp_path): + fields = ["?"] * (UPP.FIELDS_PER_BLOCK - 1) + parameters = ["?"] * UPP.PARAMS_PER_VAR + # fmt: off + control_data = [ + "2", # number of blocks + "1", # number variables in 2nd block + "2", # number variables in 1st block + "FOO", # identifier of 1st block + *fields, # fields of 1st block + *(parameters * 2) , # variable parameters of 1st block + "BAR", # identifier of 2nd block + *fields, # fields of 2nd block + *parameters, # variable parameters of 2nd block + ] + # fmt: on + control_file = tmp_path / "postxconfig-NT.txt" + with open(control_file, "w", encoding="utf-8") as f: + print("\n".join(control_data), file=f) + driverobj._config["control_file"] = str(control_file) + expected = {"gribfiles": [str(driverobj.rundir / ("%s.GrbF24" % x)) for x in ("FOO", "BAR")]} + assert driverobj.output == expected + + +def test_UPP_output_fail(driverobj): + with raises(UWConfigError) as e: assert driverobj.output - assert str(e.value) == "The output() method is not yet implemented for this driver" + assert str(e.value) == "Could not open UPP control file %s" % driverobj.config["control_file"] def test_UPP_provisioned_rundir(driverobj): From 17e010cd5e36618b1b0f91a28e8aba44c1f4fd18 Mon Sep 17 00:00:00 2001 From: Paul Madden Date: Thu, 31 Oct 2024 18:29:54 +0000 Subject: [PATCH 07/10] WIP --- src/uwtools/drivers/upp.py | 14 ++++++-------- src/uwtools/tests/drivers/test_upp.py | 4 ++-- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/src/uwtools/drivers/upp.py b/src/uwtools/drivers/upp.py index 268d92bc2..e62abb975 100644 --- a/src/uwtools/drivers/upp.py +++ b/src/uwtools/drivers/upp.py @@ -22,9 +22,9 @@ class UPP(DriverCycleLeadtimeBased): # Facts specific to the supported UPP version: - FIELDS_PER_BLOCK = 16 - GEN_PROC_TYPE_IDX = 8 - PARAMS_PER_VAR = 42 + FIELDS = 16 + GENPROCTYPE = 8 + PARAMS = 42 # Workflow tasks @@ -125,11 +125,9 @@ def output(self) -> OutputT: for _ in range(nblocks): identifier = cfg[0] paths.append(str(self.rundir / (identifier + suffix))) - fields, cfg = cfg[: self.FIELDS_PER_BLOCK], cfg[self.FIELDS_PER_BLOCK :] - _, cfg = ( - (cfg[0], cfg[1:]) if fields[self.GEN_PROC_TYPE_IDX] == "ens_fcst" else (None, cfg) - ) - cfg = cfg[self.PARAMS_PER_VAR * nvars.pop() :] + fields, cfg = cfg[: self.FIELDS], cfg[self.FIELDS :] + _, cfg = (cfg[0], cfg[1:]) if fields[self.GENPROCTYPE] == "ens_fcst" else (None, cfg) + cfg = cfg[self.PARAMS * nvars.pop() :] return {"gribfiles": paths} # Private helper methods diff --git a/src/uwtools/tests/drivers/test_upp.py b/src/uwtools/tests/drivers/test_upp.py index 5afade73e..55e4af63b 100644 --- a/src/uwtools/tests/drivers/test_upp.py +++ b/src/uwtools/tests/drivers/test_upp.py @@ -161,8 +161,8 @@ def test_UPP_namelist_file_missing_base_file(caplog, driverobj): def test_UPP_output(driverobj, tmp_path): - fields = ["?"] * (UPP.FIELDS_PER_BLOCK - 1) - parameters = ["?"] * UPP.PARAMS_PER_VAR + fields = ["?"] * (UPP.FIELDS - 1) + parameters = ["?"] * UPP.PARAMS # fmt: off control_data = [ "2", # number of blocks From acca926b4e8508a0f092124d5da3bca3601d0fcb Mon Sep 17 00:00:00 2001 From: Paul Madden Date: Thu, 31 Oct 2024 18:42:10 +0000 Subject: [PATCH 08/10] Handle exception in show_output task method --- src/uwtools/drivers/driver.py | 5 ++++- src/uwtools/tests/drivers/test_driver.py | 7 +++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/src/uwtools/drivers/driver.py b/src/uwtools/drivers/driver.py index 107fd800a..dfdf80d44 100644 --- a/src/uwtools/drivers/driver.py +++ b/src/uwtools/drivers/driver.py @@ -401,7 +401,10 @@ def show_output(self): Show the output to be created by this component. """ yield self.taskname("expected output") - print(json.dumps(self.output, indent=2, sort_keys=True)) + try: + print(json.dumps(self.output, indent=2, sort_keys=True)) + except UWConfigError as e: + log.error(e) yield asset(None, lambda: True) @task diff --git a/src/uwtools/tests/drivers/test_driver.py b/src/uwtools/tests/drivers/test_driver.py index 25dd0f625..9f33c2961 100644 --- a/src/uwtools/tests/drivers/test_driver.py +++ b/src/uwtools/tests/drivers/test_driver.py @@ -457,6 +457,13 @@ def test_driver_show_output(capsys, config): assert capsys.readouterr().out.strip() == dedent(expected).strip() +def test_driver_show_output_fail(caplog, config): + with patch.object(ConcreteDriverTimeInvariant, "output", new_callable=PropertyMock) as output: + output.side_effect = UWConfigError("FAIL") + ConcreteDriverTimeInvariant(config).show_output() + assert "FAIL" in caplog.messages + + @mark.parametrize( "base_file,update_values,expected", [ From 87932f8de9d61ddf2156421bbdef8da45f6886f9 Mon Sep 17 00:00:00 2001 From: Paul Madden Date: Thu, 31 Oct 2024 18:53:46 +0000 Subject: [PATCH 09/10] Updates --- src/uwtools/drivers/upp.py | 24 ++++++++++++++---------- src/uwtools/tests/drivers/test_upp.py | 4 ++-- 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/src/uwtools/drivers/upp.py b/src/uwtools/drivers/upp.py index e62abb975..8834ab9e9 100644 --- a/src/uwtools/drivers/upp.py +++ b/src/uwtools/drivers/upp.py @@ -22,9 +22,9 @@ class UPP(DriverCycleLeadtimeBased): # Facts specific to the supported UPP version: - FIELDS = 16 - GENPROCTYPE = 8 - PARAMS = 42 + GENPROCTYPE_IDX = 8 + NFIELDS = 16 + NPARAMS = 42 # Workflow tasks @@ -116,18 +116,22 @@ def output(self) -> OutputT: # from the block's identifier and the suffix defined above. try: with open(cf, "r", encoding="utf-8") as f: - cfg = f.read().split("\n") + lines = f.read().split("\n") except (FileNotFoundError, PermissionError) as e: raise UWConfigError(f"Could not open UPP control file {cf}") from e - nblocks, cfg = int(cfg[0]), cfg[1:] - nvars, cfg = list(map(int, cfg[:nblocks])), cfg[nblocks:] + nblocks, lines = int(lines[0]), lines[1:] + nvars, lines = list(map(int, lines[:nblocks])), lines[nblocks:] paths = [] for _ in range(nblocks): - identifier = cfg[0] + identifier = lines[0] paths.append(str(self.rundir / (identifier + suffix))) - fields, cfg = cfg[: self.FIELDS], cfg[self.FIELDS :] - _, cfg = (cfg[0], cfg[1:]) if fields[self.GENPROCTYPE] == "ens_fcst" else (None, cfg) - cfg = cfg[self.PARAMS * nvars.pop() :] + fields, lines = lines[: self.NFIELDS], lines[self.NFIELDS :] + _, lines = ( + (lines[0], lines[1:]) + if fields[self.GENPROCTYPE_IDX] == "ens_fcst" + else (None, lines) + ) + lines = lines[self.NPARAMS * nvars.pop() :] return {"gribfiles": paths} # Private helper methods diff --git a/src/uwtools/tests/drivers/test_upp.py b/src/uwtools/tests/drivers/test_upp.py index 55e4af63b..435fed7e0 100644 --- a/src/uwtools/tests/drivers/test_upp.py +++ b/src/uwtools/tests/drivers/test_upp.py @@ -161,8 +161,8 @@ def test_UPP_namelist_file_missing_base_file(caplog, driverobj): def test_UPP_output(driverobj, tmp_path): - fields = ["?"] * (UPP.FIELDS - 1) - parameters = ["?"] * UPP.PARAMS + fields = ["?"] * (UPP.NFIELDS - 1) + parameters = ["?"] * UPP.NPARAMS # fmt: off control_data = [ "2", # number of blocks From 70cef417b70c40e2189878bd91f909b1fe62911e Mon Sep 17 00:00:00 2001 From: Paul Madden Date: Thu, 31 Oct 2024 19:57:10 +0000 Subject: [PATCH 10/10] Improve test-data comment --- src/uwtools/tests/drivers/test_upp.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/uwtools/tests/drivers/test_upp.py b/src/uwtools/tests/drivers/test_upp.py index 435fed7e0..12a34dde7 100644 --- a/src/uwtools/tests/drivers/test_upp.py +++ b/src/uwtools/tests/drivers/test_upp.py @@ -168,12 +168,12 @@ def test_UPP_output(driverobj, tmp_path): "2", # number of blocks "1", # number variables in 2nd block "2", # number variables in 1st block - "FOO", # identifier of 1st block - *fields, # fields of 1st block - *(parameters * 2) , # variable parameters of 1st block - "BAR", # identifier of 2nd block - *fields, # fields of 2nd block - *parameters, # variable parameters of 2nd block + "FOO", # 1st block identifier + *fields, # 1st block fields + *(parameters * 2) , # 1st block variable parameters + "BAR", # 2nd block identifier + *fields, # 2nd block fields + *parameters, # 2nd block variable parameters ] # fmt: on control_file = tmp_path / "postxconfig-NT.txt"