Skip to content

Replace the bash complete run driver with tests/complete_run - #876

Open
chengzhuzhang wants to merge 12 commits into
mainfrom
streamline-complete-run-test
Open

chengzhuzhang wants to merge 12 commits into
mainfrom
streamline-complete-run-test

Conversation

@chengzhuzhang

@chengzhuzhang chengzhuzhang commented Sep 21, 2026 •

Copy link
Copy Markdown
Collaborator

Replaces tests/main_branch_testing/run_integration_test.bash (844 lines of bash) with tests/complete_run/, a Python package of 14 modules.

How to use

One command runs everything:

python -m tests.complete_run.automation --machine chrysalis --account e3sm

Reading the review page

Example, from a run of main against the 20260918_unified113 baseline:

https://web.lcrc.anl.gov/public/e3sm/zppy_complete_run/runs/20260918_main_run1/

  • Environment comes first, because it decides how to read everything below. It says whether dependencies moved between the baseline and this run; packages that most often move results (python, numpy, xarray, matplotlib, …) are marked with a bullet and listed first. If it says dependencies changed, an image difference is not automatically the code under test's fault. This run: 597 changes, including python 3.13.13 → 3.14.7 and numpy 2.4.4 → 2.5.3, because a fresh dev solve is being compared against E3SM-Unified 1.13.0.
  • By package rolls every cfg and task up to one row per package, worst result first. Click a package name to jump to its checks.
  • By check is one row per cfg and task, with the full severity breakdown. Click a task name to open its images.
  • A task's page shows each changed plot as actual / expected / difference, worst first, filterable by severity, cause and name. Images that are identical or cosmetic are counted but not shown, so review starts with what actually changed.

Severity runs IDENTICAL → NEGLIGIBLE (cosmetic) → MINOR → MODERATE → MAJOR → STRUCTURAL → MISSING. Only MINOR and worse need review.

Useful flags:

Flag What it does
--cfg, --task Cut the run down to the cfgs and tasks a PR actually touches
--start-stage, --stop-after Resume or stop at one of prepare → generate → submit → bundles → validate → report
--env-type <repo>=dev|unified|baseline Solve the repo's dev.yml, use E3SM-Unified, or rebuild what the baseline used

When it finishes, open the run's review page.

Promoting a baseline

Expected results are not a copy of a run: baselines/latest-main is a symlink pointing at one. Promotion flips that link, so it is atomic, instant, and leaves the previous baseline intact and still promotable.

# What is the baseline right now?
python -m tests.complete_run.promote --machine chrysalis show

# Make a run the baseline, once its differences have been reviewed and accepted
python -m tests.complete_run.promote --machine chrysalis run 20260918_main_run1

# Free space: delete old, unpromoted runs (dry run without --delete)
python -m tests.complete_run.promote --machine chrysalis prune --keep 5 --delete

A run never promotes itself; this is always a separate, deliberate command. It refuses a run that has no report, one whose status is not passed (--allow-failed once you have decided the new plots are correct), and one built from a feature branch (--allow-non-main). prune never deletes a run a channel points at.

Known gap: partial promotion

A baseline is one whole run, so a task cannot be blessed on its own: if e3sm_diags changed legitimately while another package has an unexplained difference in the same run, both are blessed together or neither is.

Note that docs/source/dev_guide/tests/update_expected_results.rst currently suggests rerunning the complete test with just the cfg and task you need, then promoting that run. That does not work: a run's expected-image lists are built by walking its own www/ tree, so a task-limited run becomes a baseline containing only that task, and every other task then records no comparison. The doc fix belongs with whichever design below is chosen, so it is not in this PR.

Two designs were considered, neither implemented here:

  • A composite run — promote assembles a new run directory from the current baseline with the blessed task's plot directories symlinked from the newer run, regenerates the image lists from the merged tree, and records per-task provenance in the manifest. A baseline stays one run directory, so nothing downstream changes; env_description.txt is already per task, so each task keeps its own commit and Generated: date.
  • Per-task pins — a per-task baseline symlink resolved everywhere a baseline is read: cfg generation, the image checker, the environment comparison and the review page. More precise, about six modules, and a pin is easy to forget.

Major changes vs the current complete test

One resumable command instead of three manual phases. The old driver had START_PHASE=1|2|3, set by hand in a config file. This runs six stages — prepare → generate → submit → bundles → validate → report — selected with --start-stage / --stop-after. Every stage writes status.json, so an interrupted run resumes where it stopped and a crashed one still produces a report.

The image check is no longer a manual step. The old README said "test_images.py must be run manually from a compute node." It is now submitted as a SLURM job and its outcome folds into the run's verdict.

Immutable worktrees instead of switching branches. The old script had to be copied out of the repo before use, because it changed branches in the clone. Each repo under test now gets a git worktree, so the clone is never touched and the exact commit tested is recorded.

A report, not just pass/fail. Produces complete-run-report.md / .json plus the HTML review page above, which ranks image differences by severity, so review starts with what actually changed.

Provenance and dependency attribution. Records the E3SM-Unified release and every repo's branch, sha and environment, and diffs the run's environments against the baseline's. That distinguishes image differences caused by the code under test from ones caused by a dependency bump. A baseline that ran from E3SM-Unified exports no environment of its own, so the comparison falls back to the conda list its env_descriptions/<task>.txt carries, and lists only packages the run's own environment has — not the hundreds E3SM-Unified bundles for other tools.

Promotion stays explicit. A complete run never promotes its own results; promote.py is a separate, deliberate step.

The driver itself is tested. 260 unit tests across 13 files (3671 lines). The bash driver had none.

Scheduled runs. A controller script and crontab template for running this unattended.

Also drops the update_*_expected_files_*.sh scripts and the legacy 3.0.0/3.1.0 bundles and comprehensive_v2 cfgs, and updates the testing docs to match.

Status

Draft. Exercised end to end on Chrysalis against the 20260918_unified113 baseline: all stages ran, 186/186 job status files OK, all five integration tests passed, and the image checker compared 35,912 images across five cfgs.

That run also surfaced a resume bug now fixed here — zppy skips tasks whose status file reads WAITING/RUNNING, so a cancelled run's leftovers made a resume silently skip the tasks that never ran. The harness now drops status files whose SLURM job no longer exists.

Reviewing that run's own pages then turned up three viewer bugs, each fixed with a regression test: images were linked one directory too deep and so never loaded; the only links to the per-task pages sat in a column a wide table scrolled out of view; and the environment panel claimed "no dependency changed" when four of five repos had in fact not been compared at all.

🤖 Generated with Claude Code

chengzhuzhang and others added 12 commits September 18, 2026 12:11
Replace tests/main_branch_testing/run_integration_test.bash with a tested
Python package, tests/complete_run/, driven by
`python -m tests.complete_run.automation`.

- Each repository is used through a detached worktree of one resolved
  commit; a developer's clone is never committed to or switched.
- Every stage writes status.json, so a run can be resumed with
  --start-stage, and always ends with JSON and Markdown reports.
- Runs live in a shared, group-writable root; a baseline is a run that
  `python -m tests.complete_run.promote` points baselines/latest-main at.
