From a70259d9125ba0a9808da715ae3617d8f4b89f48 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 24 Jun 2026 18:52:40 +0000 Subject: [PATCH 01/18] Implement inferred simboard www defaults --- tests/test_sections.py | 1 + tests/test_zppy_main.py | 93 +++++++++++++++++++++++++++++++++++++++ zppy/__main__.py | 14 ++++++ zppy/defaults/default.ini | 7 ++- 4 files changed, 113 insertions(+), 2 deletions(-) create mode 100644 tests/test_zppy_main.py diff --git a/tests/test_sections.py b/tests/test_sections.py index 8b34fd42..49a73b57 100644 --- a/tests/test_sections.py +++ b/tests/test_sections.py @@ -92,6 +92,7 @@ def test_sections(): "plugins": [], "reservation": "", "qos": "regular", + "simboard_type": "prod", "templateDir": "zppy/templates", "ts_atm_grid": "180x360_aave", "ts_atm_subsection": "", diff --git a/tests/test_zppy_main.py b/tests/test_zppy_main.py new file mode 100644 index 00000000..91b6deda --- /dev/null +++ b/tests/test_zppy_main.py @@ -0,0 +1,93 @@ +import configparser +from pathlib import Path +from typing import Any, Dict +from unittest.mock import MagicMock + +import pytest +from configobj import ConfigObj +from validate import Validator + +from zppy.__main__ import _determine_parameters + + +def _fake_machine_info() -> MagicMock: + config = configparser.ConfigParser() + config["e3sm_unified"] = {"base_path": "/unified"} + config["diagnostics"] = {"base_path": "/diagnostics"} + config["web_portal"] = { + "base_path": "/global/cfs/cdirs/e3sm/www", + "base_url": "https://portal.nersc.gov/cfs/e3sm", + } + machine_info = MagicMock() + machine_info.machine = "pm-cpu" + machine_info.config = config + machine_info.get_account_defaults.return_value = ("e3sm", "regular", "cpu", None) + return machine_info + + +def _base_config() -> Dict[str, Dict[str, Any]]: + return { + "default": { + "machine": "", + "account": "", + "partition": "", + "constraint": "", + "environment_commands": "", + "infer_path_parameters": True, + "simboard_type": "prod", + "www": "", + } + } + + +@pytest.mark.parametrize( + ("simboard_type", "expected_www"), + [ + ("prod", "/global/cfs/cdirs/e3sm/www/simboard/prod/"), + ("dev", "/global/cfs/cdirs/e3sm/www/simboard/dev/"), + ], +) +def test_determine_parameters_infers_simboard_www( + simboard_type: str, expected_www: str +) -> None: + config = _base_config() + config["default"]["simboard_type"] = simboard_type + + updated = _determine_parameters(_fake_machine_info(), config) + + assert updated["default"]["www"] == expected_www + + +def test_determine_parameters_requires_www_without_path_inference() -> None: + config = _base_config() + config["default"]["infer_path_parameters"] = False + + with pytest.raises( + ValueError, match="www must be provided when infer_path_parameters is False." + ): + _determine_parameters(_fake_machine_info(), config) + + +def test_default_ini_rejects_invalid_simboard_type(tmp_path: Path) -> None: + config_path = tmp_path / "bad_simboard.cfg" + config_path.write_text( + "\n".join( + [ + "[default]", + "case = case_name", + "input = /input", + "output = /output", + "www = /www", + "simboard_type = invalid", + ] + ) + ) + config = ConfigObj( + str(config_path), + configspec="/home/runner/work/zppy/zppy/zppy/defaults/default.ini", + ) + + result = config.validate(Validator()) + + assert result is not True + assert result["default"]["simboard_type"] is False diff --git a/zppy/__main__.py b/zppy/__main__.py index 61d9ede1..3d89da65 100644 --- a/zppy/__main__.py +++ b/zppy/__main__.py @@ -259,9 +259,23 @@ def _determine_parameters(machine_info: MachineInfo, config: ConfigObj) -> Confi config["default"][ "environment_commands" ] = f"source {unified_base}/load_latest_e3sm_unified_{machine}.sh" + _set_default_www(machine_info, config) return config +def _set_default_www(machine_info: MachineInfo, config: ConfigObj) -> None: + if config["default"]["www"] != "": + return + if not config["default"]["infer_path_parameters"]: + raise ValueError("www must be provided when infer_path_parameters is False.") + + simboard_type = config["default"]["simboard_type"] + web_portal_base_path = machine_info.config.get("web_portal", "base_path") + config["default"]["www"] = ( + f"{web_portal_base_path}/simboard/{simboard_type}/" + ) + + def _launch_scripts(config: ConfigObj, script_dir, job_ids_file, plugins) -> None: existing_bundles: List[Bundle] = [] diff --git a/zppy/defaults/default.ini b/zppy/defaults/default.ini index 05800b22..44bf5f5c 100755 --- a/zppy/defaults/default.ini +++ b/zppy/defaults/default.ini @@ -70,6 +70,8 @@ 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="") +# Which SimBoard namespace to use when inferring `www` +simboard_type = option("prod", "dev", default="prod") # 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") @@ -106,8 +108,9 @@ walltime = string(default="02:00:00") # web_portal_base_path -- NOTE: this parameter is created internally # web_portal_base_url -- NOTE: this parameter is created internally # Where the post-processing visuals should go (to be viewed online) -# NOTE: no default, must be provided by user -www = string +# Leave blank to infer `/simboard//` +# when `infer_path_parameters = True` +www = string(default="") # The years to run; "1:100:20" would mean process years 1-100 in 20-year increments years = string_list(default=list("")) From d7d426fbb0865cd1335971bbba6a5eb59b9c2b66 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 24 Jun 2026 18:53:41 +0000 Subject: [PATCH 02/18] Refine simboard default coverage --- tests/test_zppy_main.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/test_zppy_main.py b/tests/test_zppy_main.py index 91b6deda..6fa35d04 100644 --- a/tests/test_zppy_main.py +++ b/tests/test_zppy_main.py @@ -70,6 +70,7 @@ def test_determine_parameters_requires_www_without_path_inference() -> None: def test_default_ini_rejects_invalid_simboard_type(tmp_path: Path) -> None: config_path = tmp_path / "bad_simboard.cfg" + default_ini = Path(__file__).resolve().parents[1] / "zppy" / "defaults" / "default.ini" config_path.write_text( "\n".join( [ @@ -84,7 +85,7 @@ def test_default_ini_rejects_invalid_simboard_type(tmp_path: Path) -> None: ) config = ConfigObj( str(config_path), - configspec="/home/runner/work/zppy/zppy/zppy/defaults/default.ini", + configspec=str(default_ini), ) result = config.validate(Validator()) From 0c190f4162cd975878a9aecf122e4c9841d7d4d7 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 22 Jul 2026 00:44:27 +0000 Subject: [PATCH 03/18] Add explicit simboard publishing config --- tests/test_sections.py | 10 ++++- tests/test_zppy_main.py | 83 +++++++++++++++++++++++++++++++++------ zppy/__main__.py | 24 +++++++---- zppy/defaults/default.ini | 12 ++++-- zppy/simboard.py | 68 ++++++++++++++++++++++++++++++++ 5 files changed, 172 insertions(+), 25 deletions(-) create mode 100644 zppy/simboard.py diff --git a/tests/test_sections.py b/tests/test_sections.py index 49a73b57..5c78d788 100644 --- a/tests/test_sections.py +++ b/tests/test_sections.py @@ -92,7 +92,6 @@ def test_sections(): "plugins": [], "reservation": "", "qos": "regular", - "simboard_type": "prod", "templateDir": "zppy/templates", "ts_atm_grid": "180x360_aave", "ts_atm_subsection": "", @@ -108,6 +107,15 @@ def test_sections(): } compare(actual_default, expected_default) + # simboard + section_name = "simboard" + actual_section = config[section_name] + expected_section = { + "enabled": False, + "simulation_type": "production", + } + compare(actual_section, expected_section) + # ts section_name = "ts" actual_section = config[section_name] diff --git a/tests/test_zppy_main.py b/tests/test_zppy_main.py index 6fa35d04..eeb6ba10 100644 --- a/tests/test_zppy_main.py +++ b/tests/test_zppy_main.py @@ -34,41 +34,96 @@ def _base_config() -> Dict[str, Dict[str, Any]]: "constraint": "", "environment_commands": "", "infer_path_parameters": True, - "simboard_type": "prod", "www": "", - } + }, + "simboard": { + "enabled": False, + "simulation_type": "production", + }, } @pytest.mark.parametrize( - ("simboard_type", "expected_www"), + ("simulation_type", "expected_www"), [ - ("prod", "/global/cfs/cdirs/e3sm/www/simboard/prod/"), - ("dev", "/global/cfs/cdirs/e3sm/www/simboard/dev/"), + ( + "production", + "/global/cfs/cdirs/e3sm/www/diagnostics_archive/production/", + ), + ( + "development", + "/global/cfs/cdirs/e3sm/www/diagnostics_archive/development/", + ), ], ) def test_determine_parameters_infers_simboard_www( - simboard_type: str, expected_www: str + simulation_type: str, expected_www: str ) -> None: config = _base_config() - config["default"]["simboard_type"] = simboard_type + config["simboard"]["enabled"] = True + config["simboard"]["simulation_type"] = simulation_type updated = _determine_parameters(_fake_machine_info(), config) assert updated["default"]["www"] == expected_www -def test_determine_parameters_requires_www_without_path_inference() -> None: +def test_determine_parameters_preserves_explicit_www_when_simboard_enabled() -> None: + config = _base_config() + config["default"]["www"] = "/custom/www" + config["simboard"]["enabled"] = True + + updated = _determine_parameters(_fake_machine_info(), config) + + assert updated["default"]["www"] == "/custom/www" + + +def test_determine_parameters_requires_www_without_simboard() -> None: config = _base_config() - config["default"]["infer_path_parameters"] = False with pytest.raises( - ValueError, match="www must be provided when infer_path_parameters is False." + ValueError, + match=( + r"www is empty\. Provide \[default\] www or set \[simboard\] " + r"enabled = True" + ), ): _determine_parameters(_fake_machine_info(), config) -def test_default_ini_rejects_invalid_simboard_type(tmp_path: Path) -> None: +def test_determine_parameters_rejects_none_simulation_type_when_enabled() -> None: + config = _base_config() + config["simboard"]["enabled"] = True + config["simboard"]["simulation_type"] = "none" + + with pytest.raises( + ValueError, + match=( + "simboard.simulation_type must be 'production' or 'development' " + "when simboard.enabled is True." + ), + ): + _determine_parameters(_fake_machine_info(), config) + + +def test_determine_parameters_requires_inferable_web_root() -> None: + config = _base_config() + config["simboard"]["enabled"] = True + machine_info = _fake_machine_info() + machine_info.config.remove_option("web_portal", "base_path") + + with pytest.raises( + ValueError, + match=( + "www is empty and simboard.enabled is True, but machine 'pm-cpu' " + "has no web_portal.base_path in mache; cannot infer a " + "diagnostics_archive path." + ), + ): + _determine_parameters(machine_info, config) + + +def test_default_ini_rejects_invalid_simulation_type(tmp_path: Path) -> None: config_path = tmp_path / "bad_simboard.cfg" default_ini = Path(__file__).resolve().parents[1] / "zppy" / "defaults" / "default.ini" config_path.write_text( @@ -79,7 +134,9 @@ def test_default_ini_rejects_invalid_simboard_type(tmp_path: Path) -> None: "input = /input", "output = /output", "www = /www", - "simboard_type = invalid", + "", + "[simboard]", + "simulation_type = invalid", ] ) ) @@ -91,4 +148,4 @@ def test_default_ini_rejects_invalid_simboard_type(tmp_path: Path) -> None: result = config.validate(Validator()) assert result is not True - assert result["default"]["simboard_type"] is False + assert result["simboard"]["simulation_type"] is False diff --git a/zppy/__main__.py b/zppy/__main__.py index 3d89da65..bbd5d56b 100644 --- a/zppy/__main__.py +++ b/zppy/__main__.py @@ -23,6 +23,12 @@ from zppy.mpas_analysis import mpas_analysis from zppy.pcmdi_diags import pcmdi_diags from zppy.provenance import build_provenance_extras, write_provenance_settings +from zppy.simboard import ( + infer_simboard_www, + simboard, + simboard_enabled, + validate_simboard_config, +) from zppy.tc_analysis import tc_analysis from zppy.ts import ts from zppy.utils import check_status, submit_script @@ -264,16 +270,17 @@ def _determine_parameters(machine_info: MachineInfo, config: ConfigObj) -> Confi def _set_default_www(machine_info: MachineInfo, config: ConfigObj) -> None: + validate_simboard_config(config) if config["default"]["www"] != "": return - if not config["default"]["infer_path_parameters"]: - raise ValueError("www must be provided when infer_path_parameters is False.") - simboard_type = config["default"]["simboard_type"] - web_portal_base_path = machine_info.config.get("web_portal", "base_path") - config["default"]["www"] = ( - f"{web_portal_base_path}/simboard/{simboard_type}/" - ) + if not simboard_enabled(config): + raise ValueError( + "www is empty. Provide [default] www or set [simboard] enabled = " + "True to infer a SimBoard-compatible diagnostics_archive path." + ) + + config["default"]["www"] = infer_simboard_www(machine_info, config) def _launch_scripts(config: ConfigObj, script_dir, job_ids_file, plugins) -> None: @@ -282,6 +289,9 @@ def _launch_scripts(config: ConfigObj, script_dir, job_ids_file, plugins) -> Non # predefined bundles existing_bundles = predefined_bundles(config, script_dir, existing_bundles) + # simboard configuration task + existing_bundles = simboard(config, script_dir, existing_bundles, job_ids_file) + # climo tasks existing_bundles = climo(config, script_dir, existing_bundles, job_ids_file) diff --git a/zppy/defaults/default.ini b/zppy/defaults/default.ini index 44bf5f5c..57d28d72 100755 --- a/zppy/defaults/default.ini +++ b/zppy/defaults/default.ini @@ -70,8 +70,6 @@ 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="") -# Which SimBoard namespace to use when inferring `www` -simboard_type = option("prod", "dev", default="prod") # 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") @@ -108,8 +106,8 @@ walltime = string(default="02:00:00") # web_portal_base_path -- NOTE: this parameter is created internally # web_portal_base_url -- NOTE: this parameter is created internally # Where the post-processing visuals should go (to be viewed online) -# Leave blank to infer `/simboard//` -# when `infer_path_parameters = True` +# Leave blank to infer `/diagnostics_archive//` +# when `[simboard] enabled = True` www = string(default="") # The years to run; "1:100:20" would mean process years 1-100 in 20-year increments years = string_list(default=list("")) @@ -121,6 +119,12 @@ active = boolean(default=True) [[__many__]] active = boolean(default=None) +[simboard] +# Opt in to SimBoard-compatible publishing behavior. +enabled = boolean(default=False) +# Use "none" only when SimBoard publishing is disabled. +simulation_type = option("production", "development", "none", default="production") + [climo] exclude = boolean(default=False) # NOTE: always overrides value in [default] diff --git a/zppy/simboard.py b/zppy/simboard.py new file mode 100644 index 00000000..c161d939 --- /dev/null +++ b/zppy/simboard.py @@ -0,0 +1,68 @@ +from configparser import NoOptionError, NoSectionError +from typing import List + +from configobj import ConfigObj +from mache import MachineInfo + +from zppy.bundle import Bundle +from zppy.logger import _setup_custom_logger + +logger = _setup_custom_logger(__name__) + + +def simboard( + config: ConfigObj, + script_dir: str, + existing_bundles: List[Bundle], + job_ids_file: str, +) -> List[Bundle]: + del script_dir, job_ids_file + if config["simboard"].sections: + raise ValueError("The [simboard] section does not support subsections.") + return existing_bundles + + +def simboard_enabled(config: ConfigObj) -> bool: + return bool(config["simboard"]["enabled"]) + + +def validate_simboard_config(config: ConfigObj) -> None: + if not simboard_enabled(config): + return + + simulation_type = str(config["simboard"]["simulation_type"]) + if simulation_type == "none": + raise ValueError( + "simboard.simulation_type must be 'production' or 'development' " + "when simboard.enabled is True." + ) + + +def infer_simboard_www(machine_info: MachineInfo, config: ConfigObj) -> str: + simulation_type = str(config["simboard"]["simulation_type"]) + try: + web_portal_base_path = machine_info.config.get("web_portal", "base_path") + except (NoSectionError, NoOptionError) as exc: + raise ValueError( + f"www is empty and simboard.enabled is True, but machine " + f"'{machine_info.machine}' has no web_portal.base_path in mache; " + "cannot infer a diagnostics_archive path." + ) from exc + + web_portal_base_path = web_portal_base_path.rstrip("/") + if web_portal_base_path == "": + raise ValueError( + f"www is empty and simboard.enabled is True, but machine " + f"'{machine_info.machine}' has an empty web_portal.base_path in " + "mache; cannot infer a diagnostics_archive path." + ) + + inferred_www = ( + f"{web_portal_base_path}/diagnostics_archive/{simulation_type}/" + ) + logger.info( + "Inferred www=%s from mache web_portal.base_path because " + "simboard.enabled is True.", + inferred_www, + ) + return inferred_www From 1d1921ad3c3c329fd0d5ebde147c99ea677e8628 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 22 Jul 2026 00:45:41 +0000 Subject: [PATCH 04/18] Tighten simboard config messaging --- tests/test_zppy_main.py | 4 ++-- zppy/__main__.py | 5 +++-- zppy/defaults/default.ini | 1 + zppy/simboard.py | 5 ++--- 4 files changed, 8 insertions(+), 7 deletions(-) diff --git a/tests/test_zppy_main.py b/tests/test_zppy_main.py index eeb6ba10..16ddb6c0 100644 --- a/tests/test_zppy_main.py +++ b/tests/test_zppy_main.py @@ -84,8 +84,8 @@ def test_determine_parameters_requires_www_without_simboard() -> None: with pytest.raises( ValueError, match=( - r"www is empty\. Provide \[default\] www or set \[simboard\] " - r"enabled = True" + r"www is empty\. Provide \[default\] www or set `enabled = True` " + r"in the \[simboard\] section" ), ): _determine_parameters(_fake_machine_info(), config) diff --git a/zppy/__main__.py b/zppy/__main__.py index bbd5d56b..ea2d0fd4 100644 --- a/zppy/__main__.py +++ b/zppy/__main__.py @@ -276,8 +276,9 @@ def _set_default_www(machine_info: MachineInfo, config: ConfigObj) -> None: if not simboard_enabled(config): raise ValueError( - "www is empty. Provide [default] www or set [simboard] enabled = " - "True to infer a SimBoard-compatible diagnostics_archive path." + "www is empty. Provide [default] www or set `enabled = True` in " + "the [simboard] section to infer a SimBoard-compatible " + "diagnostics_archive path." ) config["default"]["www"] = infer_simboard_www(machine_info, config) diff --git a/zppy/defaults/default.ini b/zppy/defaults/default.ini index 57d28d72..6da1673f 100755 --- a/zppy/defaults/default.ini +++ b/zppy/defaults/default.ini @@ -123,6 +123,7 @@ active = boolean(default=True) # Opt in to SimBoard-compatible publishing behavior. enabled = boolean(default=False) # Use "none" only when SimBoard publishing is disabled. +# Default to "production" so enabled configs can opt in without overriding it. simulation_type = option("production", "development", "none", default="production") [climo] diff --git a/zppy/simboard.py b/zppy/simboard.py index c161d939..26811dd3 100644 --- a/zppy/simboard.py +++ b/zppy/simboard.py @@ -12,11 +12,10 @@ def simboard( config: ConfigObj, - script_dir: str, + _script_dir: str, existing_bundles: List[Bundle], - job_ids_file: str, + _job_ids_file: str, ) -> List[Bundle]: - del script_dir, job_ids_file if config["simboard"].sections: raise ValueError("The [simboard] section does not support subsections.") return existing_bundles From 3da1fee218ac9897ff0f48dfef187cf108a9b149 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 22 Jul 2026 00:46:30 +0000 Subject: [PATCH 05/18] Clarify simboard task intent --- zppy/__main__.py | 2 ++ zppy/simboard.py | 11 +++++++++++ 2 files changed, 13 insertions(+) diff --git a/zppy/__main__.py b/zppy/__main__.py index ea2d0fd4..4827e6ba 100644 --- a/zppy/__main__.py +++ b/zppy/__main__.py @@ -270,6 +270,8 @@ def _determine_parameters(machine_info: MachineInfo, config: ConfigObj) -> Confi def _set_default_www(machine_info: MachineInfo, config: ConfigObj) -> None: + # Keep SimBoard-specific validation active even when `www` is already set, + # because `[simboard] enabled = True` still enables SimBoard validation. validate_simboard_config(config) if config["default"]["www"] != "": return diff --git a/zppy/simboard.py b/zppy/simboard.py index 26811dd3..55c9a97a 100644 --- a/zppy/simboard.py +++ b/zppy/simboard.py @@ -16,12 +16,23 @@ def simboard( existing_bundles: List[Bundle], _job_ids_file: str, ) -> List[Bundle]: + """Validate the configuration-only `[simboard]` task hook. + + This section is an explicit top-level SimBoard configuration entry point, + analogous to `[bundle]`: it influences how other tasks are configured, but + it does not launch an HPC job of its own. + """ if config["simboard"].sections: raise ValueError("The [simboard] section does not support subsections.") return existing_bundles def simboard_enabled(config: ConfigObj) -> bool: + """Return whether SimBoard publishing is enabled. + + Assumes `config` has already been validated against `default.ini`, which + provides the `[simboard]` section and its default values. + """ return bool(config["simboard"]["enabled"]) From 32e13ff0874c71b484f784a392feaf38bf6f9262 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 22 Jul 2026 00:47:18 +0000 Subject: [PATCH 06/18] Clean up simboard validation helpers --- zppy/__main__.py | 3 ++- zppy/simboard.py | 9 +++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/zppy/__main__.py b/zppy/__main__.py index 4827e6ba..870a3743 100644 --- a/zppy/__main__.py +++ b/zppy/__main__.py @@ -280,7 +280,8 @@ def _set_default_www(machine_info: MachineInfo, config: ConfigObj) -> None: raise ValueError( "www is empty. Provide [default] www or set `enabled = True` in " "the [simboard] section to infer a SimBoard-compatible " - "diagnostics_archive path." + "diagnostics_archive path. Note: inference requires " + "web_portal.base_path in Mache configuration." ) config["default"]["www"] = infer_simboard_www(machine_info, config) diff --git a/zppy/simboard.py b/zppy/simboard.py index 55c9a97a..803ac235 100644 --- a/zppy/simboard.py +++ b/zppy/simboard.py @@ -20,7 +20,8 @@ def simboard( This section is an explicit top-level SimBoard configuration entry point, analogous to `[bundle]`: it influences how other tasks are configured, but - it does not launch an HPC job of its own. + it does not launch an HPC job of its own. The unused task-like parameters + are retained so this hook matches the call signature of other zppy tasks. """ if config["simboard"].sections: raise ValueError("The [simboard] section does not support subsections.") @@ -33,14 +34,14 @@ def simboard_enabled(config: ConfigObj) -> bool: Assumes `config` has already been validated against `default.ini`, which provides the `[simboard]` section and its default values. """ - return bool(config["simboard"]["enabled"]) + return config["simboard"]["enabled"] def validate_simboard_config(config: ConfigObj) -> None: if not simboard_enabled(config): return - simulation_type = str(config["simboard"]["simulation_type"]) + simulation_type = config["simboard"]["simulation_type"] if simulation_type == "none": raise ValueError( "simboard.simulation_type must be 'production' or 'development' " @@ -49,7 +50,7 @@ def validate_simboard_config(config: ConfigObj) -> None: def infer_simboard_www(machine_info: MachineInfo, config: ConfigObj) -> str: - simulation_type = str(config["simboard"]["simulation_type"]) + simulation_type = config["simboard"]["simulation_type"] try: web_portal_base_path = machine_info.config.get("web_portal", "base_path") except (NoSectionError, NoOptionError) as exc: From e6e0f1e082dc946041ed65901cccbd7a986af0b0 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 22 Jul 2026 00:48:17 +0000 Subject: [PATCH 07/18] Harden simboard config checks --- tests/test_zppy_main.py | 40 ++++++++++++++++++++++++++++++++++++++++ zppy/simboard.py | 13 +++++++++++-- 2 files changed, 51 insertions(+), 2 deletions(-) diff --git a/tests/test_zppy_main.py b/tests/test_zppy_main.py index 16ddb6c0..aeaf7ddd 100644 --- a/tests/test_zppy_main.py +++ b/tests/test_zppy_main.py @@ -8,6 +8,7 @@ from validate import Validator from zppy.__main__ import _determine_parameters +from zppy.simboard import simboard def _fake_machine_info() -> MagicMock: @@ -123,6 +124,45 @@ def test_determine_parameters_requires_inferable_web_root() -> None: _determine_parameters(machine_info, config) +def test_determine_parameters_rejects_empty_web_root() -> None: + config = _base_config() + config["simboard"]["enabled"] = True + machine_info = _fake_machine_info() + machine_info.config["web_portal"]["base_path"] = " " + + with pytest.raises( + ValueError, + match=( + "www is empty and simboard.enabled is True, but machine 'pm-cpu' " + "has an empty web_portal.base_path in mache; cannot infer a " + "diagnostics_archive path." + ), + ): + _determine_parameters(machine_info, config) + + +def test_simboard_rejects_subsections(tmp_path: Path) -> None: + config_path = tmp_path / "bad_simboard_subsection.cfg" + config_path.write_text( + "\n".join( + [ + "[simboard]", + "enabled = False", + "simulation_type = production", + "", + " [[nested]]", + " placeholder = value", + ] + ) + ) + config = ConfigObj(str(config_path)) + + with pytest.raises( + ValueError, match="The \\[simboard\\] section does not support subsections." + ): + simboard(config, "", [], "") + + def test_default_ini_rejects_invalid_simulation_type(tmp_path: Path) -> None: config_path = tmp_path / "bad_simboard.cfg" default_ini = Path(__file__).resolve().parents[1] / "zppy" / "defaults" / "default.ini" diff --git a/zppy/simboard.py b/zppy/simboard.py index 803ac235..1352aacc 100644 --- a/zppy/simboard.py +++ b/zppy/simboard.py @@ -34,7 +34,16 @@ def simboard_enabled(config: ConfigObj) -> bool: Assumes `config` has already been validated against `default.ini`, which provides the `[simboard]` section and its default values. """ - return config["simboard"]["enabled"] + enabled = config["simboard"]["enabled"] + if isinstance(enabled, bool): + return enabled + if isinstance(enabled, str): + enabled_lower = enabled.lower() + if enabled_lower == "true": + return True + if enabled_lower == "false": + return False + raise ValueError(f"Invalid value {enabled} for simboard.enabled") def validate_simboard_config(config: ConfigObj) -> None: @@ -60,7 +69,7 @@ def infer_simboard_www(machine_info: MachineInfo, config: ConfigObj) -> str: "cannot infer a diagnostics_archive path." ) from exc - web_portal_base_path = web_portal_base_path.rstrip("/") + web_portal_base_path = web_portal_base_path.strip().rstrip("/") if web_portal_base_path == "": raise ValueError( f"www is empty and simboard.enabled is True, but machine " From 3d6647409cc4390663343d6d0ef39a913930b4d9 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 22 Jul 2026 00:49:17 +0000 Subject: [PATCH 08/18] Clarify simboard validation assumptions --- tests/test_zppy_main.py | 9 ++++++++- zppy/__main__.py | 3 ++- zppy/simboard.py | 17 +++++++++++++++-- 3 files changed, 25 insertions(+), 4 deletions(-) diff --git a/tests/test_zppy_main.py b/tests/test_zppy_main.py index aeaf7ddd..0e341dec 100644 --- a/tests/test_zppy_main.py +++ b/tests/test_zppy_main.py @@ -143,9 +143,16 @@ def test_determine_parameters_rejects_empty_web_root() -> None: def test_simboard_rejects_subsections(tmp_path: Path) -> None: config_path = tmp_path / "bad_simboard_subsection.cfg" + default_ini = Path(__file__).resolve().parents[1] / "zppy" / "defaults" / "default.ini" config_path.write_text( "\n".join( [ + "[default]", + "case = case_name", + "input = /input", + "output = /output", + "www = /www", + "", "[simboard]", "enabled = False", "simulation_type = production", @@ -155,7 +162,7 @@ def test_simboard_rejects_subsections(tmp_path: Path) -> None: ] ) ) - config = ConfigObj(str(config_path)) + config = ConfigObj(str(config_path), configspec=str(default_ini)) with pytest.raises( ValueError, match="The \\[simboard\\] section does not support subsections." diff --git a/zppy/__main__.py b/zppy/__main__.py index 870a3743..c5136baa 100644 --- a/zppy/__main__.py +++ b/zppy/__main__.py @@ -271,7 +271,8 @@ def _determine_parameters(machine_info: MachineInfo, config: ConfigObj) -> Confi def _set_default_www(machine_info: MachineInfo, config: ConfigObj) -> None: # Keep SimBoard-specific validation active even when `www` is already set, - # because `[simboard] enabled = True` still enables SimBoard validation. + # because `[simboard] enabled = True` still requires validating + # `simulation_type`. validate_simboard_config(config) if config["default"]["www"] != "": return diff --git a/zppy/simboard.py b/zppy/simboard.py index 1352aacc..ab636409 100644 --- a/zppy/simboard.py +++ b/zppy/simboard.py @@ -10,6 +10,10 @@ logger = _setup_custom_logger(__name__) +def normalize_web_portal_base_path(web_portal_base_path: str) -> str: + return web_portal_base_path.strip().rstrip("/") + + def simboard( config: ConfigObj, _script_dir: str, @@ -22,7 +26,13 @@ def simboard( analogous to `[bundle]`: it influences how other tasks are configured, but it does not launch an HPC job of its own. The unused task-like parameters are retained so this hook matches the call signature of other zppy tasks. + This hook assumes the config has already been read and validated. """ + if "simboard" not in config: + raise ValueError( + "Missing [simboard] section. Validate the config against " + "default.ini before calling simboard()." + ) if config["simboard"].sections: raise ValueError("The [simboard] section does not support subsections.") return existing_bundles @@ -43,7 +53,10 @@ def simboard_enabled(config: ConfigObj) -> bool: return True if enabled_lower == "false": return False - raise ValueError(f"Invalid value {enabled} for simboard.enabled") + raise ValueError( + f"Invalid value '{enabled}' for simboard.enabled. Expected boolean " + "or string 'true'/'false'." + ) def validate_simboard_config(config: ConfigObj) -> None: @@ -69,7 +82,7 @@ def infer_simboard_www(machine_info: MachineInfo, config: ConfigObj) -> str: "cannot infer a diagnostics_archive path." ) from exc - web_portal_base_path = web_portal_base_path.strip().rstrip("/") + web_portal_base_path = normalize_web_portal_base_path(web_portal_base_path) if web_portal_base_path == "": raise ValueError( f"www is empty and simboard.enabled is True, but machine " From ee45ff1ad4576216c7a7038b2806959444028858 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 22 Jul 2026 00:50:32 +0000 Subject: [PATCH 09/18] Add simboard helper coverage --- tests/test_zppy_main.py | 81 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 80 insertions(+), 1 deletion(-) diff --git a/tests/test_zppy_main.py b/tests/test_zppy_main.py index 0e341dec..29bd8d16 100644 --- a/tests/test_zppy_main.py +++ b/tests/test_zppy_main.py @@ -8,7 +8,12 @@ from validate import Validator from zppy.__main__ import _determine_parameters -from zppy.simboard import simboard +from zppy.simboard import ( + infer_simboard_www, + normalize_web_portal_base_path, + simboard, + simboard_enabled, +) def _fake_machine_info() -> MagicMock: @@ -69,6 +74,80 @@ def test_determine_parameters_infers_simboard_www( assert updated["default"]["www"] == expected_www +@pytest.mark.parametrize( + ("base_path", "expected_www"), + [ + ( + "/global/cfs/cdirs/e3sm/www", + "/global/cfs/cdirs/e3sm/www/diagnostics_archive/production/", + ), + ( + "/global/cfs/cdirs/e3sm/www/", + "/global/cfs/cdirs/e3sm/www/diagnostics_archive/production/", + ), + ( + " /global/cfs/cdirs/e3sm/www/ ", + "/global/cfs/cdirs/e3sm/www/diagnostics_archive/production/", + ), + ], +) +def test_infer_simboard_www_normalizes_web_root( + base_path: str, expected_www: str +) -> None: + machine_info = _fake_machine_info() + machine_info.config["web_portal"]["base_path"] = base_path + config = _base_config() + + assert infer_simboard_www(machine_info, config) == expected_www + + +@pytest.mark.parametrize( + ("raw_path", "expected_path"), + [ + ("/global/cfs/cdirs/e3sm/www", "/global/cfs/cdirs/e3sm/www"), + ("/global/cfs/cdirs/e3sm/www/", "/global/cfs/cdirs/e3sm/www"), + (" /global/cfs/cdirs/e3sm/www/ ", "/global/cfs/cdirs/e3sm/www"), + (" ", ""), + ], +) +def test_normalize_web_portal_base_path(raw_path: str, expected_path: str) -> None: + assert normalize_web_portal_base_path(raw_path) == expected_path + + +@pytest.mark.parametrize( + ("enabled_value", "expected_enabled"), + [ + (True, True), + (False, False), + ("true", True), + ("TRUE", True), + ("false", False), + ("FALSE", False), + ], +) +def test_simboard_enabled_parses_bool_values( + enabled_value: Any, expected_enabled: bool +) -> None: + config = _base_config() + config["simboard"]["enabled"] = enabled_value + + assert simboard_enabled(config) is expected_enabled + + +def test_simboard_enabled_rejects_invalid_value() -> None: + config = _base_config() + config["simboard"]["enabled"] = "maybe" + + with pytest.raises( + ValueError, + match=( + "Invalid value 'maybe' for simboard.enabled. Expected boolean " + "or string 'true'/'false'." + ), + ): + simboard_enabled(config) + + def test_determine_parameters_preserves_explicit_www_when_simboard_enabled() -> None: config = _base_config() config["default"]["www"] = "/custom/www" From 5198b186da66a96730d85302ef12ba8b640cd49d Mon Sep 17 00:00:00 2001 From: Ryan Forsyth Date: Fri, 24 Jul 2026 12:44:07 -0500 Subject: [PATCH 10/18] Fix pre-commit errors --- tests/test_zppy_main.py | 8 ++++++-- zppy/simboard.py | 4 +--- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/tests/test_zppy_main.py b/tests/test_zppy_main.py index 29bd8d16..1cb133c8 100644 --- a/tests/test_zppy_main.py +++ b/tests/test_zppy_main.py @@ -222,7 +222,9 @@ def test_determine_parameters_rejects_empty_web_root() -> None: def test_simboard_rejects_subsections(tmp_path: Path) -> None: config_path = tmp_path / "bad_simboard_subsection.cfg" - default_ini = Path(__file__).resolve().parents[1] / "zppy" / "defaults" / "default.ini" + default_ini = ( + Path(__file__).resolve().parents[1] / "zppy" / "defaults" / "default.ini" + ) config_path.write_text( "\n".join( [ @@ -251,7 +253,9 @@ def test_simboard_rejects_subsections(tmp_path: Path) -> None: def test_default_ini_rejects_invalid_simulation_type(tmp_path: Path) -> None: config_path = tmp_path / "bad_simboard.cfg" - default_ini = Path(__file__).resolve().parents[1] / "zppy" / "defaults" / "default.ini" + default_ini = ( + Path(__file__).resolve().parents[1] / "zppy" / "defaults" / "default.ini" + ) config_path.write_text( "\n".join( [ diff --git a/zppy/simboard.py b/zppy/simboard.py index ab636409..f425fcef 100644 --- a/zppy/simboard.py +++ b/zppy/simboard.py @@ -90,9 +90,7 @@ def infer_simboard_www(machine_info: MachineInfo, config: ConfigObj) -> str: "mache; cannot infer a diagnostics_archive path." ) - inferred_www = ( - f"{web_portal_base_path}/diagnostics_archive/{simulation_type}/" - ) + inferred_www = f"{web_portal_base_path}/diagnostics_archive/{simulation_type}/" logger.info( "Inferred www=%s from mache web_portal.base_path because " "simboard.enabled is True.", From de3b2a05419dd424ba0b085383b7420265d30320 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 24 Jul 2026 17:59:43 +0000 Subject: [PATCH 11/18] Fix NoOptionError crash before SimBoard www inference Wrap `web_portal.base_path` and `web_portal.base_url` reads in `_determine_parameters()` with try/except to catch `NoSectionError`/`NoOptionError`, defaulting to `""`. This lets the `NoOptionError` be handled properly downstream in `infer_simboard_www()`, which raises the intended `ValueError` when `www` is empty and `simboard.enabled` is True. Fixes `test_determine_parameters_requires_inferable_web_root` which was failing with `configparser.NoOptionError` before reaching the SimBoard error-handling code. --- zppy/__main__.py | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/zppy/__main__.py b/zppy/__main__.py index c5136baa..1c96939c 100644 --- a/zppy/__main__.py +++ b/zppy/__main__.py @@ -225,12 +225,18 @@ def _determine_parameters(machine_info: MachineInfo, config: ConfigObj) -> Confi config["default"]["diagnostics_base_path"] = machine_info.config.get( "diagnostics", "base_path" ) - config["default"]["web_portal_base_path"] = machine_info.config.get( - "web_portal", "base_path" - ) - config["default"]["web_portal_base_url"] = machine_info.config.get( - "web_portal", "base_url" - ) + try: + config["default"]["web_portal_base_path"] = machine_info.config.get( + "web_portal", "base_path" + ) + except (configparser.NoSectionError, configparser.NoOptionError): + config["default"]["web_portal_base_path"] = "" + try: + config["default"]["web_portal_base_url"] = machine_info.config.get( + "web_portal", "base_url" + ) + except (configparser.NoSectionError, configparser.NoOptionError): + config["default"]["web_portal_base_url"] = "" # Determine machine to decide which header files to use if ("machine" not in config["default"]) or (config["default"]["machine"] == ""): From bfa34d452c81b4c5e975ddf85bf45763c8b8fc53 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 24 Jul 2026 18:27:04 +0000 Subject: [PATCH 12/18] Add docs for the new [simboard] configuration section - Add docs/source/user_guide/tasks/simboard.rst describing the [simboard] section, its expected behavior table, a configuration example, and its parameters. - Add simboard to the tasks index (list and toctree). - Update parameters.rst: www is now optional (inferred when simboard.enabled=True); add a SimBoard section parameters table at the end. --- docs/source/user_guide/parameters.rst | 34 ++++++++- docs/source/user_guide/tasks/index.rst | 3 + docs/source/user_guide/tasks/simboard.rst | 90 +++++++++++++++++++++++ 3 files changed, 125 insertions(+), 2 deletions(-) create mode 100644 docs/source/user_guide/tasks/simboard.rst diff --git a/docs/source/user_guide/parameters.rst b/docs/source/user_guide/parameters.rst index ee44b65a..053d3572 100644 --- a/docs/source/user_guide/parameters.rst +++ b/docs/source/user_guide/parameters.rst @@ -68,9 +68,11 @@ There are 6 output-specific parameters: - *(none)* - Where the post-processing results (``post/`` directory) should go. * - ``www`` - - **Yes** - - *(none)* + - No + - ``""`` - Where the post-processing visuals should go (to be viewed online). + Leave empty and set ``[simboard] enabled = True`` to have ``zppy`` + infer this path from Mache. See :doc:`tasks/simboard` for details. * - ``campaign`` - No - ``"none"`` @@ -404,6 +406,34 @@ These are no longer defined in ``zppy/defaults/default.ini``: These are still defined in ``zppy/defaults/default.ini``, but have no effect: .. code-block:: text + ncclimo_cmd nrows ncols + +SimBoard section parameters +============================ + +The ``[simboard]`` section controls SimBoard-compatible publishing. It is +a configuration-only hook; see :doc:`tasks/simboard` for full details. + +.. list-table:: + :header-rows: 1 + :widths: 22 10 18 50 + + * - Parameter + - Required + - Default + - Description + * - ``enabled`` + - No + - ``False`` + - Set to ``True`` to enable SimBoard-compatible publishing. + When ``True`` and ``[default] www`` is empty, ``zppy`` infers + ``www`` from Mache's ``web_portal.base_path``. + * - ``simulation_type`` + - No + - ``"production"`` + - Archive sub-directory for the run. One of ``"production"``, + ``"development"``, or ``"none"``. + Must not be ``"none"`` when ``enabled = True``. diff --git a/docs/source/user_guide/tasks/index.rst b/docs/source/user_guide/tasks/index.rst index 9dc8dd63..985e5e57 100644 --- a/docs/source/user_guide/tasks/index.rst +++ b/docs/source/user_guide/tasks/index.rst @@ -21,6 +21,8 @@ Listed for reference (bundle jobs are submitted after task jobs are generated in - Description * - :doc:`bundle` - Bundle multiple tasks into a single SLURM job + * - :doc:`simboard` + - Configure SimBoard-compatible diagnostics publishing * - :doc:`climo` - Generate climatology files using NCO's ``ncclimo`` * - :doc:`ts` @@ -50,6 +52,7 @@ Listed for reference (bundle jobs are submitted after task jobs are generated in :hidden: bundle + simboard climo ts e3sm_to_cmip diff --git a/docs/source/user_guide/tasks/simboard.rst b/docs/source/user_guide/tasks/simboard.rst new file mode 100644 index 00000000..b0fa991f --- /dev/null +++ b/docs/source/user_guide/tasks/simboard.rst @@ -0,0 +1,90 @@ +.. _task-simboard: + +simboard — SimBoard Publishing Configuration +============================================ + +The ``simboard`` section is a configuration-only hook that controls +SimBoard-compatible publishing behavior. Like :doc:`bundle`, it does not +launch an HPC job of its own; instead it influences how other tasks are +configured — specifically, it can infer the ``www`` output path from the +machine's Mache configuration. + +When ``enabled = True`` and ``www`` is left empty in ``[default]``, +``zppy`` derives ``www`` from the ``web_portal.base_path`` recorded in +Mache for the current machine: + +.. code-block:: text + + /diagnostics_archive// + +This gives SimBoard a single, predictable archive root to scan for +diagnostics. + +Expected behavior +----------------- + +.. list-table:: + :header-rows: 1 + :widths: 20 15 65 + + * - ``simboard.enabled`` + - ``www`` + - Behavior + * - ``False`` + - any + - ``zppy`` does nothing SimBoard-specific. + * - ``True`` + - empty + - Infer the SimBoard archive path from Mache's + ``web_portal.base_path``. + * - ``True`` + - set + - Use the explicit ``www`` path and do not override it. + ``simboard.enabled`` still controls SimBoard-specific validation + (e.g., ``simulation_type`` must not be ``"none"``). + * - ``True`` + - empty, but path cannot be inferred + - Raise a clear configuration error. + +Configuration example +--------------------- + +.. code-block:: cfg + + [default] + case = v3.LR.historical_0051 + input = /path/to/input + output = /path/to/output + # Leave www empty to let zppy infer it from Mache when simboard is enabled. + www = + + [simboard] + enabled = True + simulation_type = production + +Parameters +---------- + +.. list-table:: + :header-rows: 1 + :widths: 22 10 18 50 + + * - Parameter + - Required + - Default + - Description + * - ``enabled`` + - No + - ``False`` + - Set to ``True`` to enable SimBoard-compatible publishing behavior. + When enabled and ``[default] www`` is empty, ``zppy`` infers + ``www`` from Mache's ``web_portal.base_path``. + * - ``simulation_type`` + - No + - ``"production"`` + - Diagnostic classification for the archive path. One of + ``"production"``, ``"development"``, or ``"none"``. + Must not be ``"none"`` when ``enabled = True``. + +.. note:: + The ``[simboard]`` section does not support subsections. From 230c8cf380fd2b83115b16cd3e9b20637c427916 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 27 Jul 2026 17:52:49 +0000 Subject: [PATCH 13/18] Add dry-run simboard integration settings tests --- tests/integration/test_simboard_settings.py | 132 ++++++++++++++++++++ zppy/__main__.py | 1 + 2 files changed, 133 insertions(+) create mode 100644 tests/integration/test_simboard_settings.py diff --git a/tests/integration/test_simboard_settings.py b/tests/integration/test_simboard_settings.py new file mode 100644 index 00000000..16523bff --- /dev/null +++ b/tests/integration/test_simboard_settings.py @@ -0,0 +1,132 @@ +import ast +import configparser +import re +from pathlib import Path +from typing import Any, Dict, Optional +from unittest.mock import MagicMock + +import pytest +from configobj import ConfigObj +from validate import Validator + +from zppy.__main__ import _determine_parameters +from zppy.utils import write_settings_file + +_WEB_PORTAL_BASE_PATH = "/global/cfs/cdirs/e3sm/www" + + +def _default_ini_path() -> Path: + return Path(__file__).resolve().parents[2] / "zppy" / "defaults" / "default.ini" + + +def _write_cfg(tmp_path: Path, *, www: str, simboard_enabled: bool) -> Path: + config_path = tmp_path / "simboard_settings.cfg" + config_path.write_text( + "\n".join( + [ + "[default]", + "case = test_case", + "input = /input", + f"output = {tmp_path / 'output'}", + "dry_run = True", + "machine = pm-cpu", + f"www = {www}", + "", + "[simboard]", + f"enabled = {'True' if simboard_enabled else 'False'}", + "simulation_type = production", + "", + ] + ) + ) + return config_path + + +def _validated_config(config_path: Path) -> ConfigObj: + config = ConfigObj(str(config_path), configspec=str(_default_ini_path())) + validation_result = config.validate(Validator()) + assert validation_result is True + return config + + +def _fake_machine_info( + web_portal_base_path: Optional[str] = _WEB_PORTAL_BASE_PATH, +) -> MagicMock: + machine_config = configparser.ConfigParser() + machine_config["e3sm_unified"] = {"base_path": "/unified"} + machine_config["diagnostics"] = {"base_path": "/diagnostics"} + machine_config["web_portal"] = {"base_url": "https://portal.nersc.gov/cfs/e3sm"} + if web_portal_base_path is not None: + machine_config["web_portal"]["base_path"] = web_portal_base_path + + machine_info = MagicMock() + machine_info.machine = "pm-cpu" + machine_info.config = machine_config + machine_info.get_account_defaults.return_value = ("e3sm", "regular", "cpu", None) + return machine_info + + +def _write_default_settings(tmp_path: Path, config: ConfigObj) -> Path: + settings_path = tmp_path / "resolved_default.settings" + write_settings_file(str(settings_path), dict(config["default"]), (1, 1)) + return settings_path + + +def _read_default_settings(settings_path: Path) -> Dict[str, Any]: + settings_text = settings_path.read_text() + parsed_settings = ast.parse(settings_text, mode="exec") + assert parsed_settings.body + first_expression = parsed_settings.body[0] + assert isinstance(first_expression, ast.Expr) + default_settings = ast.literal_eval(first_expression.value) + assert isinstance(default_settings, dict) + return default_settings + + +def test_simboard_disabled_preserves_explicit_www(tmp_path: Path) -> None: + config_path = _write_cfg( + tmp_path, www="/some/explicit/path", simboard_enabled=False + ) + config = _validated_config(config_path) + + updated_config = _determine_parameters(_fake_machine_info(), config) + settings_path = _write_default_settings(tmp_path, updated_config) + default_settings = _read_default_settings(settings_path) + + assert default_settings["www"] == "/some/explicit/path" + + +def test_simboard_enabled_infers_www_from_machine_info(tmp_path: Path) -> None: + config_path = _write_cfg(tmp_path, www="", simboard_enabled=True) + config = _validated_config(config_path) + expected_www = f"{_WEB_PORTAL_BASE_PATH}/diagnostics_archive/production/" + + updated_config = _determine_parameters(_fake_machine_info(), config) + settings_path = _write_default_settings(tmp_path, updated_config) + default_settings = _read_default_settings(settings_path) + + assert default_settings["www"] == expected_www + + +def test_simboard_enabled_preserves_explicit_www(tmp_path: Path) -> None: + config_path = _write_cfg(tmp_path, www="/custom/path", simboard_enabled=True) + config = _validated_config(config_path) + + updated_config = _determine_parameters(_fake_machine_info(), config) + settings_path = _write_default_settings(tmp_path, updated_config) + default_settings = _read_default_settings(settings_path) + + assert default_settings["www"] == "/custom/path" + + +def test_simboard_enabled_empty_www_requires_inferable_path(tmp_path: Path) -> None: + config_path = _write_cfg(tmp_path, www="", simboard_enabled=True) + config = _validated_config(config_path) + expected_message = ( + "www is empty and simboard.enabled is True, but machine 'pm-cpu' " + "has no web_portal.base_path in mache; cannot infer a " + "diagnostics_archive path." + ) + + with pytest.raises(ValueError, match=re.escape(expected_message)): + _determine_parameters(_fake_machine_info(web_portal_base_path=None), config) diff --git a/zppy/__main__.py b/zppy/__main__.py index 1c96939c..a4f68d1f 100644 --- a/zppy/__main__.py +++ b/zppy/__main__.py @@ -1,4 +1,5 @@ import argparse +import configparser import errno import importlib import io From 41892d02b7e77d286e5fdc1c0a6a415d8b175dff Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 30 Jul 2026 19:32:10 +0000 Subject: [PATCH 14/18] Change simulation_type default to "development"; update docs - Change `default.ini` default from "production" to "development" for `[simboard] simulation_type`. Accidentally publishing to the production archive is more harmful than publishing to development. - Update `test_sections.py` to match the new default. - Update user guide `simboard.rst`: - Config example now shows `simulation_type = development`. - Parameter table reflects new default. - Add a "Promoting diagnostics from development to production" section explaining the two-step process. - Update `parameters.rst` SimBoard table to show new default. - Add dev guide `docs/source/dev_guide/tasks/simboard.rst`. - Register simboard in `docs/source/dev_guide/tasks/index.rst`. --- docs/source/dev_guide/tasks/index.rst | 1 + docs/source/dev_guide/tasks/simboard.rst | 81 +++++++++++++++++++++++ docs/source/user_guide/parameters.rst | 2 +- docs/source/user_guide/tasks/simboard.rst | 23 ++++++- tests/test_sections.py | 2 +- zppy/defaults/default.ini | 5 +- 6 files changed, 108 insertions(+), 6 deletions(-) create mode 100644 docs/source/dev_guide/tasks/simboard.rst diff --git a/docs/source/dev_guide/tasks/index.rst b/docs/source/dev_guide/tasks/index.rst index 285ed532..db1fd583 100644 --- a/docs/source/dev_guide/tasks/index.rst +++ b/docs/source/dev_guide/tasks/index.rst @@ -11,6 +11,7 @@ other tasks. :maxdepth: 1 bundle + simboard climo ts e3sm_to_cmip diff --git a/docs/source/dev_guide/tasks/simboard.rst b/docs/source/dev_guide/tasks/simboard.rst new file mode 100644 index 00000000..b0e6ed1d --- /dev/null +++ b/docs/source/dev_guide/tasks/simboard.rst @@ -0,0 +1,81 @@ +.. _dev-task-simboard: + +simboard (Developer Reference) +================================ + +Implementation +-------------- + +- **Python module**: ``zppy/simboard.py`` +- **Jinja2 template**: none (configuration-only hook, no HPC job is submitted) + +The ``simboard`` section is a configuration-only task hook, analogous to +:doc:`bundle`. It performs validation and, when ``enabled = True`` and +``[default] www`` is empty, infers ``www`` from Mache's +``web_portal.base_path``. + +Key functions in ``zppy/simboard.py``: + +- ``simboard(config, script_dir, existing_bundles, job_ids_file)``: the + main hook registered in ``_launch_scripts``. Validates that the + ``[simboard]`` section contains no subsections and returns + ``existing_bundles`` unchanged. +- ``simboard_enabled(config)``: parses the ``enabled`` field from a bool + or ``"true"``/``"false"`` string (case-insensitive). +- ``validate_simboard_config(config)``: rejects ``simulation_type = "none"`` + when ``enabled = True``. +- ``normalize_web_portal_base_path(path)``: strips leading/trailing + whitespace and trailing slashes. +- ``infer_simboard_www(machine_info, config)``: builds + ``/diagnostics_archive//``; raises + a descriptive ``ValueError`` if Mache has no (or empty) + ``web_portal.base_path`` for the machine. + +``www`` inference is wired into ``_determine_parameters`` in +``zppy/__main__.py`` via the ``_set_default_www`` helper, which: + +1. Always calls ``validate_simboard_config`` (checks ``simulation_type`` + even when ``www`` is already set). +2. Returns immediately if ``www`` is already set. +3. Otherwise requires ``simboard.enabled = True``; calls + ``infer_simboard_www`` and sets ``config["default"]["www"]``. + +Config defaults (``zppy/defaults/default.ini``) +------------------------------------------------ + +.. code-block:: ini + + [simboard] + enabled = boolean(default=False) + simulation_type = option("production", "development", "none", default="development") + +Dependencies +------------ + +**Upstream (what simboard depends on):** + +- None + +**Downstream (what depends on simboard):** + +- None (the ``[simboard]`` section has no downstream task dependencies; it + only sets ``www``, which is consumed by every visual-output task) + +Testing +------- + +Unit tests are in ``tests/test_zppy_main.py`` and cover: + +- ``www`` inference for both ``production`` and ``development`` types. +- Path normalization (trailing slash, leading/trailing whitespace). +- ``simboard_enabled`` parsing (bool, string, invalid). +- Explicit ``www`` is preserved when SimBoard is enabled. +- Error on empty ``www`` with SimBoard disabled. +- Error on ``simulation_type = "none"`` when enabled. +- Errors when Mache has no or empty ``web_portal.base_path``. +- Rejection of subsections under ``[simboard]``. +- Rejection of invalid ``simulation_type`` values via ConfigObj validation. + +Integration tests are in ``tests/integration/test_simboard_settings.py`` +and cover all four rows of the expected-behavior table using a real +``zppy`` config file with ``dry_run = True``. diff --git a/docs/source/user_guide/parameters.rst b/docs/source/user_guide/parameters.rst index 053d3572..974b7ba6 100644 --- a/docs/source/user_guide/parameters.rst +++ b/docs/source/user_guide/parameters.rst @@ -433,7 +433,7 @@ a configuration-only hook; see :doc:`tasks/simboard` for full details. ``www`` from Mache's ``web_portal.base_path``. * - ``simulation_type`` - No - - ``"production"`` + - ``"development"`` - Archive sub-directory for the run. One of ``"production"``, ``"development"``, or ``"none"``. Must not be ``"none"`` when ``enabled = True``. diff --git a/docs/source/user_guide/tasks/simboard.rst b/docs/source/user_guide/tasks/simboard.rst index b0fa991f..0ba14f23 100644 --- a/docs/source/user_guide/tasks/simboard.rst +++ b/docs/source/user_guide/tasks/simboard.rst @@ -60,7 +60,7 @@ Configuration example [simboard] enabled = True - simulation_type = production + simulation_type = development Parameters ---------- @@ -81,10 +81,29 @@ Parameters ``www`` from Mache's ``web_portal.base_path``. * - ``simulation_type`` - No - - ``"production"`` + - ``"development"`` - Diagnostic classification for the archive path. One of ``"production"``, ``"development"``, or ``"none"``. Must not be ``"none"`` when ``enabled = True``. + Defaults to ``"development"`` — see :ref:`simboard-promotion` below. .. note:: The ``[simboard]`` section does not support subsections. + +.. _simboard-promotion: + +Promoting diagnostics from development to production +----------------------------------------------------- + +The default ``simulation_type`` is ``"development"`` rather than +``"production"``. Accidentally placing development diagnostics under the +``production`` archive is more harmful than placing production diagnostics +under ``development``, so production is an explicit opt-in. + +To promote a run's diagnostics to the production archive: + +1. Update ``simulation_type = production`` in the ``[simboard]`` section of + your ``zppy`` configuration file and re-run ``zppy``. +2. Manually move (or copy) the existing diagnostic output from + ``/diagnostics_archive/development//`` to + ``/diagnostics_archive/production//``. diff --git a/tests/test_sections.py b/tests/test_sections.py index 5c78d788..ca4cff24 100644 --- a/tests/test_sections.py +++ b/tests/test_sections.py @@ -112,7 +112,7 @@ def test_sections(): actual_section = config[section_name] expected_section = { "enabled": False, - "simulation_type": "production", + "simulation_type": "development", } compare(actual_section, expected_section) diff --git a/zppy/defaults/default.ini b/zppy/defaults/default.ini index 6da1673f..095095e9 100755 --- a/zppy/defaults/default.ini +++ b/zppy/defaults/default.ini @@ -123,8 +123,9 @@ active = boolean(default=True) # Opt in to SimBoard-compatible publishing behavior. enabled = boolean(default=False) # Use "none" only when SimBoard publishing is disabled. -# Default to "production" so enabled configs can opt in without overriding it. -simulation_type = option("production", "development", "none", default="production") +# Default to "development" to avoid accidentally publishing to the production +# archive. Users must explicitly set "production" to publish there. +simulation_type = option("production", "development", "none", default="development") [climo] exclude = boolean(default=False) From 162d4e3516b01ea27993f909a5ae3a410f293f87 Mon Sep 17 00:00:00 2001 From: Ryan Forsyth Date: Thu, 30 Jul 2026 17:17:26 -0500 Subject: [PATCH 15/18] Minor doc rewording --- docs/source/user_guide/tasks/simboard.rst | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/docs/source/user_guide/tasks/simboard.rst b/docs/source/user_guide/tasks/simboard.rst index 0ba14f23..235493c8 100644 --- a/docs/source/user_guide/tasks/simboard.rst +++ b/docs/source/user_guide/tasks/simboard.rst @@ -102,8 +102,7 @@ under ``development``, so production is an explicit opt-in. To promote a run's diagnostics to the production archive: -1. Update ``simulation_type = production`` in the ``[simboard]`` section of - your ``zppy`` configuration file and re-run ``zppy``. +1. Update ``simulation_type`` to be ``production`` on the SimBoard UI itself. 2. Manually move (or copy) the existing diagnostic output from ``/diagnostics_archive/development//`` to ``/diagnostics_archive/production//``. From d0935cc40ef71e8ab1a2934387221058d20f078f Mon Sep 17 00:00:00 2001 From: chengzhuzhang Date: Wed, 5 Aug 2026 15:50:36 -0500 Subject: [PATCH 16/18] Skip www writes on a dry run A dry run created `//` and copied the provenance cfg/settings there before any check of `dry_run`, so it published artifacts for a run that never launched a job. This matters more with the new `[simboard]` section: when `enabled = True` and `www` is empty, `www` is inferred to the shared, machine-wide diagnostics_archive, so a dry run wrote into the real publishing tree rather than a path the user chose. The `output` script directory is untouched -- a dry run still writes the generated scripts and settings there, which is the point of a dry run. Co-Authored-By: Claude Opus 5 --- docs/source/user_guide/parameters.rst | 2 +- zppy/__main__.py | 37 +++++++++++++++------------ 2 files changed, 22 insertions(+), 17 deletions(-) diff --git a/docs/source/user_guide/parameters.rst b/docs/source/user_guide/parameters.rst index 974b7ba6..786ad39d 100644 --- a/docs/source/user_guide/parameters.rst +++ b/docs/source/user_guide/parameters.rst @@ -84,7 +84,7 @@ There are 6 output-specific parameters: * - ``dry_run`` - No - ``False`` - - This should be set to True if you don't want the batch jobs to be submitted. I.e., you only want to see what *would* be submitted. + - This should be set to True if you don't want the batch jobs to be submitted. I.e., you only want to see what *would* be submitted. A dry run does not write anything to ``www``, since that is a shared, published location; the generated scripts and settings still go to ``output``. * - ``fail_on_dependency_skip`` - No - ``False`` diff --git a/zppy/__main__.py b/zppy/__main__.py index a4f68d1f..d33a4990 100644 --- a/zppy/__main__.py +++ b/zppy/__main__.py @@ -92,22 +92,27 @@ def main(): shutil.copy(args.config, provenance) write_provenance_settings(provenance_settings, provenance_extras) # Web output directory - www = config["default"]["www"] - username = os.environ.get("USER") - www = www.replace("$USER", username) - www_case_dir = os.path.join(www, config["default"]["case"]) - www_provenance = os.path.join(www_case_dir, f"provenance.{ts_utc}.cfg") - www_provenance_settings = os.path.join( - www_case_dir, f"provenance.{ts_utc}.settings" - ) - try: - os.makedirs(www_case_dir) - except OSError as exc: - if exc.errno != errno.EEXIST: - raise OSError("Cannot create www case directory") - shutil.copy(args.config, www_provenance) - if os.path.isfile(provenance_settings): - shutil.copy(provenance_settings, www_provenance_settings) + # A dry run must not touch `www`. It is a shared, published location -- + # with `[simboard] enabled = True` it is inferred to the machine-wide + # diagnostics_archive -- so creating directories and copying provenance + # there would publish artifacts for a run that never happens. + if not config["default"]["dry_run"]: + www = config["default"]["www"] + username = os.environ.get("USER") + www = www.replace("$USER", username) + www_case_dir = os.path.join(www, config["default"]["case"]) + www_provenance = os.path.join(www_case_dir, f"provenance.{ts_utc}.cfg") + www_provenance_settings = os.path.join( + www_case_dir, f"provenance.{ts_utc}.settings" + ) + try: + os.makedirs(www_case_dir) + except OSError as exc: + if exc.errno != errno.EEXIST: + raise OSError("Cannot create www case directory") + shutil.copy(args.config, www_provenance) + if os.path.isfile(provenance_settings): + shutil.copy(provenance_settings, www_provenance_settings) if args.last_year: config["default"]["last_year"] = args.last_year _launch_scripts(config, script_dir, job_ids_file, plugins) From 40ac3d1b91f362e266a6d8d055029c31497e04ba Mon Sep 17 00:00:00 2001 From: chengzhuzhang Date: Wed, 5 Aug 2026 18:48:32 -0500 Subject: [PATCH 17/18] Add case_group to provenance and the inferred SimBoard path Read CASE_GROUP from env_case.xml into provenance, and use it as a level in the inferred www path: /diagnostics_archive/// CASE_GROUP is optional in CIME. When a simulation has none, zppy warns and publishes directly under /; the new `[default] case_group` parameter lets users supply one. Co-Authored-By: Claude Opus 5 --- docs/source/user_guide/parameters.rst | 4 ++ docs/source/user_guide/tasks/simboard.rst | 8 +++- tests/test_sections.py | 8 ++++ tests/test_zppy_main.py | 22 ++++++++++ tests/test_zppy_provenance.py | 39 ++++++++++++++++++ zppy/__main__.py | 18 +++++++- zppy/defaults/default.ini | 5 +++ zppy/provenance.py | 50 +++++++++++++++++++++-- zppy/simboard.py | 31 +++++++++++++- 9 files changed, 177 insertions(+), 8 deletions(-) diff --git a/docs/source/user_guide/parameters.rst b/docs/source/user_guide/parameters.rst index 786ad39d..895c1cca 100644 --- a/docs/source/user_guide/parameters.rst +++ b/docs/source/user_guide/parameters.rst @@ -81,6 +81,10 @@ There are 6 output-specific parameters: - No - ``False`` - Set to True to have ``zppy`` produce more verbose output and retain temporary workdirs. This is helpful for debugging. + * - ``case_group`` + - No + - ``""`` + - The group this simulation belongs to (e.g. ``"v3.LR"``). Normally read from ``CASE_GROUP`` in ``env_case.xml``; set this only when the simulation has no ``CASE_GROUP`` there, or to override it. When set, SimBoard-inferred ``www`` paths gain a ```` level. * - ``dry_run`` - No - ``False`` diff --git a/docs/source/user_guide/tasks/simboard.rst b/docs/source/user_guide/tasks/simboard.rst index 235493c8..b7f57b60 100644 --- a/docs/source/user_guide/tasks/simboard.rst +++ b/docs/source/user_guide/tasks/simboard.rst @@ -15,7 +15,13 @@ Mache for the current machine: .. code-block:: text - /diagnostics_archive// + /diagnostics_archive/// + +```` is included only when the simulation has one. It is read from +``CASE_GROUP`` in ``env_case.xml`` (e.g. ``v3.LR``), falling back to the +``case_group`` parameter in ``[default]``. ``CASE_GROUP`` is optional in CIME, +so when neither is set ``zppy`` warns and publishes directly under +``/``. This gives SimBoard a single, predictable archive root to scan for diagnostics. diff --git a/tests/test_sections.py b/tests/test_sections.py index ca4cff24..397169b9 100644 --- a/tests/test_sections.py +++ b/tests/test_sections.py @@ -69,6 +69,7 @@ def test_sections(): "bundle": "", "campaign": "none", "case": "CASE", + "case_group": "", "constraint": "", "debug": False, "dry_run": False, @@ -145,6 +146,7 @@ def test_sections(): "area_nm": "area", "campaign": "none", "case": "CASE", + "case_group": "", "constraint": "", "debug": False, "default_case": "CASE", @@ -217,6 +219,7 @@ def test_sections(): "bundle": "", "campaign": "none", "case": "CASE", + "case_group": "", "climo_jobs": 0, "constraint": "", "debug": False, @@ -286,6 +289,7 @@ def test_subsections(): "bundle": "", "campaign": "none", "case": "CASE", + "case_group": "", "constraint": "", "debug": False, "dry_run": False, @@ -377,6 +381,7 @@ def test_subsections(): "bundle": "", "campaign": "none", "case": "CASE", + "case_group": "", "constraint": "", "debug": False, "default_case": "CASE", @@ -432,6 +437,7 @@ def test_subsections(): "bundle": "", "campaign": "none", "case": "CASE", + "case_group": "", "constraint": "", "debug": False, "default_case": "CASE", @@ -522,6 +528,7 @@ def test_subsections(): "bundle": "", "campaign": "none", "case": "CASE", + "case_group": "", "climo_jobs": 0, "constraint": "", "debug": False, @@ -571,6 +578,7 @@ def test_subsections(): "bundle": "", "campaign": "none", "case": "CASE", + "case_group": "", "climo_jobs": 0, "constraint": "", "debug": False, diff --git a/tests/test_zppy_main.py b/tests/test_zppy_main.py index 1cb133c8..a31c5e47 100644 --- a/tests/test_zppy_main.py +++ b/tests/test_zppy_main.py @@ -279,3 +279,25 @@ def test_default_ini_rejects_invalid_simulation_type(tmp_path: Path) -> None: assert result is not True assert result["simboard"]["simulation_type"] is False + + +@pytest.mark.parametrize( + ("case_group", "expected_suffix"), + [ + ("v3.LR", "diagnostics_archive/production/v3.LR/"), + ("", "diagnostics_archive/production/"), + ], +) +def test_infer_simboard_www_groups_by_case_group( + case_group: str, expected_suffix: str +) -> None: + inferred = infer_simboard_www(_fake_machine_info(), _base_config(), case_group) + + assert inferred == f"/global/cfs/cdirs/e3sm/www/{expected_suffix}" + + +def test_infer_simboard_www_rejects_multi_component_case_group() -> None: + # The case group becomes a single directory name, so it must not be able to + # redirect output elsewhere in the archive. + with pytest.raises(ValueError, match="Invalid case_group"): + infer_simboard_www(_fake_machine_info(), _base_config(), "v3.LR/historical") diff --git a/tests/test_zppy_provenance.py b/tests/test_zppy_provenance.py index c48e9b4a..18e03338 100644 --- a/tests/test_zppy_provenance.py +++ b/tests/test_zppy_provenance.py @@ -9,6 +9,7 @@ build_diagnostics_url, build_provenance_extras, parse_env_case_xml, + resolve_case_group, write_provenance_settings, ) @@ -60,6 +61,23 @@ def _fake_machine_info( return mi +# --------------------------------------------------------------------------- +# resolve_case_group +# --------------------------------------------------------------------------- + + +def test_resolve_case_group_prefers_env_case_xml(): + assert resolve_case_group({"case_group": "ignored"}, "v3.LR") == "v3.LR" + + +def test_resolve_case_group_falls_back_to_cfg(): + assert resolve_case_group({"case_group": "v3.HR"}, "") == "v3.HR" + + +def test_resolve_case_group_returns_empty_when_unset(): + assert resolve_case_group({}, "") == "" + + # --------------------------------------------------------------------------- # parse_env_case_xml # --------------------------------------------------------------------------- @@ -78,6 +96,27 @@ def test_parse_env_case_xml_happy(tmp_path): } +def test_parse_env_case_xml_reads_case_group(tmp_path): + _write_env_case_xml( + str(tmp_path), + { + "CASE": "v3.LR.historical_0051", + "MACH": "chrysalis", + "REALUSER": "ac.wlin", + "CASE_GROUP": "v3.LR", + }, + ) + assert parse_env_case_xml(str(tmp_path))["case_group"] == "v3.LR" + + +def test_parse_env_case_xml_omits_empty_case_group(tmp_path): + # CASE_GROUP is optional in CIME, so the entry is often present but empty. + _write_env_case_xml( + str(tmp_path), {"CASE": "v3.LR.historical_0051", "CASE_GROUP": ""} + ) + assert "case_group" not in parse_env_case_xml(str(tmp_path)) + + def test_parse_env_case_xml_missing_file(tmp_path): # No case_scripts dir at all. assert parse_env_case_xml(str(tmp_path)) == {} diff --git a/zppy/__main__.py b/zppy/__main__.py index d33a4990..4f01b61b 100644 --- a/zppy/__main__.py +++ b/zppy/__main__.py @@ -23,7 +23,12 @@ from zppy.logger import _setup_custom_logger from zppy.mpas_analysis import mpas_analysis from zppy.pcmdi_diags import pcmdi_diags -from zppy.provenance import build_provenance_extras, write_provenance_settings +from zppy.provenance import ( + build_provenance_extras, + parse_env_case_xml, + resolve_case_group, + write_provenance_settings, +) from zppy.simboard import ( infer_simboard_www, simboard, @@ -297,7 +302,16 @@ def _set_default_www(machine_info: MachineInfo, config: ConfigObj) -> None: "web_portal.base_path in Mache configuration." ) - config["default"]["www"] = infer_simboard_www(machine_info, config) + # The case group adds a grouping level to the inferred path. It comes from + # env_case.xml, falling back to cfg `case_group`; `resolve_case_group` + # warns when neither is set. + config_default = config["default"] + input_dir = config_default.get("input", "") + xml_case_group = ( + parse_env_case_xml(input_dir).get("case_group", "") if input_dir else "" + ) + case_group = resolve_case_group(config_default, xml_case_group) + config["default"]["www"] = infer_simboard_www(machine_info, config, case_group) def _launch_scripts(config: ConfigObj, script_dir, job_ids_file, plugins) -> None: diff --git a/zppy/defaults/default.ini b/zppy/defaults/default.ini index 095095e9..bcdc1e65 100755 --- a/zppy/defaults/default.ini +++ b/zppy/defaults/default.ini @@ -10,6 +10,11 @@ campaign = string(default="none") # The case name of the simulation # NOTE: no default, must be provided by user case = string +# The group this simulation belongs to (e.g. "v3.LR"). +# Normally read from CASE_GROUP in env_case.xml; set this only when the +# simulation has no CASE_GROUP there, or to override it. +# When set, SimBoard-inferred `www` paths gain a level. +case_group = string(default="") # The constraint of the machine to run on constraint = string(default="") # Set to True to keep temporary directories/files after zppy completes diff --git a/zppy/provenance.py b/zppy/provenance.py index 6106fa6e..74214818 100644 --- a/zppy/provenance.py +++ b/zppy/provenance.py @@ -20,6 +20,7 @@ "case_name": "CASE", "machine": "MACH", "hpc_username": "REALUSER", + "case_group": "CASE_GROUP", } @@ -51,13 +52,16 @@ def parse_env_case_xml(input_dir: str) -> Dict[str, str]: # CIME nests elements inside wrappers, so we # need a descendant search rather than a direct-child lookup. entry = root.find(f".//entry[@id='{entry_id}']") - if entry is None or entry.get("value") is None: + # CASE_GROUP is optional in CIME, so the entry is often present but + # empty. Treat that the same as absent. + value = "" if entry is None else (entry.get("value") or "").strip() + if not value: logger.warning( f"env_case.xml at {xml_path} has no '{entry_id}' entry; " f"'{field}' will be omitted from provenance." ) continue - values[field] = entry.get("value", "") + values[field] = value return values @@ -110,12 +114,46 @@ def write_provenance_settings( f.write(f"{key} = {value}\n") +def resolve_case_group(config_default: Dict[str, str], xml_case_group: str) -> str: + """Return the case group, preferring `env_case.xml` over the cfg. + + `env_case.xml` is authoritative when it has a `CASE_GROUP`, matching how + `case_name` is handled. Cfg `case_group` is the fallback for simulations + that were never assigned one -- it is optional in CIME, so plenty of cases + have no value. When neither is set, warn: without a case group, SimBoard + output lands directly under `/` rather than being grouped. + """ + cfg_case_group = (config_default.get("case_group", "") or "").strip() + + if xml_case_group: + if cfg_case_group and cfg_case_group != xml_case_group: + logger.warning( + f"cfg case_group='{cfg_case_group}' does not match env_case.xml " + f"CASE_GROUP='{xml_case_group}'; using the env_case.xml value." + ) + return xml_case_group + + if cfg_case_group: + return cfg_case_group + + logger.warning( + "No case group found: env_case.xml has no CASE_GROUP and cfg " + "'case_group' is unset. Set `case_group` in [default] (e.g. " + '`case_group = "v3.LR"`) to group this simulation in the SimBoard ' + "archive; otherwise its output is published directly under the " + "simulation type." + ) + return "" + + def build_provenance_extras( config_default: Dict[str, str], machine_info: MachineInfo ) -> Dict[str, str]: """Assemble the dict of extra provenance metadata fields. - - `case_name`, `machine`, `hpc_username` from `env_case.xml` under cfg `input`. + - `case_name`, `machine`, `hpc_username`, `case_group` from `env_case.xml` + under cfg `input`. + - `case_group` falls back to cfg `case_group` when `env_case.xml` has none. - `diagnostics_url` from cfg `www` + `case` + machine `web_portal` config. - Warns (but does not fail) when cfg `case` disagrees with env_case.xml `CASE`. """ @@ -139,6 +177,12 @@ def build_provenance_extras( f"using env_case.xml value as authoritative case_name." ) + case_group = resolve_case_group(config_default, extras.get("case_group", "")) + if case_group: + extras["case_group"] = case_group + else: + extras.pop("case_group", None) + diag_url = build_diagnostics_url(www, case, machine_info) if diag_url: extras["diagnostics_url"] = diag_url diff --git a/zppy/simboard.py b/zppy/simboard.py index f425fcef..de4488df 100644 --- a/zppy/simboard.py +++ b/zppy/simboard.py @@ -14,6 +14,25 @@ def normalize_web_portal_base_path(web_portal_base_path: str) -> str: return web_portal_base_path.strip().rstrip("/") +def _normalize_case_group(case_group: str) -> str: + """Return the trailing-slashed path segment for a case group, or "". + + A case group comes from `env_case.xml` or the cfg, so it is free-form text. + Only a single path component is meaningful here; anything containing a + separator is rejected rather than silently used to build a deeper tree. + """ + case_group = (case_group or "").strip().strip("/") + if not case_group: + return "" + if "/" in case_group or case_group in (".", ".."): + raise ValueError( + f"Invalid case_group '{case_group}': it becomes a single directory " + "name in the SimBoard archive path, so it cannot contain '/' or be " + "'.' or '..'." + ) + return f"{case_group}/" + + def simboard( config: ConfigObj, _script_dir: str, @@ -71,7 +90,9 @@ def validate_simboard_config(config: ConfigObj) -> None: ) -def infer_simboard_www(machine_info: MachineInfo, config: ConfigObj) -> str: +def infer_simboard_www( + machine_info: MachineInfo, config: ConfigObj, case_group: str = "" +) -> str: simulation_type = config["simboard"]["simulation_type"] try: web_portal_base_path = machine_info.config.get("web_portal", "base_path") @@ -90,7 +111,13 @@ def infer_simboard_www(machine_info: MachineInfo, config: ConfigObj) -> str: "mache; cannot infer a diagnostics_archive path." ) - inferred_www = f"{web_portal_base_path}/diagnostics_archive/{simulation_type}/" + # Group the simulation under its case group when it has one, so SimBoard + # sees e.g. `.../production/v3.LR//` instead of a flat list of cases. + case_group_segment = _normalize_case_group(case_group) + inferred_www = ( + f"{web_portal_base_path}/diagnostics_archive/" + f"{simulation_type}/{case_group_segment}" + ) logger.info( "Inferred www=%s from mache web_portal.base_path because " "simboard.enabled is True.", From 9f6415b15acee632f711cc4499d7e286b909520c Mon Sep 17 00:00:00 2001 From: Ryan Forsyth Date: Fri, 21 Aug 2026 19:55:46 -0500 Subject: [PATCH 18/18] docs(simboard): align zppy SimBoard page with new diagnostics linkage guide Update zppy/docs/source/user_guide/tasks/simboard.rst to incorporate SimBoard's new diagnostics.md guidance, per the SimBoard developer's review comment on this PR. - Add a pre-publish checklist: confirm the case exists in SimBoard, verify provenance (case_name, machine, hpc_username) matches, and apply the grouped/ungrouped archive layout rule. - Add a publishing section covering the provenance.settings file, output verification, and the periodic (~15 min) SimBoard scanner that performs linkage. - Add a "Stable URLs" section describing link stability across content updates, and the manual steps required when output is moved or deleted. - Add a troubleshooting section for missing links, links to the wrong output, and dead links, with a pointer to SimBoard's diagnostics linkage architecture doc. - Keep existing zppy-specific configuration content (www inference, parameters table, dev-to-production promotion) unchanged, with a clarifying note that promotion is a zppy-side archive move, not a SimBoard link update. No functional/code changes; docs only. --- docs/source/user_guide/tasks/simboard.rst | 100 ++++++++++++++++++++++ 1 file changed, 100 insertions(+) diff --git a/docs/source/user_guide/tasks/simboard.rst b/docs/source/user_guide/tasks/simboard.rst index b7f57b60..28a91be6 100644 --- a/docs/source/user_guide/tasks/simboard.rst +++ b/docs/source/user_guide/tasks/simboard.rst @@ -96,6 +96,56 @@ Parameters .. note:: The ``[simboard]`` section does not support subsections. +Before you publish +------------------- + +SimBoard links diagnostics to an *existing* SimBoard case; it does not +create the case for you. Before running zppy with ``[simboard] enabled = +True``, confirm the following: + +1. The intended case is already visible in SimBoard. If it is not, + contact the SimBoard administrator (`Tom Vo `_) + before publishing. +2. The provenance that zppy will record — ``case_name``, ``machine``, and + ``hpc_username`` — matches that SimBoard case. +3. The archive layout that results from your ``[simboard]`` and + ``[default]`` settings agrees with that provenance: + + - Ungrouped output must land at ``/``. + - Grouped output must land at + ``//``, using the ``CASE_GROUP`` + value from your E3SM run script configuration (see the ```` + inference described above). ``CASE_GROUP`` is not itself a zppy + configuration option — zppy only reads it to build the path. + +If the layout and the provenance disagree, SimBoard's discovery process +will not find the output, even if the diagnostics are otherwise published +correctly. + +Publishing diagnostics and linking the case +-------------------------------------------- + +Once ``[simboard]`` is configured and the checklist above is satisfied: + +1. Run and publish the zppy diagnostics using the configured + ``simulation_type``. This produces the ``provenance.settings`` file + that SimBoard uses to discover and link the output. +2. Confirm the published diagnostics output is complete and opens + successfully in a browser. +3. Confirm the completed output is at the archive path matching the + grouped or ungrouped layout described above. +4. Wait for the scheduled SimBoard scanner to link the case — linking is + not immediate, and the scanner runs periodically (currently every 15 + minutes). +5. Once the link appears, open the case in SimBoard and follow its + diagnostics link. + +SimBoard's discovery always uses the *latest valid* provenance for a +published diagnostics case. If a run's provenance is incomplete or +invalid, re-run and re-publish the zppy diagnostics to regenerate it +rather than editing the provenance file by hand — manually edited or +stale provenance files are not used for discovery. + .. _simboard-promotion: Promoting diagnostics from development to production @@ -112,3 +162,53 @@ To promote a run's diagnostics to the production archive: 2. Manually move (or copy) the existing diagnostic output from ``/diagnostics_archive/development//`` to ``/diagnostics_archive/production//``. + +This move/copy is the only supported way to promote diagnostics. +Promotion is a zppy-side archive change, not a SimBoard link update — do +not expect SimBoard to move or re-link existing output on its own. + +Stable URLs and moved, deleted, or missing output +--------------------------------------------------- + +The external URL SimBoard links to is stable for a given published case +path: once a case is first linked, updating the content at that same +path keeps working with the existing link. + +If diagnostics output is later deleted or moved to a different path: + +- Restore the output at its original URL to keep the existing SimBoard + link working, **or** +- Manually update or remove the link in SimBoard. + +SimBoard does not dynamically check for or remove links whose external +output has become unavailable, so a link left pointing at deleted or +moved output will continue to appear valid in SimBoard until it is +corrected. + +Troubleshooting +---------------- + +**The case does not receive a diagnostics link.** +Check, in order: the configured ``simulation_type``; whether the output +follows the correct grouped or ungrouped archive layout; whether the +latest provenance and its paired settings file are present and valid; +whether the case identity (``case_name``, ``machine``, ``hpc_username``) +matches the SimBoard case; and whether the completed output is publicly +accessible. If the link is still missing after checking all of these, +contact `Tom Vo `_. + +**The link opens the wrong output.** +Check ``simulation_type``, ``case_group``, and the published path. +SimBoard does not semantically validate whether the ``simulation_type`` +you chose is appropriate for the output — an incorrect value will still +produce a link, just to the wrong place. + +**The link no longer opens.** +Restore the output at its original URL, or manually update or remove the +SimBoard link — see `Stable URLs and moved, deleted, or missing output`_ +above. + +For SimBoard scanner implementation details beyond zppy's configuration, +see SimBoard's own `Diagnostics Linkage Architecture +`_ +documentation.