Skip to content

Refactor Omega's buildnml into a validated, testable omega_buildnml package - #494

Open
andrewdnolan wants to merge 19 commits into
E3SM-Project:developfrom
andrewdnolan:omega/buildnml-build-out
Open

Refactor Omega's buildnml into a validated, testable omega_buildnml package#494
andrewdnolan wants to merge 19 commits into
E3SM-Project:developfrom
andrewdnolan:omega/buildnml-build-out

Conversation

@andrewdnolan

@andrewdnolan andrewdnolan commented Aug 3, 2026

Copy link
Copy Markdown

Summary

Extract components/omega/cime_config/buildnml's logic into a new omega_buildnml Python package, adding validation of all input configuration, support for user_nl_omega, unit tests, a CI workflow, and developer/user documentation.

Refactor

  • Move the config-building logic out of buildnml and into omega_buildnml, split across read_write.py (reading/writing YAML), config.py (merging config layers, resolving mesh input files), and validate.py (validation).
  • Move mesh input file mappings and coupled/mesh overrides out of buildnml into packaged data files, data/input_files.yaml and data/config_overrides.yaml

Validation

  • Validate config_overrides.yaml structure and its options against configs/Default.yml, so overrides can't silently set unknown options.
  • Validate that validate.py's KNOWN_STREAMS hasn't drifted from the IOStreams actually defined in Default.yml.
  • Fail fast on duplicate keys when parsing YAML files.
  • Add validate_config.py, a CLI entry point that validates every mesh and override entry in the packaged configuration files (rather than just the ones needed for a single case).

user_nl_omega support

  • Read and validate user overrides from user_nl_omega, merging them in as the highest-precedence configuration layer.
  • Enforce that a fixed set of options controlled by CIME/the coupler (e.g. TimeIntegration.StartTime, restart/forcing IOStreams) cannot be overridden by users.

Testing and CI

  • Add unit tests for config.py, read_write.py, and validate.py.
  • Add the omega-buildnml GitHub Actions workflow, running validate_config.py and the unit tests (across supported Python versions) on pull requests touching cime_config/ or Default.yml.

Documentation

  • Add READMEs for omega_buildnml/ and omega_buildnml/data/.
  • Add a new devGuide page covering buildnml, configuration validation, CI, and how to add a new supported mesh.
  • Add a "Coupled Run Configuration (user_nl_omega)" section to the userGuide's Config.md, describing the configuration layering and which options are blocked from user override.

Checklist

  • Documentation:

  • Linting

  • Provide relevant details in a comment to the PR titled Testing with the following:

    • Which machines CTest unit tests have been run on and indicate that are all passing.
    • The Polaris omega_pr test suite has passed, using the Polaris e3sm_submodules/Omega baseline
    • Document machine(s), compiler(s), and the build path(s) used for -p for both the baseline (Polaris e3sm_submodules/Omega) and the PR build
    • Indicate "All tests passed" or document failing tests
    • Document testing used to verify the changes including any tests that are added/modified/impacted.

@andrewdnolan
andrewdnolan marked this pull request as draft August 3, 2026 21:28
@andrewdnolan
andrewdnolan force-pushed the omega/buildnml-build-out branch 5 times, most recently from 03a951b to 8353ebe Compare August 4, 2026 17:31
@andrewdnolan andrewdnolan changed the title Build out functionality and testing of buildnml Refactor Omega's buildnml into a validated, testable omega_buildnml package Aug 4, 2026
@andrewdnolan
andrewdnolan marked this pull request as ready for review August 4, 2026 17:46
@andrewdnolan
andrewdnolan requested review from cbegeman and xylar and a lite review from Copilot August 4, 2026 17:46

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Refactors Omega’s CIME buildnml into a standalone omega_buildnml Python package that cleanly separates config reading/merging from validation, adds user_nl_omega support, and introduces CI + documentation to keep packaged configuration and Default.yml consistent over time.

Changes:

  • Extract buildnml logic into components/omega/cime_config/omega_buildnml/ with explicit validation and YAML duplicate-key rejection.
  • Add validate_config.py, packaged YAML data files for meshes/overrides, and pytest unit coverage for the new package.
  • Update user/developer docs and add a dedicated GitHub Actions workflow for validation + unit tests.

Reviewed changes

