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 ee44b65a..895c1cca 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"`` @@ -79,10 +81,14 @@ 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`` - - 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`` @@ -404,6 +410,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 + - ``"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/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..28a91be6 --- /dev/null +++ b/docs/source/user_guide/tasks/simboard.rst @@ -0,0 +1,214 @@ +.. _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/// + +```` 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. + +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 = development + +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 + - ``"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. + +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 +----------------------------------------------------- + +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`` 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//``. + +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. 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/tests/test_sections.py b/tests/test_sections.py index 8b34fd42..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, @@ -107,6 +108,15 @@ def test_sections(): } compare(actual_default, expected_default) + # simboard + section_name = "simboard" + actual_section = config[section_name] + expected_section = { + "enabled": False, + "simulation_type": "development", + } + compare(actual_section, expected_section) + # ts section_name = "ts" actual_section = config[section_name] @@ -136,6 +146,7 @@ def test_sections(): "area_nm": "area", "campaign": "none", "case": "CASE", + "case_group": "", "constraint": "", "debug": False, "default_case": "CASE", @@ -208,6 +219,7 @@ def test_sections(): "bundle": "", "campaign": "none", "case": "CASE", + "case_group": "", "climo_jobs": 0, "constraint": "", "debug": False, @@ -277,6 +289,7 @@ def test_subsections(): "bundle": "", "campaign": "none", "case": "CASE", + "case_group": "", "constraint": "", "debug": False, "dry_run": False, @@ -368,6 +381,7 @@ def test_subsections(): "bundle": "", "campaign": "none", "case": "CASE", + "case_group": "", "constraint": "", "debug": False, "default_case": "CASE", @@ -423,6 +437,7 @@ def test_subsections(): "bundle": "", "campaign": "none", "case": "CASE", + "case_group": "", "constraint": "", "debug": False, "default_case": "CASE", @@ -513,6 +528,7 @@ def test_subsections(): "bundle": "", "campaign": "none", "case": "CASE", + "case_group": "", "climo_jobs": 0, "constraint": "", "debug": False, @@ -562,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 new file mode 100644 index 00000000..a31c5e47 --- /dev/null +++ b/tests/test_zppy_main.py @@ -0,0 +1,303 @@ +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 +from zppy.simboard import ( + infer_simboard_www, + normalize_web_portal_base_path, + simboard, + simboard_enabled, +) + + +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, + "www": "", + }, + "simboard": { + "enabled": False, + "simulation_type": "production", + }, + } + + +@pytest.mark.parametrize( + ("simulation_type", "expected_www"), + [ + ( + "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( + simulation_type: str, expected_www: str +) -> None: + config = _base_config() + config["simboard"]["enabled"] = True + config["simboard"]["simulation_type"] = simulation_type + + updated = _determine_parameters(_fake_machine_info(), config) + + 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" + 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() + + with pytest.raises( + ValueError, + match=( + r"www is empty\. Provide \[default\] www or set `enabled = True` " + r"in the \[simboard\] section" + ), + ): + _determine_parameters(_fake_machine_info(), config) + + +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_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" + 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", + "", + " [[nested]]", + " placeholder = value", + ] + ) + ) + config = ConfigObj(str(config_path), configspec=str(default_ini)) + + 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" + ) + config_path.write_text( + "\n".join( + [ + "[default]", + "case = case_name", + "input = /input", + "output = /output", + "www = /www", + "", + "[simboard]", + "simulation_type = invalid", + ] + ) + ) + config = ConfigObj( + str(config_path), + configspec=str(default_ini), + ) + + result = config.validate(Validator()) + + 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 61d9ede1..4f01b61b 100644 --- a/zppy/__main__.py +++ b/zppy/__main__.py @@ -1,4 +1,5 @@ import argparse +import configparser import errno import importlib import io @@ -22,7 +23,18 @@ 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, + 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 @@ -85,22 +97,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) @@ -219,12 +236,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"] == ""): @@ -259,15 +282,47 @@ 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: + # Keep SimBoard-specific validation active even when `www` is already set, + # because `[simboard] enabled = True` still requires validating + # `simulation_type`. + validate_simboard_config(config) + if config["default"]["www"] != "": + return + + if not simboard_enabled(config): + raise ValueError( + "www is empty. Provide [default] www or set `enabled = True` in " + "the [simboard] section to infer a SimBoard-compatible " + "diagnostics_archive path. Note: inference requires " + "web_portal.base_path in Mache configuration." + ) + + # 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: existing_bundles: List[Bundle] = [] # 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 05800b22..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 @@ -106,8 +111,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 `/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("")) @@ -118,6 +124,14 @@ 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. +# 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) # NOTE: always overrides value in [default] 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 new file mode 100644 index 00000000..de4488df --- /dev/null +++ b/zppy/simboard.py @@ -0,0 +1,126 @@ +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 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, + 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. 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 + + +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. + """ + 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. Expected boolean " + "or string 'true'/'false'." + ) + + +def validate_simboard_config(config: ConfigObj) -> None: + if not simboard_enabled(config): + return + + simulation_type = 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, case_group: str = "" +) -> str: + simulation_type = 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 = 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 " + f"'{machine_info.machine}' has an empty web_portal.base_path in " + "mache; cannot infer a diagnostics_archive path." + ) + + # 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.", + inferred_www, + ) + return inferred_www