- Intermediate data (worktrees, zppy's post-processing output) goes to
  per-user scratch.
- Everything that exercises zppy runs in the environment built from the
  commit under test.
- The weekly cfgs, their cases and image-checked tasks are defined once in
  tests/integration/weekly_cfgs.py. Four legacy cfgs that had become copies
  of the current ones are removed, and livvkit and legacy 3.1.0
  pcmdi_diags plots are now image-checked.
- Each run gets an image review summary page across every package.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The image checker's batch script ran `source ~/.bashrc` and then a
per-machine alias (`lcrc_conda`, `compy_conda`, `nersc_conda`). Those aliases
exist only in some people's shell setup, so the job failed immediately for
anyone else ("lcrc_conda: command not found").

Source the conda profile the run is given with --conda-profile, as the
generated task scripts already do, and drop the alias from MachineProfile.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The settings and bash baselines were copied from the output the integration
tests leave in the worktree, but each test deletes that output when it
passes, so a passing run captured none of them ("missing 8 settings
baselines").

Regenerate each one from its dry-run cfg with the zppy under test, pruned
exactly as its test prunes it. On Chrysalis all eight match the current
expected files under the tests' own diff rules.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A run's cfgs source load_latest_e3sm_unified_<machine>.sh, and the manifest
listed only checked-out repositories, so nothing machine-readable said which
Unified release a baseline was built with, or that any package came from it.

Read the version, environment path and key package versions from the package
list the run already collects, and record them in status.json and
manifest.json. The manifest now lists repositories run from Unified too.
`promote show` and the report show the release.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Add --solver (default mamba, falling back to conda when mamba is not
installed). It is used only to create environments; activating, running,
listing and exporting still use conda, whose output the run parses. On
Chrysalis mamba solves zppy's dev.yml in 75 s, where conda 23.3's classic
solver had not finished after 15 minutes.

MPAS-Analysis's dev-spec.txt cannot name channels, so it was solved against
whatever the operator's condarc lists; with only `defaults` it never
finishes. Pass conda-forge with strict priority explicitly, as
MPAS-Analysis's README directs.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
zppy skips a task whose status file begins with OK, WAITING or RUNNING. A
cancelled or crashed run leaves WAITING and RUNNING files behind pointing at
jobs that no longer exist, so resuming into the same output directory skipped
exactly the tasks that never ran. Validation then swept a tree whose every
status file read OK and the run could report a pass over work that never
happened.

Before submitting, drop the status files whose job SLURM no longer has queued.
OK and ERROR are left alone: OK means the work is done, and zppy already
resubmits ERROR. A job still in the queue keeps its file, since deleting a live
one would submit the same task twice.

Without the queue there is no way to tell a stale file from a genuinely pending
one, so a queue that cannot be read clears nothing. That also keeps the unit
tests working where SLURM is absent, since run_command raises on a missing
binary even when check is False.

Found while resuming a run that died when the filesystem filled: 11 of its
global_time_series tasks would have been silently skipped.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Running tests/images in CI exposed that test_compare cannot pass anywhere
mache is unable to discover a machine: it built the diff directory from the
machine's web_portal base_path and $USER, so a GitHub runner failed with
"Unable to discover machine from host name".

Neither value is under test. The diff images are an artifact of the
comparison, so write them to a temporary directory instead. That also stops a
unit test leaving diffs behind in the shared web portal.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The image checker names each image relative to the cfg's diff directory, so
names start with the task ("e3sm_diags/..."), but each review page is written
inside that task's directory. Linking the bare name doubled the task
directory, and no image on any review page loaded. write_viewer now links
images relative to the parent directory.

The summary's only links were a trailing "Review" column in a wide table that
scrolls horizontally, so they started out of view. Package names now jump to
their rows among the checks, and the link to each review page is on the task
name.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A baseline whose repositories ran from E3SM-Unified exports an environment
only for zppy, so every other repository was skipped as "not compared" and
the heading and note, based on zppy alone, said no dependency changed and
that image differences were attributable to the code under test. Comparing a
dev-solve run against a Unified baseline, that is the opposite of the truth.

When the baseline has no export for a repository, compare against the
`conda list` its env_descriptions/<task>.txt carries, by version only, with
package names matched across pip and conda (a dev environment pip-installs
the package under test that Unified has from conda-forge). And never call a
run clean while a repository went uncompared, in the note, the viewer
heading, or the report.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
E3SM-Unified bundles every tool, while each repository's dev environment
holds only what that repository needs, so about 70% of a repository's
differences were Unified packages its environment never had, listed as
removed. They buried the dependencies that actually differ. Against a
Unified baseline, list only packages present in this run's environment,
and state how many Unified-only packages were left out.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The CSS escape for the bullet sat in a plain Python string, where "\202" is
an octal escape, so every notable package in the environment table was
followed by an invisible control character and a literal "2".

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A run's expected image lists are built by walking its own www tree, so a run
made with a reduced --cfg or --task selection becomes a baseline holding only
those tasks, and every other task then records no comparison against it. The
advice narrowed the baseline instead of updating part of it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@chengzhuzhang
chengzhuzhang marked this pull request as ready for review September 22, 2026 23:15
@chengzhuzhang

chengzhuzhang commented Sep 22, 2026 •

Copy link
Copy Markdown
Collaborator Author

@forsyth2 — this is ready for a look. It replaces tests/main_branch_testing/run_integration_test.bash with tests/complete_run/, a Python package driving the whole complete run test. It is refactored based on the original tests with some additional features. I know you have a separate automation effort going, we can compare notes to merge features. This is not user-facing, so please take your time to review and try out.

@forsyth2

Copy link
Copy Markdown
Collaborator

Thanks @chengzhuzhang. I won't have time to actually learn this new approach and try it out until after I get the PCMDI PRs (#815, E3SM-Project/zppy-interfaces#49) merged. In the meantime, I've asked Claude to compare the two approaches.

I did this by giving it:

  1. The doc page explaining the totally manual process I used to do, docs/source/dev_guide/tests/manual_test.rst.
  2. The doc page explaining the current mostly-automated process, docs/source/dev_guide/tests/automated_test.rst, along with the most relevant files: tests/main_branch_testing/run_integration_test.bash & tests/main_branch_testing/zppy_test.cfg
  3. The diff of my PR (currently +1,743, -409 lines), Improve test automation #871, as "Approach A: Extension"
  4. The diff of this PR (currently +10,096, -4,261 lines), Replace the bash complete run driver with tests/complete_run #876, as "Approach B: Redesign"

I'm pasting its conclusions below.


Context

Both PRs are evolutions of the same weekly zppy integration-test workflow — which started as a fully manual, documented-in-RST checklist, and had already been partially automated into a single run_integration_test.bash script + cfg file that handles environment setup, job submission, and status checks, while leaving report-writing and the final image-check step as manual work. The two PRs pick very different strategies for closing that remaining gap.

Approach A — Extension (bolt automation onto the existing bash script)

What it does: keeps run_integration_test.bash as the single orchestrator and adds capability to it:

  • Auto-launches the image checker (test_images.py) as its own SLURM batch job and waits on it, rather than requiring a manual compute-node step.
  • Writes an env_description.txt per task (commit hash + conda list) and a test_report_<TAG>.md that stitches together the unit-test results, job status, and image-check summary tables.
  • Improves the "what changed since expected results were baselined" logic — now per-task, using the production date recorded in env_description.txt rather than a promotion-date ls -lt, with per-repo branch overrides.
  • Documents how to run the whole thing as a weekly cron job now that AUTO_MODE=true removes all interactive checkpoints.
  • Adds one real Python module + tests (image_summary_report.py) for the report's failing-tests section — the one piece pulled out of bash into testable Python.

Strengths

  • Small, reviewable diff relative to a known-good script; low risk of regressing behavior nobody's touched in years.
  • Immediate, concrete payoff: the two most annoying manual steps (launching the image checker, writing the report) disappear, and weekly runs become genuinely unattended/cron-able.
  • No relearning cost — anyone who already knows the cfg file and script keeps using the same mental model.

Weaknesses

  • The core logic (SLURM state tracking, date detection, report assembly) stays in bash, threaded through global variables (IMAGE_CHECKER_JOB_STATE, SLURM_JOBS_INCOMPLETE, etc.). That's harder to unit test and reason about than equivalent Python, and it shows: almost none of the new bash logic gets direct test coverage, unlike the one piece extracted to Python.
  • Still relies on switching branches inside a shared, long-lived checkout per repo (the docs explicitly warn to keep a dedicated clone per _DIR so the script doesn't clobber other work) — no worktree isolation, so runs still aren't safely parallelizable.
  • No environment-reproducibility story: env_description.txt records what ran, but there's no mechanism to rebuild that exact environment later or diff it against a baseline, so "did the plots change because of code or because a dependency drifted" is still an eyeball judgment.
  • Promotion of expected results is untouched — still whatever manual copy process existed before, with no atomicity guarantee or guardrail against promoting a failing/wrong-branch run.
  • Doesn't address gaps in test coverage itself (e.g., a task a cfg runs but nobody wired into the image checker) — there's no consistency check between the "what runs" and "what's checked" tables.
  • Reviewing results still means reading a Markdown table and, if that's not enough, opening raw SLURM .o/.e files — no purpose-built way to browse the actual image diffs.

Approach B — Redesign (replace the bash driver with a tests/complete_run Python package)

What it does: deletes the bash script and rebuilds the orchestration as a proper package — worktrees.py, environments.py, params.py, layout.py, promote.py, provenance.py, envdiff.py, validate.py, report.py, viewer.py, slurm.py, commands.py, automation.py — each with its own dedicated test file. Notable design decisions:

  • Git worktrees instead of branch-switching in a shared clone, so runs no longer need a dedicated checkout per repo and can run concurrently.
  • A real CLI (automation.main([...])) that tests drive through the actual argument parser, rather than hand-built Namespace objects — closes the "flag the parser never defined went unnoticed" failure mode.
  • Three environment modes — dev, unified, baseline — where baseline rebuilds the exact environment a prior baseline was produced with (via an exported conda env export, not just conda list), enabling real reproducibility.
  • envdiff.py explicitly diffs a run's environment against the baseline's and surfaces that in every report/viewer, so environment drift versus code change is no longer a matter of judgment.
  • Atomic, explicit promotion: baselines are symlinks (baselines/latest-main -> ../runs/<tag>) flipped by a separate promote command that refuses failing or feature-branch runs unless overridden, plus a prune command — on a shared, group-writable root rather than a personal directory.
  • Single source of truth for test coverage: weekly_cfgs.py defines which cfgs run, on what case, with which tasks image-checked, and test_weekly_cfgs.py fails if that table disagrees with the actual templates — directly targeting a real past incident where a task ran but was never image-checked.
  • A generated HTML viewer for browsing image diffs (worst-first, filterable) instead of a raw web directory of thousands of PNGs.
  • Optional concurrent environment builds (--parallel-envs) and its own crontab template for scheduled runs.

Strengths

  • The test-automation code is itself heavily tested — there's a whole suite (test_complete_run_automation.py, ..._envdiff.py, ..._environments.py, ..._layout.py, ..._promote.py, ..._provenance.py, ..._report.py, worktrees tests, etc.), so future changes to the harness are guarded, unlike A where the bash logic is essentially untested.
  • Solves problems A doesn't even attempt: parallel-safe runs (worktrees), environment reproducibility and drift attribution (baseline mode + envdiff), atomic/guarded promotion, and a structural check against silent test-coverage gaps.
  • A much better reviewer experience for the actual output (generated HTML viewer vs. grepping status files).
  • Still supports scheduled/cron execution and adds parallel env builds as a performance win A doesn't have.

Weaknesses

  • Much larger surface area — a full package rewrite carries real risk of losing subtle behavior baked into years of bash-script tweaks, and is inherently harder to review as "one PR" than an incremental extension.
  • Bigger migration/relearning cost: docs, muscle memory, and any external tooling built around run_integration_test.bash all need updating.
  • More total code and moving parts to maintain long term (offset, but not eliminated, by the stronger test coverage).
  • Because it's a rewrite rather than an addition, correctness depends on the whole new system being right, not just on a small delta to something already proven in production.

Bottom line

Approach A is the lower-risk, faster-to-land move: it keeps the proven bash orchestrator and removes the two remaining manual steps (launching the image checker, writing the report), which is most of what was actually painful week-to-week. Approach B is the more ambitious structural fix: it treats the test harness as real software — testable, parallel-safe, reproducible, and self-checking for coverage gaps — at the cost of a much bigger, riskier rewrite. Which is "better" depends on whether the team's actual pain points are manual steps (A solves that cheaply) or trust in the results/harness itself — environment drift, silent coverage gaps, promotion mistakes, concurrent runs (B is aimed squarely at those, but asks for a much bigger bet).

@chengzhuzhang

Copy link
Copy Markdown
Collaborator Author

@forsyth2 Thanks for sharing the code analysis. Yeah, no hurry on this, but it should be really straightforward to try out with the instructions in the PR, especially with an agent.

@forsyth2

Copy link
Copy Markdown
Collaborator

@chengzhuzhang I had a moment to ask Claude a couple more questions. I pasted its comments on Approach A (extension) vs Approach B (redesign) below. A couple thoughts from me:

  • Re: promote.py -- note that my equivalent work for this is not Improve test automation #871 but rather Enable partial updates of expected results for testing #852.
  • I realize this is perhaps just a demo at this point, but when we actually want to review the code, we should not try to merge a 10,000 line pull request. We should split it up into chunks. Claude outlines two methods below; the stage-by-stage delegation makes the most sense to me.

Functionality found in A but not B

What A has that B doesn't: A auto-generates the "Step 2: what changed since the baseline" table — it detects, per task, the date the expected results were actually produced (reading Generated: out of env_description.txt, taking the earliest across all cfgs), then fetches each dependency's commit log and renders a table of every commit/PR merged since that date, with links, split so global_time_series and pcmdi_diags get independent rows (since zppy-interfaces bundles two tasks that get refreshed on different schedules), and a "Branch tested" column when the branch under test differs from the branch the baseline was produced from.

B's docs for the equivalent step say, verbatim: "Review each commit log and note commits made since that date... This step is human judgement; the report records what was tested, but deciding which changes matter is yours." B gives you the ingredients — the baseline's commit SHA per repo (promote ... show), and each task's production date (the Generated: line in that baseline's env_descriptions/*.txt) — but nothing fetches the commit log or renders the table. It's a data gap in presentation, not in data availability: B recorded everything A's automation reads, it just never automates the last mile of turning that into a rendered "changes since baseline" table.


Splitting up this PR

A +10k/-4k diff isn't reviewable the way a normal PR is. Nobody can hold that
much new logic in their head at once, so review degrades into skimming file
names and trusting the tests — which also means if something's subtly wrong
(a race condition in the SLURM polling, a path that's right on Chrysalis but
wrong on Perlmutter), it likely isn't caught until the first real weekly run
after merge, at which point there's no small commit to bisect back to — just
one giant one. Splitting it isn't just about tidiness: it's what makes it
possible to actually verify each piece works before trusting the next one on
top of it. Fortunately, the new tests/complete_run/* modules import from
each other in one direction (no cycles), and the pipeline itself has clean
stage boundaries, so it splits cleanly along either of two lines:

Split method PRs involved Runnable at... Replaces the old bash script at...
Bottom-up (build each layer, wire at the end) 5 stacked PRs, following the import graph PR 5, not before PR 5 (splittable into 5a/5b — see below)
Stage-by-stage delegation (strangler fig) 4 PRs, one per pipeline stage — the bash script calls into each as it lands Every PR — it's the live weekly test throughout Gradually, one responsibility at a time; final PR deletes the shim

Bottom-up: complete layers, wire them together at the end

Each PR is a fully-built, fully-tested module (or small group of them), in
dependency order:

  1. commands, worktrees, slurm, weekly_cfgs — the primitives: run a
    subprocess, check out a detached worktree, poll SLURM, and the table
    describing what should run. Complete and tested, but nothing calls them yet.
  2. environments, params, layout — builds conda environments and
    defines where everything lives on disk. Also complete, also uncalled.
  3. envdiff, provenance, viewer — environment-diffing, per-task
    provenance records, the HTML diff viewer. Same story.
  4. validate, report — decides whether a run passed and writes the
    Markdown/JSON report. Still nothing invokes this end to end.
  5. automation.py — the CLI orchestrator that actually calls all four
    layers above in sequence. This is the only PR that produces something you
    can run, because it's the only piece that wires the rest together.

PRs 1–4 are individually small, low-risk, and fully covered by unit tests
(each has a matching test_complete_run_*.py) — but each one just adds files
with no runtime caller until PR 5 lands. You're reviewing "correct in
isolation," not "correct in practice," until the very end.

PR 5 is also the one that has to delete run_integration_test.bash, so it's
worth splitting further on its own:

  • 5a: land automation.py, but leave the old script in place. Run both for
    a cycle or two and confirm they produce the same verdict on the same commits.
  • 5b: once confirmed, a small closing PR that deletes the bash script and
    its README, and swaps the docs over. Nothing left to debug at that point —
    it's pure deletion.

Stage-by-stage delegation: the bash script hands off one piece at a time

Instead of building the new package and switching over all at once, the old
run_integration_test.bash stays the orchestrator throughout, and each PR
replaces one of its responsibilities with a call into the new Python — a
"strangler fig" migration. This maps onto four real seams in the current
pipeline:

  1. Env activation — the bash script currently builds conda envs and
    activates them per task itself. PR 1 adds commands.py, worktrees.py,
    environments.py, provenance.py (writes env_description.txt),
    params.py, layout.py, and changes the bash script to shell out to them
    for this one step instead of doing it inline.
  2. Job generation + submission — PR 2 adds the cfg-generation logic and
    slurm.py, and the bash script now delegates "generate cfgs and submit the
    zppy jobs" to the new code, while still doing its own polling/status checks
    for anything not yet handed off.
  3. Integration tests + image checker — PR 3 adds validate.py (status
    sweep, integration tests, submits the image checker as its own SLURM job),
    envdiff.py, viewer.py. The bash script now calls this instead of
    launching test_images.py manually.
  4. Report generation — PR 4 adds report.py (and optionally promote.py).
    The bash script's job is now just "call each of the above in order and stop."

By the end of PR 4, the bash script isn't doing any of the actual work anymore
— it's a shim: a thin layer whose only job is to call the new Python for
each step, in order, and pass along whatever it returns. Nothing in it is
"real" logic at that point, just wiring — which is exactly why it's safe to
delete in a final, small PR that calls automation.py directly instead.

Every PR here changes the real weekly test immediately — there's no "built
but uncalled" period like in the bottom-up split. The trade-offs:

  • Each boundary needs a small translation layer. The bash script's world
    (MACHINE, CFGS_TO_RUN, *_ENV_TYPE, per-user output dirs) doesn't match
    Approach B's world (CLI flags, manifest.json, a shared immutable run
    directory), so at each stage the bash script needs a thin adapter to consume
    the new Python's output and keep driving the remaining stages. That adapter
    is itself throwaway — useful for a few PRs, deleted once bash is fully
    retired.
  • Bugs land in production, one stage at a time, not in a side-by-side
    comparison.
    The dual-run trick from the bottom-up split still applies per
    stage, though: e.g., PR 1 can run both the old and new env-activation paths
    and diff their outputs, deleting the old path only once they agree —
    cheaper insurance than one big dual-run at the end, spread across four
    smaller checkpoints instead.

Bottom line: bottom-up gives you smaller, easier-to-isolate PRs, but the
real system doesn't change (or benefit) until the last one lands. Stage-by-stage
means the weekly test keeps working — and keeps improving — after every single
PR, at the cost of writing (and later deleting) glue code at each handoff.


Independent of either path:

  • promote.py isn't imported by automation.py at all, so it can land
    whenever — it doesn't affect either column above.
  • weekly_cfgs.py + its consistency test are worth landing first no matter
    what — they work against the current bash-driven system as-is.
  • ~2,000 lines of this diff are unrelated cleanup (legacy cfg templates
    that had drifted into being copies of the current ones, minus paths).
    Pulling that into its own PR shrinks the "real" redesign diff regardless of
    split method.
  • Feature-parity gap: B doesn't currently automate the "what changed since
    the baseline" commit/PR table that Approach A added. It has the same
    underlying data, just not the last-mile automation. If wanted, that's an
    additional PR either way, not something either split produces for free.

@chengzhuzhang

chengzhuzhang commented Sep 24, 2026 •

Copy link
Copy Markdown
Collaborator Author

@forsyth2 Thanks for the feedback. This refactor (also borrows feature from e3sm_diags complete run) was done under time pressure, knowing your time share on E3SM will drop, and what I want out of it is a more automated test framework that can be handed off more easily.

Good catch on #852 -- it's actually a conflict, not just an overlap.

I don't think it's necessary to break the refactor itself into smaller PRs. The first commit is the bulk of it, but the eleven after it are already small and self-contained, and most of the diff is unit tests and deleted legacy cfgs. I agree it should get more testing though. I'll run it on Perlmutter (and setting up team paths for results) as well as Chrysalis.

@forsyth2

Copy link
Copy Markdown
Collaborator

I don't think it's necessary to break the refactor itself into smaller PRs.

This PR is 10,000 lines long.

most of the diff is unit tests and deleted legacy cfgs

Having Claude break down the +10,096 added lines by category yields:

Category Files Lines added % of total
New library modules (tests/complete_run/*.py — the orchestration logic) 13 .py + 3 shell/config templates ~5,186 ~51%
Unit tests for those modules (test_complete_run_*.py, test_weekly_cfgs.py) 14 ~3,745 ~37%
New config table (tests/integration/weekly_cfgs.py) 1 ~92 ~1%
Changes to existing test code (test_images.py, image_checker.py, utils.py, test_image_checker.py) 4 ~388 ~4%
Docs (.rst files, AGENTS.md) 5 ~640 ~6%
Generated/template cfg files, CI/misc config 32 ~52 <1%

Tests are a genuinely large share (37%), and the legacy cfg cleanup is real
(2,351 removed lines, on the deletion side) — but the single largest category
by added lines is new functionality at ~51%, which is bigger than the tests
bucket on its own.

@chengzhuzhang

chengzhuzhang commented Sep 24, 2026 •

Copy link
Copy Markdown
Collaborator Author

Based on the pros and cons discussed here: #876 (comment) I think the new architecture is the better long-term direction. The main concern is how we migrate to it safely, rather than whether we should move in that direction. I think this can be addressed through more testing in practice, for example, by testing with the upcoming E3SM-Unified environment.

@forsyth2

Copy link
Copy Markdown
Collaborator

I think the new architecture is the better long-term direction. The main concern is how we migrate to it safely, rather than whether we should move in that direction.

Yes, exactly. The bash script is the result of many incremental changes but a Python-based modular refactor would make things much more maintainable.

more testing in practice

Now that Shixuan and I have merged #815 (which unfortunately seems to have introduced merge conflicts here) and its companion PR E3SM-Project/zppy-interfaces#49, I am going to start running tests using both #871 and this PR. It's a bit of duplicated testing, but it's really important to see how they match up before switching over completely. I will try out a complete run using the working NCO (5.3.9) as well as Charlie's fix for #875. I'm using Claude to help setup this test framework to match however I setup #871.

A few things Claude has identified as potential differences so far:

  • DependencyNeverSatisfied job handling -- e.g., cancelling if a job will literally never be able to run (and only looking at jobs related to this run)
  • Explicit documentation of commits made since last test (useful for knowing what changed to have caused errors)
  • Looks like some parameters might be mainly command-line flags; in general I find config files more user-friendly (and documentable).

I want to actually try it out of course. Also a couple things that don't appear to be addressed here or in #871:

  • I see you've removed legacy cfgs that are for all intents and purposes duplicates, but we still need to replace the 3.0.0 and 3.1.0 cfgs with 3.2.0 cfgs (that's for New post-v3.2.0 test cfgs #837)
  • climo/ts/e3sm_to_cmip/tc_analysis aren't independently selectable. (Historically I've been focused on the image checker, but these non-plot producing tasks can also be the sources of errors. For example, [Bug]: e3sm_to_cmip failing due to srun error #875 is affecting e3sm_to_cmip, so we don't really need to run anything beyond that).

One of the difficulties of testing tests is that there are many ways for code to fail a test. To be fully automated, the test script needs to handle all those things gracefully (e.g., for "don't wait around for jobs that can't ever run" -- you'd never encounter this when testing code that works) and continue as far as it can. So ironically you need failing code (ideally in multiple obscure ways) to improve the tests.

Another factor is configurability -- e.g., it needs to work for a cron-based weekly testing of main, for a test of e3sm_to_cmip alone using Charlie's NCO path, for pcmdi_diags using a feature branch. I've often encountered errors in test code that arise only when we're testing just a portion of everything.

@forsyth2

forsyth2 commented Sep 25, 2026 •

Copy link
Copy Markdown
Collaborator

Test approach comparison 1: NCO error in e3sm_to_cmip

✅ Both approaches yield the /home/ac.zender/bin_chrysalis/ncremap: line 3830: ppp_pid[${fl_idx}]: unbound variable noted here.

Suggestions for PR 876:

  • For the baseline checking (python -m tests.complete_run.promote --machine chrysalis show), the sha field is useful, but perhaps a sha_date field would be good as well, so it's easy to immediately identify how old it is.
  • We should come up with more generic vocabulary. main_branch_testing, complete_run aren't always reflective of what we're actually testing. The test suite is for testing various configurations.
  • The test isn't recognizing the DependencyNeverSatisfied jobs. It needs to review its list of job IDs and cancel any that have DependencyNeverSatisfied. In this case, the one remaining job that has only Dependency is because it's actually waiting on DependencyNeverSatisfied jobs. That is, it's a second-order dependency never satisified situation. So the test also has to keep cancelling jobs until none of the jobs it requested are in the queue. Then, it should continue on to print info about where to find output.
  • PR 871 also doesn't address this, but here we're really interested in NCO's use in e3sm_to_cmip, so there's not much of a need to run any of the plotting tasks. As mentioned above, "climo/ts/e3sm_to_cmip/tc_analysis aren't independently selectable"

@chengzhuzhang Do you have a preference on where I post these comparison reports? I plan on running several. I can post them on this PR (as I've done here), on the testing discussion (but that's really just for complete testing of main), or open a new discussion page (would allow for threaded conversations on each report)

Using PR 871 (incremental improvements)

Set up the zppy branch

cd ~/ez/zppy
git status
# nothing to commit, working tree clean

# We merged the PCMDI updates: https://github.com/E3SM-Project/zppy/pull/815/commits
# So we only need the test automation framework
git checkout issue-869-improve-test-automation
git log --oneline | head -n 31
# c1d069cf Add pcmdi_diags to legacy 3.1.0 image checks
# ...
# 0b1dc8b8 Automate image checker, add per-task env descriptions, and auto-generate test report
# 64bffc9f Merge pull request #815 from E3SM-Project/zppy_pcmdi_enhancement

# Good, we have the 30 commits from https://github.com/E3SM-Project/zppy/pull/871/commits

Set up the test script

cd ~/ez/zppy
git status
# On branch issue-869-improve-test-automation
# nothing to commit, working tree clean

# NOTE: For `ZPPY_BASE_BRANCH="issue-869-improve-test-automation"` to work,
# that branch needs to be on GitHub so the script can fetch that branch.

# Now, copy the test script and cfg from the zppy repo into the directory
# that you'll be running the test script from.
mkdir -p ~/ez/zppy_main_branch_tests/test_20260925_run2
cd ~/ez/zppy_main_branch_tests/test_20260925_run2
cp ~/ez/zppy/tests/main_branch_testing/run_integration_test.bash .
cp ~/ez/zppy/tests/main_branch_testing/zppy_test.cfg .

# Now, edit the test cfg as needed
emacs zppy_test.cfg

Set up the test cfg

ls -lt /lcrc/group/e3sm/public_html/zppy_test_resources/expected_comprehensive_v3/
# Sep 14 21:11 pcmdi_diags
# Sep  4 11:55 mpas_analysis
# Sep  4 11:53 e3sm_diags
# Aug 14 17:01 global_time_series
# May 20 13:52 livvkit
# May 20 13:51 ilamb

shows the date results were promoted to be official expected results. We can look at the testing log to see what the run before that date was.

  • Before 9/14: 9/4 test
  • Before 9/4: 8/28 test
  • Before 8/14: 8/12 test
  • Before 5/20: These were the results from E3SM Unified 1.13.0
RUN_NUMBER=2

ZPPY_BASE_BRANCH="issue-869-improve-test-automation"

# Because we're using a different base branch for zppy, 
# we need to make sure we specify the correct expected_results_branch.
ZPPY_EXPECTED_RESULTS_BRANCH="main" # Keep as-is

# We don't need to spend time building dev environments for most of these:
DIAGS_ENV_TYPE="unified"
E3SM_TO_CMIP_ENV_TYPE="dev" # Keep as-is, it's what we're testing!
MPAS_ENV_TYPE="unified"
ZI_ENV_TYPE="dev" # Keep as-is, `zppy-interfaces` should almost always be dev

NCO_PATH="/home/ac.zender/bin_chrysalis" # Use NEW NCO

# We only care about running something that uses e3sm_to_cmip
CFGS_TO_RUN="weekly_comprehensive_v3"
TASKS_TO_RUN="pcmdi_diags"

EXPECTED_RESULTS_DIR="/lcrc/group/e3sm/public_html/zppy_test_resources"

DIAGS_EXPECTED_RESULTS_DATE="2026-08-28"
MPAS_EXPECTED_RESULTS_DATE="2026-08-28"
ZI_GLOBAL_TIME_SERIES_EXPECTED_RESULTS_DATE="2026-08-12"
ZI_PCMDI_DIAGS_EXPECTED_RESULTS_DATE="2026-09-04"
E3SM_TO_CMIP_LAST_TESTED_DATE="2026-09-15"
ZPPY_LAST_TESTED_DATE="2026-09-15"

EZ_DIR="/lcrc/group/e3sm/ac.forsyth2/zppy_main_branch_test_dirs"

Run the test script

# NOTE: This part is not listed in the docs, as the script should check this.
# To be on the safer side though, it's a good idea to check that these directories
# don't have uncommitted changes.
cd /lcrc/group/e3sm/ac.forsyth2/zppy_main_branch_test_dirs/e3sm_to_cmip && git status
# nothing to commit, working tree clean
cd /lcrc/group/e3sm/ac.forsyth2/zppy_main_branch_test_dirs/zppy-interfaces && git status
# nothing to commit, working tree clean
cd /lcrc/group/e3sm/ac.forsyth2/zppy_main_branch_test_dirs/zppy && git status
# nothing to commit, working tree clean
cd ~/ez/zppy_main_branch_tests/test_20260925_run2

screen # Use `screen`` so that even if the terminal connection is interrupted, the script will keep running.
ulimit -s unlimited # This is necessary for MPAS-Analysis to work inside `screen`
cd ~/ez/zppy_main_branch_tests/test_20260925_run2
cat zppy_test.cfg # Make sure changes are there
time ./run_integration_test.bash --config zppy_test.cfg 2>&1 | tee integration_test_run2.log
# [2026-09-25 12:33:43] Starting zppy integration test automation
# Ctrl-A D to detach from screen
screen -ls # See what screen sessions you have
tail -f integration_test_run2.log
Jobs remaining: 15 (elapsed: 0s / max: 14400s)[2026-09-25 13:20:06] ✗ Jobs with DependencyNeverSatisfied -- cancelling immediately:
           1295268     debug pcmdi_di ac.forsy PD       0:00      1 (DependencyNeverSatisfied) 
           1295269     debug pcmdi_di ac.forsy PD       0:00      1 (DependencyNeverSatisfied) 
           1295270     debug pcmdi_di ac.forsy PD       0:00      1 (DependencyNeverSatisfied) 
           1295271     debug pcmdi_di ac.forsy PD       0:00      1 (DependencyNeverSatisfied) 
Jobs remaining: 1 (elapsed: 600s / max: 14400s)

D. Review the output

# CTRL C # Exit tail
screen -R
# CTRL C # End

# real    47m55.145s
# user    6m4.075s
# sys     1m45.022s

exit # Exit screen
scancel -u ac.forsyth2

cd /lcrc/group/e3sm/ac.forsyth2/zppy_weekly_comprehensive_v3_output/zppy_main_branch_test_20260925_run2/v3.LR.historical_0051/post/scripts
tail -n 1 e3sm_to_cmip_atm_monthly_180x360_aave_1985-1986-0002.o1295263 
# /home/ac.zender/bin_chrysalis/ncremap: line 3830: ppp_pid[${fl_idx}]: unbound variable
Using PR 876 (refactor)
# One time setup
cd /lcrc/group/e3sm/ac.forsyth2
mkdir zppy_pr876_test_dirs
cd zppy_pr876_test_dirs
git clone git@github.com:E3SM-Project/zppy.git
git clone git@github.com:E3SM-Project/zppy-interfaces.git
git clone git@github.com:E3SM-Project/e3sm_diags.git
git clone git@github.com:E3SM-Project/e3sm_to_cmip.git
git clone git@github.com:MPAS-Dev/MPAS-Analysis.git
cd zppy
git remote add upstream git@github.com:E3SM-Project/zppy.git
cd ../zppy-interfaces
git remote add upstream git@github.com:E3SM-Project/zppy-interfaces.git
cd ../e3sm_diags
git remote add upstream git@github.com:E3SM-Project/e3sm_diags.git
cd ../e3sm_to_cmip
git remote add upstream git@github.com:E3SM-Project/e3sm_to_cmip.git
cd ../MPAS-Analysis
git remote add upstream git@github.com:MPAS-Dev/MPAS-Analysis.git

# Get the refactor (PR 876)
cd /lcrc/group/e3sm/ac.forsyth2/zppy_pr876_test_dirs/zppy
git fetch upstream streamline-complete-run-test
git checkout -b streamline-complete-run-test upstream/streamline-complete-run-test

# Set up a conda env that has mache and pytest
lcrc_conda # Activate conda
rm -rf build
conda clean --all --y
conda env create -f conda/dev.yml -n zppy-test-pr876-20260925
conda activate zppy-test-pr876-20260925
pre-commit run --all-files
python -m pip install .

# Check the baselines
python -m tests.complete_run.promote --machine chrysalis show

gives:

{
  "channel": "main",
  "exists": true,
  "generated": "2026-09-18T17:50:18.240674+00:00",
  "link": "/lcrc/group/e3sm/public_html/zppy_complete_run/baselines/latest-main",
  "machine": "chrysalis",
  "repos": {
    "e3sm_diags": {
      "environment_type": "unified",
      "package": "e3sm_diags",
      "package_version": "3.2.0",
      "unified_version": "1.13.0"
    },
    "e3sm_to_cmip": {
      "environment_type": "unified",
      "package": "e3sm_to_cmip",
      "package_version": "1.14.0",
      "unified_version": "1.13.0"
    },
    "mpas_analysis": {
      "environment_type": "unified",
      "package": "mpas-analysis",
      "package_version": "1.15.0",
      "unified_version": "1.13.0"
    },
    "zppy": {
      "branch": "streamline-complete-run-test",
      "environment": "test-zppy-streamline-complete-run-test-20260918_unified113",
      "environment_type": "dev",
      "remote": "local",
      "sha": "16969263de247311e1662d1c170e797b76196aee",
      "short_sha": "16969263",
      "source_repo": "/gpfs/fs1/home/ac.zhang40/zppy",
      "worktree": "/lcrc/globalscratch/ac.zhang40/zppy_complete_run/20260918_unified113/worktrees/zppy"
    },
    "zppy_interfaces": {
      "environment_type": "unified",
      "package": "zppy-interfaces",
      "package_version": "0.2.1",
      "unified_version": "1.13.0"
    }
  },
  "run": "/lcrc/group/e3sm/public_html/zppy_complete_run/runs/20260918_unified113",
  "tag": "20260918_unified113",
  "unified": {
    "load_script": "/lcrc/soft/climate/e3sm-unified/load_latest_e3sm_unified_chrysalis.sh",
    "packages": {
      "e3sm_diags": "3.2.0",
      "e3sm_to_cmip": "1.14.0",
      "livvkit": "3.3.1",
      "mpas-analysis": "1.15.0",
      "nco": "5.3.9",
      "numpy": "2.4.4",
      "python": "3.13.13",
      "xarray": "2026.2.0",
      "xcdat": "0.11.2",
      "zppy": "3.2.0",
      "zppy-interfaces": "0.2.1"
    },
    "prefix": "/lcrc/soft/climate/e3sm-unified/e3smu_1_13_0/chrysalis/pixi_login/.pixi/envs/default",
    "recorded": "backfilled 2026-09-18 from env_descriptions/e3sm_diags.txt, which the run wrote",
    "resolved_load_script": "/lcrc/soft/climate/e3sm-unified/load_e3sm_unified_1.13.0_chrysalis.sh",
    "version": "1.13.0"
  }
}
screen
ulimit -s unlimited
# We're running this from the very repo we're testing.
# This differs from #871, where we have to 
# copy the script out of the repo so that it 
# can change the repo as needed.
python -m tests.complete_run.automation \
    --machine chrysalis \
    --repo-root /lcrc/group/e3sm/ac.forsyth2/zppy_pr876_test_dirs \
    --branch zppy=streamline-complete-run-test \
    --env-type e3sm_diags=unified \
    --env-type mpas_analysis=unified \
    --nco-path /home/ac.zender/bin_chrysalis \
    --cfg weekly_comprehensive_v3 \
    --task pcmdi_diags \
    --run-number 3 \
    2>&1 | tee complete_run_run3.log

# Ctrl-A D to detach

screen -ls
tail -f complete_run_run3.log
# 2026-09-25 14:02:58,811 INFO    === Stage: prepare ===
# Log: 2026-09-25 14:30:51,240 INFO    Jobs remaining: 4 (elapsed 600s / max 14400s)
> squeue -u ac.forsyth2
             JOBID PARTITION     NAME     USER ST       TIME  NODES NODELIST(REASON) 
           1295284     debug pcmdi_di ac.forsy PD       0:00      1 (DependencyNeverSatisfied) 
           1295285     debug pcmdi_di ac.forsy PD       0:00      1 (DependencyNeverSatisfied) 
           1295286     debug pcmdi_di ac.forsy PD       0:00      1 (DependencyNeverSatisfied) 
           1295287     debug pcmdi_di ac.forsy PD       0:00      1 (Dependency) 
cd /lcrc/globalscratch/ac.forsyth2/zppy_complete_run/20260925_run3/output/zppy_weekly_comprehensive_v3_output/run/v3.LR.historical_0051/post/scripts
grep -v "OK" *status
# e3sm_to_cmip_atm_monthly_180x360_aave_1985-1986-0002.status:ERROR (1)
# e3sm_to_cmip_atm_monthly_180x360_aave_1987-1988-0002.status:ERROR (1)
# e3sm_to_cmip_atm_monthly_180x360_aave_1989-1990-0002.status:ERROR (1)
# e3sm_to_cmip_atm_monthly_180x360_aave_1991-1992-0002.status:ERROR (1)
# e3sm_to_cmip_atm_monthly_180x360_aave_1993-1994-0002.status:ERROR (1)
# pcmdi_diags_mean_climate_model_vs_obs_1985-1994.status:WAITING 1295284
# pcmdi_diags_synthetic_plots_model_vs_obs.status:WAITING 1295287
# pcmdi_diags_variability_modes_atm_model_vs_obs_1985-1994.status:WAITING 1295286
# pcmdi_diags_variability_modes_cpl_model_vs_obs_1985-1994.status:WAITING 1295285
tail -n 1 e3sm_to_cmip_atm_monthly_180x360_aave_1985-1986-0002.o1295279
# /home/ac.zender/bin_chrysalis/ncremap: line 3830: ppp_pid[${fl_idx}]: unbound variable

Checking back in with the log:

# 2026-09-25 14:30:51,240 INFO    Jobs remaining: 4 (elapsed 600s / max 14400s)
# 2026-09-25 14:40:51,259 INFO    Jobs remaining: 4 (elapsed 1200s / max 14400s)
> squeue -u ac.forsyth2
             JOBID PARTITION     NAME     USER ST       TIME  NODES NODELIST(REASON) 
           1295284     debug pcmdi_di ac.forsy PD       0:00      1 (DependencyNeverSatisfied) 
           1295285     debug pcmdi_di ac.forsy PD       0:00      1 (DependencyNeverSatisfied) 
           1295286     debug pcmdi_di ac.forsy PD       0:00      1 (DependencyNeverSatisfied) 
           1295287     debug pcmdi_di ac.forsy PD       0:00      1 (Dependency) 

It looks like the script has not identified that these jobs will never run and is thus hanging.

@chengzhuzhang

Copy link
Copy Markdown
Collaborator Author

Do you have a preference on where I post these comparison reports? I plan on running several. I can post them on this PR (as I've done here), on the testing discussion (but that's really just for complete testing of main), or open a new discussion page (would allow for threaded conversations on each report)

@forsyth2 Since they are specifically helping us evaluate the new approach. I think we can post these comparison here. Thank you for taking the time to identify the gaps of this workflow and to run these comparisons! They’ll be really helpful for validating this PR.

@forsyth2

Copy link
Copy Markdown
Collaborator

Another suggestion for PR 876: enable automatic pinning of dependency versions

One issue I'm running into trying to do a main branch test is that because NCO is currently broken (#875), I need to pin an older NCO for these packages myself. This issue applies to either approach, incremental improvement (#871) or refactor (#876).

For reference, git grep --name-only run_nco inside zppy shows that these 5 tasks require NCO be available in the dev environments: climo, ts, e3sm_to_cmip, tc_analysis, pcmdi_diags. Usually climo, ts, and tc_analysis are run simply using the Unified environment, so new NCO issues most typically first show up in e3sm_to_cmip and/or pcmdi_diags. For yet more context, these packages require NCO in their conda dev-environment setup files: e3sm_to_cmip, MPAS-Analysis, zppy-interfaces (after E3SM-Project/zppy-interfaces#62 merges). e3sm_diags does not.

To address this issue, Claude suggests something like:

python -m tests.complete_run.automation \
    --machine chrysalis --repo-root ... \
    ... \
    --stop-after prepare \
    --run-number 4

# Check status.json's "repos" section for the exact env names it just built, then:
conda install -n test-e3sm_diags-main-20260926_run4 "nco<5.4.0" --yes
conda install -n test-e3sm_to_cmip-main-20260926_run4 "nco<5.4.0" --yes
# ...repeat for each dev-type repo

python -m tests.complete_run.automation \
    --machine chrysalis --repo-root ... \
    ... \
    --start-stage generate --tag 20260926_run4

which I suppose mimics what could be done for the bash script of #871 as well. It's not very elegant though, and it requires waiting for environments to build, and then doing a manual step before instructing the script to continue on.

What would be particularly useful is to be able to specify in the config any dependencies that should be pinned and have the test do this post-creation conda install itself automatically. Something like PIN_DEPENDENCIES: "nco<5.4.0," This also would have been helpful for addressing the issue we saw in August re: PMP's version (#848). To clarify, I do not mean testing with frozen environments which we already decided against; I mean being able to temporarily pin an old version when we've already determined it's the dependency, and not our packages, causing the issue.

(Note: I realize I'm suggesting even more features despite expressing concern that the PR is too large already. I think we should at least identify all the issues and possible improvements. Then comes the question of how best to review and integrate the code into main).

@forsyth2 forsyth2 mentioned this pull request Sep 28, 2026
1 of 17 tasks
@forsyth2

Copy link
Copy Markdown
Collaborator

Test approach comparison 2: main branch testing using NCO 5.3.9

NC0 5.3.9 had to be used because of #875.

Using PR 871 (incremental improvements): see the 9/28 test results on the testing log.

Using PR 876 (refactor)
# Get the refactor (PR 876)
cd /lcrc/group/e3sm/ac.forsyth2/zppy_pr876_test_dirs/zppy
git status
# On branch streamline-complete-run-test
# Has uncommitted changes
git add -A
git commit -m "Testing" --no-verify
 
git fetch upstream streamline-complete-run-test
git checkout -b streamline-complete-run-test-rebased20260929 upstream/streamline-complete-run-test
git fetch upstream main
git rebase upstream/main
# Need to `git rm` the 2 deleted test cfgs

# Set up a conda env that has mache and pytest
lcrc_conda # Activate conda
rm -rf build
conda clean --all --y
conda env create -f conda/dev.yml -n zppy-test-pr876-rebased20260929
conda activate zppy-test-pr876-rebased20260929
pre-commit run --all-files
python -m pip install .

# Check the baselines
python -m tests.complete_run.promote --machine chrysalis show

gives:

{
  "channel": "main",
  "exists": true,
  "generated": "2026-09-18T17:50:18.240674+00:00",
  "link": "/lcrc/group/e3sm/public_html/zppy_complete_run/baselines/latest-main",
  "machine": "chrysalis",
  "repos": {
    "e3sm_diags": {
      "environment_type": "unified",
      "package": "e3sm_diags",
      "package_version": "3.2.0",
      "unified_version": "1.13.0"
    },
    "e3sm_to_cmip": {
      "environment_type": "unified",
      "package": "e3sm_to_cmip",
      "package_version": "1.14.0",
      "unified_version": "1.13.0"
    },
    "mpas_analysis": {
      "environment_type": "unified",
      "package": "mpas-analysis",
      "package_version": "1.15.0",
      "unified_version": "1.13.0"
    },
    "zppy": {
      "branch": "streamline-complete-run-test",
      "environment": "test-zppy-streamline-complete-run-test-20260918_unified113",
      "environment_type": "dev",
      "remote": "local",
      "sha": "16969263de247311e1662d1c170e797b76196aee",
      "short_sha": "16969263",
      "source_repo": "/gpfs/fs1/home/ac.zhang40/zppy",
      "worktree": "/lcrc/globalscratch/ac.zhang40/zppy_complete_run/20260918_unified113/worktrees/zppy"
    },
    "zppy_interfaces": {
      "environment_type": "unified",
      "package": "zppy-interfaces",
      "package_version": "0.2.1",
      "unified_version": "1.13.0"
    }
  },
  "run": "/lcrc/group/e3sm/public_html/zppy_complete_run/runs/20260918_unified113",
  "tag": "20260918_unified113",
  "unified": {
    "load_script": "/lcrc/soft/climate/e3sm-unified/load_latest_e3sm_unified_chrysalis.sh",
    "packages": {
      "e3sm_diags": "3.2.0",
      "e3sm_to_cmip": "1.14.0",
      "livvkit": "3.3.1",
      "mpas-analysis": "1.15.0",
      "nco": "5.3.9",
      "numpy": "2.4.4",
      "python": "3.13.13",
      "xarray": "2026.2.0",
      "xcdat": "0.11.2",
      "zppy": "3.2.0",
      "zppy-interfaces": "0.2.1"
    },
    "prefix": "/lcrc/soft/climate/e3sm-unified/e3smu_1_13_0/chrysalis/pixi_login/.pixi/envs/default",
    "recorded": "backfilled 2026-09-18 from env_descriptions/e3sm_diags.txt, which the run wrote",
    "resolved_load_script": "/lcrc/soft/climate/e3sm-unified/load_e3sm_unified_1.13.0_chrysalis.sh",
    "version": "1.13.0"
  }
}
# The branch needs to be fetch-able!
git push upstream streamline-complete-run-test-rebased20260929

time python -m tests.complete_run.automation \
    --machine chrysalis \
    --repo-root /lcrc/group/e3sm/ac.forsyth2/zppy_pr876_test_dirs \
    --branch zppy=streamline-complete-run-test-rebased20260929\
    --stop-after prepare \
    --run-number 5 \
    2>&1 | tee complete_run_run5.log

# 2026-09-29 13:59:33,685 INFO    Leaving 5 worktrees in place under /lcrc/globalscratch/ac.forsyth2/zppy_complete_run/20260929_run5/worktrees

# real	32m33.844s
# user	7m54.361s
# sys	1m15.474s

# Manually install NCO<5.4.0, since that it is not yet a feature of the refactor

conda install -n test-e3sm_to_cmip-master-20260929_run5 "nco<5.4.0" --yes
conda install -n test-zppy_interfaces-main-20260929_run5 "nco<5.4.0" --yes

screen
cd /lcrc/group/e3sm/ac.forsyth2/zppy_pr876_test_dirs/zppy
ulimit -s unlimited
# We're running this from the very repo we're testing.
# This differs from #871, where we have to 
# copy the script out of the repo so that it 
# can change the repo as needed.
python -m tests.complete_run.automation \
    --machine chrysalis \
    --repo-root /lcrc/group/e3sm/ac.forsyth2/zppy_pr876_test_dirs \
    --branch zppy=streamline-complete-run-test-rebased20260929 \
    --start-stage generate \
    --run-number 5 \
    2>&1 | tee complete_run_run5b.log

# CTRL A D # Exit screen
screen -ls
tail -f complete_run_run5b.log
# 2026-09-29 14:38:13,098 INFO    === Stage: generate ===
# ...
# 2026-09-29 17:16:38,294 INFO    Wrote image review summary: /lcrc/group/e3sm/public_html/zppy_complete_run/runs/20260929_run5/index.html
# 2026-09-29 17:16:38,688 ERROR   The image checker did not pass (state FAILED).

# CTRL C # Exit tail
screen -R 
exit # Exit screen

Analysis of refactored test's output

/lcrc/group/e3sm/public_html/zppy_complete_run/runs/20260929_run5/index.html maps to https://web.lcrc.anl.gov/public/e3sm/zppy_complete_run/runs/20260929_run5/index.html. That page links to the run report, which in turn references /lcrc/group/e3sm/public_html/zppy_complete_run/runs/20260929_run5/test_images_summary_20260929_run5.md (maps to https://web.lcrc.anl.gov/public/e3sm/zppy_complete_run/runs/20260929_run5/test_images_summary_20260929_run5.md). Copying that page's content and condensing it to "only failing image-check tests, sorted by task", we get:

e3sm_diags:

Test name Total images Correct images Identical Cosmetic only Missing images Needs review Severity
bundles_e3sm_diags 1762 1703 3 1700 0 59 22 moderate, 37 minor, 1700 negligible, 3 identical
comprehensive_v2_e3sm_diags 3806 3725 3 3722 1 80 1 missing, 1 major, 49 moderate, 30 minor, 3722 negligible, 3 identical
comprehensive_v3_e3sm_diags 5369 5290 3 5287 0 79 37 moderate, 42 minor, 5287 negligible, 3 identical
legacy_3.1.0_comprehensive_v3_e3sm_diags 5365 5289 3 5286 0 76 37 moderate, 39 minor, 5286 negligible, 3 identical
legacy_3.0.0_comprehensive_v3_e3sm_diags 5365 5289 3 5286 0 76 37 moderate, 39 minor, 5286 negligible, 3 identical

mpas_analysis:

Test name Total images Correct images Identical Cosmetic only Missing images Needs review Severity
comprehensive_v2_mpas_analysis 856 613 4 609 0 243 63 moderate, 180 minor, 609 negligible, 4 identical
comprehensive_v3_mpas_analysis 1280 903 6 897 0 377 71 moderate, 306 minor, 897 negligible, 6 identical
legacy_3.1.0_comprehensive_v3_mpas_analysis 856 580 4 576 0 276 63 moderate, 213 minor, 576 negligible, 4 identical
legacy_3.0.0_comprehensive_v3_mpas_analysis 856 580 4 576 0 276 63 moderate, 213 minor, 576 negligible, 4 identical

global_time_series:

Test name Total images Correct images Identical Cosmetic only Missing images Needs review Severity
bundles_global_time_series 3 3 0 3 0 0 3 negligible
comprehensive_v2_global_time_series 12 12 0 12 0 0 12 negligible
comprehensive_v3_global_time_series 1404 1398 0 1398 0 6 6 minor, 1398 negligible
legacy_3.1.0_comprehensive_v3_global_time_series 1404 1398 0 1398 0 6 6 minor, 1398 negligible
legacy_3.0.0_comprehensive_v3_global_time_series 90 90 0 90 0 0 90 negligible

pcmdi_diags:

Test name Total images Correct images Identical Cosmetic only Missing images Needs review Severity
comprehensive_v3_pcmdi_diags 647 312 1 311 156 179 156 missing, 4 structural, 36 major, 15 moderate, 124 minor, 311 negligible, 1 identical
legacy_3.1.0_comprehensive_v3_pcmdi_diags 617 312 1 311 126 179 126 missing, 4 structural, 36 major, 15 moderate, 124 minor, 311 negligible, 1 identical

These results look completely different from the 9/28 test using the incremental improvements. The number of diffs don't match up for e3sm_diags or pcmdi_diags (asides from "Missing images" in the latter), and the 9/28 test didn't show any non-negligible errors for mpas_analysis or global_time_series. This is likely a sign that the expected results baseline is out of sync between the two approaches.

Indeed, looking at the results from python -m tests.complete_run.promote --machine chrysalis show, we can see that the expected results baseline for the refactored test used Unified for all called packages with only zppy itself using a dev environment. Compare that to the expected results baseline for the 9/28 test, where we found the following:

Test run Date its results were promoted Packages whose baseline got updated
9/4 9/14 pcmdi_diags
8/28 9/4 e3sm_diags, mpas_analysis
8/12 8/14 global_time_series
E3SM Unified 1.13.0 5/20 ilamb, livvkit

Therefore, this test was not actually apples-to-apples.

Nevertheless, let's continue our analysis of the refactored test's output by returning to the run report. Issues of note here:

  • It looks like there might be a unicode issue with some version numbers (alot of —)

Returning one step back to the results home page. Issues of note here:

  • "Compared against baseline 20260918_unified113" implies there can't be varied baselines, which should be addressed per the issues discussed in Enable partial updates of expected results for testing and the testing strategy discussion. We can't be waiting around for every task to pass before updating expected results. The longer ago expected results were generated, the more diffs come up and it becomes harder to know what's a new error.
  • It's unintuitive to me that clicking the package link in the "By package" table just brings you to the very row you just clicked. I believe the point is that you can share the link with others, but it makes the link look broken.
  • The "By package" section gives us no indicator of the environment used for tasks that don't produce a plot (e.g., e3sm_to_cmip), or for zppy itself.

Let's look at some of the "By check" links. For example e3sm_diags weekly_comprehensive_v3. This is pretty nice -- it expand the image checker grid so it's easier to view at a glance. One thing I dislike is that the "Environment" box is open by default -- there are so many dependencies listed it just looks like noise before getting to what we really care about, the image diff grid.

In summary, suggestions for PR 876:

  • Check for Unicode issues in the environment listings
  • Allow for partial updates of the expected results (maybe even allow the user to set which expected results to use as baseline?)
  • Indicate environments used for all tasks, not just the ones generating plots.
  • Have the "Environment" box closed by default.

I've also asked Claude for further analysis (copied below):


Claude's further analysis

I went through the linked pages under 20260929_run5 (report, image summary, and index.html) in detail, cross-checked the numbers, and spot-checked a couple of the live conda environments against what the report claims. Summary below, split into things that need work and things that are clear wins over the PR 871 approach.

Needs work

  1. The Environment diff table can misreport what actually ran. The report says zppy_interfaces ran nco 5.4.0; conda list -n test-zppy_interfaces-main-20260929_run5 nco shows it's actually 5.3.9. e3sm_to_cmip wasn't even included in the comparison (Not compared for: e3sm_to_cmip), but its live env is also correctly 5.3.9. So the "This run" column looks like a snapshot taken early (likely right after conda env create, before the manual nco<5.4.0 pins were applied) that never gets refreshed before the final report is written. This needs a fix at the source — capture the environment manifest at or after task execution, not once during setup — and e3sm_to_cmip should be included in the comparison rather than excluded, since it currently hides exactly this class of bug. (The one nco row that may genuinely be 5.4.0 is zppy itself, which doesn't call NCO, so that particular mismatch has no functional consequence — but it's a coincidence, not evidence the underlying bug is benign elsewhere.)
  2. No support yet for staggered per-package baselines. promote --machine chrysalis show and the report (Compared against baseline 20260918_unified113) both show one global baseline used for every package except zppy's own dev env. PR 871's config exposes DIAGS_EXPECTED_RESULTS_DATE, MPAS_EXPECTED_RESULTS_DATE, etc. for exactly this reason. Until this is ported, none of the diff counts between the two approaches are apples-to-apples.
  3. zppy and e3sm_to_cmip are invisible in the "By package" results table. Only e3sm_diags, ILAMB, LIVVkit, MPAS-Analysis, and zppy-interfaces get a row. There's no result/status indicator at all for the two tasks that don't produce plots.
  4. The "changes since expected results were updated" section from PR 871's report didn't make it into the rewrite. That per-package commit list since the baseline was generated was genuinely useful for triage and should either be ported or explicitly deprioritized.
  5. The Environment <details> box on the per-check pages defaults to open (<details class='env' open>), burying the image diff grid under a long dependency list. Should default to closed.
  6. "Needs review" means two different things in two files that sit side by side. test_images_summary_*.md's per-check "Needs review" column excludes missing images (tracked separately); index.html's "By package" table's "Images needing review" includes them. The underlying numbers are internally consistent, but this will read as a discrepancy to anyone cross-referencing the two files.

Clear improvements over the PR 871 / current method

  • The image checker now runs as an automated SLURM job end-to-end — no more manual compute-node step to kick off pytest tests/integration/test_images.py.
  • Job-status and integration-test tracking (test_last_year.py, test_bash_generation.py, test_campaign.py, test_defaults.py, test_bundles.py) is retained and reported cleanly, at parity with PR 871.
  • The per-check pages (e.g. the e3sm_diags weekly_comprehensive_v3 view) give a much nicer expandable image-diff grid for at-a-glance review than anything in the old report.
  • The "By package"/"By check" tables give shareable, anchor-linkable views into a single run — a real step up in navigability over a flat markdown report.
  • Overall runtime is roughly on par with the current script (~3h in both cases), so the rewrite isn't paying a performance tax for the added structure.

Net: the reporting and automation UX is a real improvement, but before we can use this to judge the refactor against PR 871, we need the environment-snapshot bug fixed (so we can trust any package's reported version) and the staggered-baseline feature ported (so the diff comparison is actually apples-to-apples). Until then, the diff numbers in this run shouldn't be used as evidence either way.

@forsyth2

Copy link
Copy Markdown
Collaborator

A couple more suggestions that come to mind:

  • /lcrc/group/e3sm/public_html/zppy_complete_run/runs has multiple runs, and it'd be very easy for two testers to accidentally choose the same test name. I think the username should be inserted into the path so it's something like runs/$USER/... to prevent this.
  • It'd be great if we can make what expected results we're comparing against completely configurable. That is, for each task, for each cfg, there'd be an archive of expected results (up to a certain point in the past to avoid wasting space), and the tester could choose which set of expected results to use per task. The test report would then list which expected results set was used for each task. The cron job config would also specify which expected results it should work off of. For example, on the 9/28 test we were using four different sets of expected results, as described by the table in the above comment:
Test run Date its results were promoted Packages whose baseline got updated
9/4 9/14 pcmdi_diags
8/28 9/4 e3sm_diags, mpas_analysis
8/12 8/14 global_time_series
E3SM Unified 1.13.0 5/20 ilamb, livvkit

@chengzhuzhang

Copy link
Copy Markdown
Collaborator Author
  • /lcrc/group/e3sm/public_html/zppy_complete_run/runs has multiple runs, and it'd be very easy for two testers to accidentally choose the same test name. I think the username should be inserted into the path so it's something like runs/$USER/... to prevent this.

I’d vote against adding /$USER. It seems unlikely that two testers would run the same test at the same time, and we’d also like to maintain a single shared baseline rather than have baselines scattered across $USER directories.

@forsyth2

Copy link
Copy Markdown
Collaborator

I’d vote against adding /$USER. It seems unlikely that two testers would run the same test at the same time

> ls /lcrc/group/e3sm/public_html/zppy_complete_run/runs/
20260918_main_run1  20260918_unified113  20260925_run3  20260929_run1  20260929_run2  20260929_run3  20260929_run4  20260929_run5

@chengzhuzhang These are pretty similar names. All it would take for a name collision is two developers testing different features on the same day. There's no timestamp or any sort of further layer of differentiation.

we’d also like to maintain a single shared baseline rather than have baselines scattered across $USER directories.

To clarify, the $USER suggestion was for runs, not expected results baselines. Those should definitely not be user specific.

@chengzhuzhang

Copy link
Copy Markdown
Collaborator Author

@chengzhuzhang These are pretty similar names. All it would take for a name collision is two developers testing different features on the same day. There's no timestamp or any sort of further layer of differentiation.

Maybe we could use a timestamp to the test name to provide another layer of differentiation

screen -ls # See what screen sessions you have
tail -f integration_test_runN.log
Packages that most often move results (``numpy``, ``matplotlib``, ``xarray``,
``nco``, ``esmf``, the analysis packages themselves) are listed first and

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

PMP should also be part of "Packages that most often move results"

This branch has not been deployed

No deployments
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.

2 participants