Copilot reviewed 22 out of 23 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
components/omega/doc/userGuide/Config.md Documents user_nl_omega layering/precedence and blocked override options.
components/omega/doc/index.md Adds the new BuildNml dev guide page to the docs index.
components/omega/doc/devGuide/BuildNml.md New developer documentation for buildnml, validation, CI, and mesh support.
components/omega/dev-conda.txt Adds pytest/PyYAML to the Omega dev conda environment.
components/omega/cime_config/validate_config.py New CLI validator to check all packaged meshes/overrides + KNOWN_STREAMS.
components/omega/cime_config/user_nl_omega Converts template guidance to YAML-based overrides and documents blocked settings.
components/omega/cime_config/omega_buildnml/validate.py Implements validation rules for packaged configs and user overrides.
components/omega/cime_config/omega_buildnml/tests/test_validate_overrides.py Unit tests for overrides validation + blocked options + known-streams drift.
components/omega/cime_config/omega_buildnml/tests/test_validate_input_files.py Unit tests for mesh input-file validation rules.
components/omega/cime_config/omega_buildnml/tests/test_read_write.py Unit tests for duplicate-key YAML parsing and user_nl_omega parsing.
components/omega/cime_config/omega_buildnml/tests/test_config.py Unit tests for merge precedence and blocked-options invariants.
components/omega/cime_config/omega_buildnml/tests/init.py Marks tests as a package.
components/omega/cime_config/omega_buildnml/README.md Documents package purpose, dependency constraints, and local dev/test steps.
components/omega/cime_config/omega_buildnml/read_write.py Adds YAML read/write helpers, packaged-data reading, and unique-key YAML loader.
components/omega/cime_config/omega_buildnml/data/README.md Documents packaged YAML data purpose and validation.
components/omega/cime_config/omega_buildnml/data/input_files.yaml Defines mesh→input-file mappings for required IOStreams.
components/omega/cime_config/omega_buildnml/data/config_overrides.yaml Defines coupled and mesh-specific override layers.
components/omega/cime_config/omega_buildnml/config.py Implements layer merging plus runtime overrides and stream file resolution.
components/omega/cime_config/omega_buildnml/_types.py Adds shared typing aliases.
components/omega/cime_config/omega_buildnml/init.py Exposes the package public API (__all__).
components/omega/cime_config/buildnml Switches CIME entrypoint to use omega_buildnml package + user_nl_omega.
.pre-commit-config.yaml Adds types-PyYAML for mypy; adjusts flake8 hook deps.
.github/workflows/omega-buildnml-workflow.yml Adds CI workflow to validate configs and run pytest across Python versions.
Suppressed comments (1)

components/omega/cime_config/omega_buildnml/read_write.py:28

  • The example path in this docstring points to components/omega/config/Defaults.yml, but the repo uses components/omega/configs/Default.yml. Keeping the example accurate helps prevent copy/paste mistakes.
        Path to default config file (i.e. components/omega/config/Defaults.yml)

Comment thread components/omega/cime_config/omega_buildnml/validate.py Outdated
Comment thread components/omega/cime_config/omega_buildnml/read_write.py Outdated
Comment thread components/omega/cime_config/omega_buildnml/config.py Outdated
Comment thread components/omega/cime_config/omega_buildnml/read_write.py Outdated
Comment thread components/omega/cime_config/omega_buildnml/read_write.py Outdated
Comment thread components/omega/cime_config/omega_buildnml/read_write.py Outdated
Comment thread components/omega/cime_config/omega_buildnml/config.py Outdated
Comment thread components/omega/cime_config/omega_buildnml/data/input_files.yaml Outdated

@xylar xylar left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@andrewdnolan, this looks great! I'm really happy with the direction it's taking.

Just one real topic to discuss, and then a few small comments or questions.

Comment thread components/omega/cime_config/omega_buildnml/validate.py Outdated
Comment thread components/omega/cime_config/omega_buildnml/validate.py Outdated
Comment thread .pre-commit-config.yaml Outdated
@andrewdnolan

Copy link
Copy Markdown
Author

@xylar A follow-up to correct/clarify my earlier reply: I overstated the bug a bit.

KNOWN_STREAMS was only ever checked in input_files.yaml validation (plus a CI-only drift check confirming KNOWN_STREAMS matched Default.yml). user_nl_omega and config_overrides.yaml's coupled section were always validated through validate_overrides(), where IOStreams is an OPEN_SECTIONS entry — custom stream names there (like MyCustomHiFreq in my example above) were never checked against KNOWN_STREAMS or any fixed list. That path was never actually broken.

The real, narrower bug: input_files.yaml used a hardcoded, driftable KNOWN_STREAMS list instead of validating stream names dynamically against Default.yml. That's now fixed — validate_input_files_config() takes the parsed Default.yml and checks stream names against defaults["IOStreams"] directly, so there's no list to keep in sync anymore. input_files.yaml streams still have to exist in Default.yml (that file only overrides a Filename; the rest of the stream's config comes from the default entry), but a brand-new stream introduced in user_nl_omega or config_overrides.yaml's coupled section doesn't need a corresponding entry anywhere else first.

Separately, while looking at this I noticed config_overrides.yaml's per-mesh entries had never been restricted from setting IOStreams at all, even though setting up a whole IOStream differently per-mesh isn't really a supported use case. I added an explicit check rejecting IOStreams under a mesh entry there, pointing to coupled or user_nl_omega instead.

Also added omega_buildnml/tests/test_validate_user_overrides.py, including test_custom_iostreams_are_allowed, which exercises validate_user_overrides() directly with a custom stream (mirroring the MyCustomHiFreq example above) as a regression test, per your suggestion above.

@andrewdnolan
andrewdnolan force-pushed the omega/buildnml-build-out branch from b5bdac1 to 0418b03 Compare August 5, 2026 15:39
@andrewdnolan

Copy link
Copy Markdown
Author

@xylar Just pushed a rebased version, with ruff formatting applied, following the merge of #498.

@xylar xylar left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

With the known streams things and the linting done after the rebase, I'm happy to approve!

@xylar
xylar self-requested a review August 5, 2026 15:48
@andrewdnolan
andrewdnolan force-pushed the omega/buildnml-build-out branch from 0418b03 to 10e314d Compare August 5, 2026 17:02

@xylar xylar left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Re-approving now that KNOWN_STREAMS is gone from the code for real.

@cbegeman cbegeman left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Approving on the basis of code inspection. Let me know if any testing would be helpful. Thanks, @andrewdnolan !

@andrewdnolan
andrewdnolan force-pushed the omega/buildnml-build-out branch from 647770d to 11b8a8e Compare August 12, 2026 16:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants