Skip to content

Enable EAMxx CMORization, E3SM Diags and add ts-level vertical regrid#827

Merged
chengzhuzhang merged 16 commits into
mainfrom
enable-e3sm_to_cmip-eamxx
May 28, 2026
Merged

Enable EAMxx CMORization, E3SM Diags and add ts-level vertical regrid#827
chengzhuzhang merged 16 commits into
mainfrom
enable-e3sm_to_cmip-eamxx

Conversation

@chengzhuzhang

@chengzhuzhang chengzhuzhang commented May 14, 2026

Copy link
Copy Markdown
Collaborator

Summary

Objectives:

  • Fix the fragile input_files.split(".")[0] gate in e3sm_to_cmip.bash that
    excluded EAMxx runs whose input_files doesn't start with cam/eam/eamxx
    (e.g. input_files = "AVERAGE.nmonths_x1" with input_component = "eamxx").
    Replaced with a prc_typ-based gate that honors input_component. Fix [Bug]: fragile component detection before vertical interpolation #820
  • Add EAMxx-specific ncremap flags (--ps_nm=${file}/ps, --vrt_in=<L128>)
    inside the e3sm_to_cmip vertical interpolation block, auto-emitted when
    prc_typ == 'eamxx'. vrt_in_file defaults under
    {diagnostics_base_path}/e3sm_to_cmip_data/grids/vert_L128.nc.
  • Add an additive, opt-in ts-level vertical regrid path. New [ts] parameters
    vrt_remap_vars, vrt_remap_file, vrt_in_file: when set, the ts task
    also writes vertically remapped files to a sibling directory
    post/{component}/{grid}/ts_vrt_remap/{frequency}/{ypf}yr/ alongside ts/,
    so users can use native-level and pressure-level data simultaneously.
  • Wire e3sm_to_cmip.bash to auto-detect ts_vrt_remap/ files (via
    readlink on the input symlinks) and skip its inline regrid for those vars,
    avoiding redundant work.
  • Add examples/post.v3.eamxx.cfg demonstrating the EAMxx pipeline end-to-end
    (climo, ts, e3sm_to_cmip, e3sm_diags). Contributed by @zhangshixuan1987
  • Decouple e3sm_diags web publish path from subsection case overrides to allow flexibility for eamxx file names: e.g. 1ma_ne30pg2.AVERAGE.nmonths_x1.1995-01-01-00000.nc. web results stays with the top-level case-named path.

Results can be viewed from here.
Example configuration file is available here.

Issue resolution:

Select one: This pull request is...

  • a bug fix: increment the patch version
  • a small improvement: increment the minor version
  • a new feature: increment the minor version
  • an incompatible (non-backwards compatible) API change: increment the major version

Please fill out either the "Small Change" or "Big Change" section (the latter includes the numbered subsections), and delete the other.

Small Change

  • To merge, I will use "Squash and merge". That is, this change should be a single commit.
  • Logic: I have visually inspected the entire pull request myself.
  • Pre-commit checks: All the pre-commits checks have passed.

Big Change

  • To merge, I will use "Create a merge commit". That is, this change is large enough to require multiple units of work (i.e., it should be multiple commits).

1. Does this do what we want it to do?

Required:

  • Product Management: I have confirmed with the stakeholders that the objectives above are correct and complete.
  • Testing: I have added or modified at least one "min-case" configuration file to test this change. Every objective above is represented in at least one cfg.
  • Testing: I have considered likely and/or severe edge cases and have included them in testing.

If applicable:

  • Testing: this pull request introduces an important feature or bug fix that we must test often. I have updated the weekly-test configuration files, not just a "min-case" one.
  • Testing: this pull request adds at least one new possible parameter to the cfg. I have tested using this parameter with and without any other parameter that may interact with it.

2. Are the implementation details accurate & efficient?

Required:

  • Logic: I have visually inspected the entire pull request myself.
  • Logic: I have left GitHub comments highlighting important pieces of code logic. I have had these code blocks reviewed by at least one other team member.

If applicable:

  • Dependencies: This pull request introduces a new dependency. I have discussed this requirement with at least one other team member. The dependency is noted in zppy/conda, not just an import statement.

3. Is this well documented?

Required:

  • Documentation: by looking at the docs, a new user could easily understand the functionality introduced by this pull request.

4. Is this code clean?

Required:

  • Readability: The code is as simple as possible and well-commented, such that a new team member could understand what's happening.
  • Pre-commit checks: All the pre-commits checks have passed.

If applicable:

  • Software architecture: I have discussed relevant trade-offs in design decisions with at least one other team member. It is unlikely that this pull request will increase tech debt.

chengzhuzhang and others added 8 commits May 14, 2026 15:12
Closes #820: replace fragile input_files.split(".") gate in
e3sm_to_cmip.bash with a prc_typ-based check that honors input_component
override (e.g. EAMxx runs with input_files="AVERAGE.nmonths_x1").

Addresses #817: add EAMxx-specific ncremap flags (--ps_nm=${file}/ps,
--vrt_in=vert_L128.nc) when prc_typ == 'eamxx', auto-defaulted from
diagnostics_base_path.

New additive [ts] vert_remap_vars path produces a sibling ts_vert_remap/
directory alongside ts/, so users can use native-level and
pressure-level data simultaneously. e3sm_to_cmip.bash auto-detects
ts_vert_remap/ files (via readlink) and skips redundant inline regrid.

Files changed:
- zppy/defaults/default.ini: new [ts] vert_remap_vars, vrt_remap_file,
  vrt_in_file; new [e3sm_to_cmip] vrt_in_file
- zppy/templates/ts.bash: post-ncclimo vert remap block
- zppy/ts.py: path defaults for vrt_remap_file / vrt_in_file
- zppy/templates/e3sm_to_cmip.bash: prc_typ gate, EAMxx flags,
  ts_vert_remap consumption
- zppy/e3sm_to_cmip.py: EAMxx vrt_in_file default
- examples/post.v3.eamxx.e3sm_to_cmip.cfg: example based on issue #820

Legacy [e3sm_to_cmip] interp_vars path is preserved unchanged for
backward compatibility.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Renames examples/post.v3.eamxx.e3sm_to_cmip.cfg to post.v3.eamxx.cfg
and merges in climo, additional ts subsections, e3sm_to_cmip
land_monthly, and e3sm_diags tasks. Contributed by Shixuan Zhang.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
A prior run replaces the symlink in place via mv ${file}.plev ${file},
so on re-run the file is a real plev file with no lev/ilev dim and
ncremap fails. Bail out early when the file is no longer a symlink.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The web publish path used the subsection-resolved ${case}, which for
EAMxx cfgs that override case in [climo]/[[atm_monthly_180x360_aave]]
(to match the ncclimo input filename prefix, e.g. "1ma_ne30pg2") would
land under that short prefix instead of the canonical [default] case
name. Snapshot config["default"]["case"] into c["default_case"] and use
it for web_dir so file-prefix overrides no longer reroute publishing.
Climo file-prefix lookups and param.test_name still use the subsection
case, so file discovery is unchanged.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@chengzhuzhang chengzhuzhang changed the title Enable EAMxx CMORization and add ts-level vertical regrid Enable EAMxx CMORization, E3SM Diags and add ts-level vertical regrid May 15, 2026
@chengzhuzhang
chengzhuzhang marked this pull request as ready for review May 15, 2026 21:30
@chengzhuzhang
chengzhuzhang requested a review from Copilot May 15, 2026 21:30

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Enables EAMxx CMORization and e3sm_diags pipelines end-to-end, replaces the fragile input_files-prefix gate before vertical interpolation with a prc_typ-based gate, and adds an opt-in [ts] vertical-regrid path that writes pressure-level files to a sibling ts_vert_remap/ directory which e3sm_to_cmip then auto-detects to avoid redundant regridding.

Changes:

  • Replace fragile input_files.split(".")[0] gate with prc_typ in ('cam','eam','eamxx'); emit EAMxx-specific ncremap flags (--ps_nm, --vrt_in) when prc_typ=='eamxx'; auto-skip vars already vert-remapped by [ts].
  • Add [ts] parameters vert_remap_vars/vrt_remap_file/vrt_in_file with EAMxx defaults under diagnostics_base_path, and [ts] template logic to produce a parallel ts_vert_remap/ output tree.
  • Switch e3sm_diags web publish path to ${www}/{{ default_case }}/... (decoupled from subsection case overrides) and add examples/post.v3.eamxx.cfg.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
zppy/templates/e3sm_to_cmip.bash New prc_typ gate, ts_vert_remap symlink overlay, readlink-based skip, and EAMxx ncremap flags.
zppy/templates/ts.bash New optional vertical regrid loop producing output_plev/ and ts_vert_remap/ sibling destination.
zppy/templates/e3sm_diags.bash web_dir now uses {{ default_case }} instead of subsection ${case}.
zppy/ts.py Defaults for new vrt_remap_file/vrt_in_file when vert_remap_vars is set.
zppy/e3sm_to_cmip.py EAMxx-only default for vrt_in_file.
zppy/e3sm_diags.py Propagates config['default']['case'] as default_case for the bash template.
zppy/defaults/default.ini Declares new [ts] and [e3sm_to_cmip] parameters (and __many__ overrides).
tests/test_sections.py Adds new params to expected section/subsection dicts.
examples/post.v3.eamxx.cfg New end-to-end EAMxx example (climo, ts, e3sm_to_cmip, e3sm_diags).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread examples/post.v3.eamxx.cfg Outdated
Comment thread zppy/templates/e3sm_to_cmip.bash Outdated
Comment thread zppy/templates/e3sm_diags.bash
Comment thread zppy/ts.py Outdated
Comment thread zppy/templates/ts.bash Outdated
Comment thread zppy/defaults/default.ini
chengzhuzhang and others added 2 commits May 15, 2026 14:52
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@chengzhuzhang

chengzhuzhang commented May 15, 2026

Copy link
Copy Markdown
Collaborator Author

@zhangshixuan1987 this PR should enable cmorization and e3sm Diags (except dirunal cycle and TC sets) for eamxx. I needed to make changes to both zppy and your example cfg file to make things work. Could you review: https://github.com/E3SM-Project/zppy/blob/f61b71a1980b8800daa095d0f7764b563083da55/examples/post.v3.eamxx.cfg. and let me know if it looks okay? e3sm_digas results can be find here. There was an issue for start and end year for Wheeler Kiladis, I will have a rerun to create it.

@chengzhuzhang

Copy link
Copy Markdown
Collaborator Author

@zhangshixuan1987 #830 should already be taken care of by this PR, without needing to add additional parameters.

@zhangshixuan1987

Copy link
Copy Markdown
Collaborator

@zhangshixuan1987 #830 should already be taken care of by this PR, without needing to add additional parameters.

@chengzhuzhang Then we can forget about the #830

@chengzhuzhang
chengzhuzhang marked this pull request as draft May 15, 2026 23:22
@chengzhuzhang

Copy link
Copy Markdown
Collaborator Author

I realized that more work needed for eamxx daily output AVERAGE.ndays_x1, converting to draft..

@zhangshixuan1987

Copy link
Copy Markdown
Collaborator

@zhangshixuan1987 this PR should enable cmorization and e3sm Diags (except dirunal cycle and TC sets) for eamxx. I needed to make changes to both zppy and your example cfg file to make things work. Could you review: https://github.com/E3SM-Project/zppy/blob/f61b71a1980b8800daa095d0f7764b563083da55/examples/post.v3.eamxx.cfg. and let me know if it looks okay? e3sm_digas results can be find here. There was an issue for start and end year for Wheeler Kiladis, I will have a rerun to create it.

Hi @chengzhuzhang, I think the cfg file looks consistent with what I have on my local use. Below are some of my comments for your reference:

  1. I think it may be more convenient to combine atm_2d_monthly_180x360_aave and atm_3d_monthly_180x360_aave into a single workflow, but I originally split them to highlight the different extra_vars requirements. For 2D fields, area,landfrac,ocnfrac are mainly included for downstream analysis convenience, whereas 3D fields also require ps,hyai,hyam,hybi,hybm for vertical interpolation. In particular, ps is only needed for 3D vertical interpolation. If the two sections are combined, all extra_vars may be applied to both 2D and 3D variables, meaning unnecessary variables such as ps could be included in every 2D time-series file. This would introduce redundancy and could substantially increase, potentially even double, the file size for 2D variable outputs. My concern here is mainly about the robustness and efficiency of the processing workflow. I think there are two possible solutions: (1) keep the approach I used, which splits the 2D and 3D field processing into separate workflows; or (2) modify the zppy ts workflow to clean up and remove redundant variables after time-series processing. Overall, I think careful consideration is valuable here in terms of saving space.

  2. For the e3sm_to_cmip section, I think there is no problem with omitting cmip_vars. However, the downstream workflow should add a step to check how many variables in the cmip_vars list can actually be processed based on the available model output files. Otherwise, the workflow may generate many error messages that could be misleading, especially for users who are not familiar with this part of the code or with what is happening internally.

All other parts look reasonable to me. Thank you @chengzhuzhang for merging this cfg example.

@chengzhuzhang

chengzhuzhang commented May 18, 2026

Copy link
Copy Markdown
Collaborator Author

@zhangshixuan1987 thank you for your comment.
Regarding your first comment about splitting the 2D and 3D time-series processing into separate subsections:
the reason this was being considered is that extra_vars = "ps,hyai,hyam,hybi,hybm,area,landfrac,ocnfrac"
attaches ps (and the others) to every per-variable file, including the 2D ones. The underlying need for
extra_vars is that for EAMxx files, ncclimo does not automatically attach ps and the other ancillary
coordinate variables to the 3D output during splitting (unlike --prc_typ=eam). If -p eamxx were updated to
mirror the -p eam behavior, we could drop extra_vars entirely and keep a single unified cfg subsection.

Let's check with @czender if it is reasonable to update ncclimo this way.

Regarding to e3sm_to_cmip session, e3sm_to_cmip processes everything in cmip_vars, succeeds on what it can, errors on what it can't, and exits 0 as long as something was produced. So this has high tolerance of missing variables. I will add the line to specifying --cmip_vars, and comment on the e3sm_to_cmip behavior as well.

@chengzhuzhang
chengzhuzhang marked this pull request as ready for review May 19, 2026 23:03
@czender

czender commented May 19, 2026

Copy link
Copy Markdown

@zhangshixuan1987 The latest NCO snapshot, now on Chrysalis and PM, tolerates missing p0/P0 for EAMxx compatibility. If you are able to test it, please do. Otherwise it will be in NCO 5.4.0 in a few weeks.

ac.zender@chrlogin1:~$ ncremap -P eamxx --vrt_out=${DATA}/grids/vrt_prs_ncep_L19.nc ${DATA}/bm/eamxx_ne30pg2l128.nc ~/foo.nc
Input #00: /home/ac.zender/data/bm/eamxx_ne30pg2l128.nc
Vertical : /home/ac.zender/data/grids/vrt_prs_ncep_L19.nc
ncks: INFO nco_ntp_vrt() temperature extrapolated toward/into surface assuming constant moist adiabatic lapse rate = 6.5 K/(100 hPa) for variable T_mid
ac.zender@chrlogin1:~$ 

@forsyth2 forsyth2 left a comment

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.

@chengzhuzhang @zhangshixuan1987 Thanks for working on this! I've done a very-high-level visual inspection, but I'd definitely like to run the integration tests. While the current tests can confirm that nothing breaks, we'd need to include a new test cfg using an EAMxx simulation as the input to check the new features. I see /pscratch/sd/z/zhan391/e3smv4_project/ne256pg2_ne256pg2.F20TR-SCREAMv1.July-1.spanc800.2xauto.acc150.n0032.test2.1 is being used in the example cfg examples/post.v3.eamxx.cfg here -- should I transfer that (or a portion of that) to Chrysalis to test? Or is there another simulation that would be good for testing?

@chengzhuzhang

Copy link
Copy Markdown
Collaborator Author

@forsyth2 thanks for a review. I'm not aware if lcrc has a similar eamxx run. If a transfer is needed, we only need a portion of ne256pg2_ne256pg2.F20TR-SCREAMv1.July-1.spanc800.2xauto.acc150.n0032.test2.1

@forsyth2

Copy link
Copy Markdown
Collaborator

@chengzhuzhang Ok, I can also just run the test cfg on Perlmutter, but the node queue time can be an issue. In any case, I'll work on making a test cfg version of the example cfg here.

@zhangshixuan1987

Copy link
Copy Markdown
Collaborator

@chengzhuzhang @chengzhuzhang : As Jill mentioned, the data has to be transferred to Chrysalis for a full test related to the code base in this pull request. I do not have a copy on Chrysalis for any of the EAMXX model output data.

@chengzhuzhang

Copy link
Copy Markdown
Collaborator Author

@forsyth2 I just realized that the TC set is not yet completed #828
I think we can also wait until that PR is merged, then create the eamxx test.

@forsyth2

Copy link
Copy Markdown
Collaborator

I think we can also wait until that PR is merged, then create the eamxx test.

@chengzhuzhang Ok, we can do that. We at least have confidence it's working since you were able to produce example output. We just don't want to be adding too many EAMxx features without a test cfg that will confirm the code is behaving as expected.

Would you prefer me to just do a deeper visual inspection on this PR and then merge it? Or should we at least run the existing tests before merging?

@chengzhuzhang

Copy link
Copy Markdown
Collaborator Author

@forsyth2 yes, we should make sure this won't break existing tests before merging. Thanks!

@forsyth2 forsyth2 mentioned this pull request May 26, 2026
3 tasks
@forsyth2

Copy link
Copy Markdown
Collaborator

we should make sure this won't break existing tests before merging.

Sounds good, I'll work on getting those tests started.

I think we can also wait until that PR is merged, then create the eamxx test.

We just don't want to be adding too many EAMxx features without a test cfg that will confirm the code is behaving as expected.

I made a new issue, #837, to specifically track the new test cfgs (the EAMxx one and also the legacy 3.2.0 cfgs) that we'll need to create

Comment thread zppy/e3sm_to_cmip.py
# EAM/CAM have vertical coord info embedded in the file already.
if c["prc_typ"] == "eamxx":
default_vrt_in_file = (
f"{c['diagnostics_base_path']}/e3sm_to_cmip_data/grids/vert_L128.nc"

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.

I see the code already had a reference to {c['diagnostics_base_path']}/e3sm_to_cmip_data/ a few lines above, but it does seem a little strange since e3sm_to_cmip is downstream from ts.

Claude says the following, emphasis my own:

diagnostics_base_path points to a shared data installation directory (something like /usr/share/e3sm_unified/...) that happens to be organized under a folder named e3sm_to_cmip_data/ because that's where the e3sm_to_cmip package installs its reference data files (grid descriptors, pressure-level maps, etc.). The ts task is just borrowing files from that same installed data directory — it doesn't depend on the e3sm_to_cmip task having run first.
So the path e3sm_to_cmip_data/grids/vert_L128.nc means "the vert_L128.nc file that ships with the e3sm_to_cmip software package," not "a file produced by the e3sm_to_cmip zppy task." The e3sm_to_cmip task uses the same path for the same reason.
The slight awkwardness is that ts.py now has a hard-coded dependency on the directory layout of the e3sm_to_cmip package's data installation. If that package ever reorganizes its data files, both ts.py and e3sm_to_cmip.py would need updating. It might be worth a comment in the code noting that these are static reference files from the installed package, not outputs of the downstream task.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

It might be worth a comment in the code noting that these are static reference files from the installed package, not outputs of the downstream task.

@forsyth2 Prefixing with diagnostics_base_path should already imply the data are pre-installed. I think the current approach is fine, but feel free to update if you think it improves clarity.

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.

Prefixing with diagnostics_base_path should already imply the data are pre-installed

Yes I suppose looking at it again, that makes sense. It was just a little confusing when I was trying to figure out which tasks had e3sm_to_cmip as a dependency. A comment might be nice, but not necessary.

@czender

czender commented May 27, 2026

Copy link
Copy Markdown

@chengzhuzhang I'm looking into the reason for the slowdown in ncclimo HFC mode. I cannot find the original files that you are attempting to process. Please point me to the ne30pg2 EAMxx input files. Thanks.

@chengzhuzhang

Copy link
Copy Markdown
Collaborator Author

@chengzhuzhang I'm looking into the reason for the slowdown in ncclimo HFC mode. I cannot find the original files that you are attempting to process. Please point me to the ne30pg2 EAMxx input files. Thanks.

@czender I have been testing with files here: /pscratch/sd/z/zhan391/e3smv4_project/ne256pg2_ne256pg2.F20TR-SCREAMv1.July-1.spanc800.2xauto.acc150.n0032.test2.1/run/3ha_ne30pg2.AVERAGE.nhours_x3*nc. Thanks!

@forsyth2 forsyth2 left a comment

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.

✅ Test passed. I also tried to a deeper visual inspection of the PR; I only left some minor nitpick comments that don't necessarily need to be addressed. This should be good to merge (reminder: we'll want to implement #837 soon to actually test an EAMxx simulation).

Testing

Set up environments
lcrc_conda # Bash function to activate conda.

# Set up e3sm_to_cmip env
# https://github.com/E3SM-Project/e3sm_to_cmip/commits/master/
# There have been 2 commits since `e3sm_to_cmip v1.14.0`,
# so, we should use a dev env rather than the Unified env.
cd ~/ez/
git clone git@github.com:E3SM-Project/e3sm_to_cmip.git
cd e3sm_to_cmip
git remote -v
git remote add upstream git@github.com:E3SM-Project/e3sm_to_cmip.git
git status # Check that branch is `master`, and for "nothing to commit, working tree clean"
git fetch upstream master
git reset --hard upstream/master
git log --oneline | head -n 3
# a54c636 Extra eamxx var (#347)
# 87e6479 fix: make `--var-list` parsing tolerant to comma-separated input (#346)
# 84bd5ff Bump to v1.14.0 (#345)
# Good, matches https://github.com/E3SM-Project/e3sm_to_cmip/commits/master/
rm -rf build
conda clean --all --y
conda env create -f conda-env/dev.yml -n test-e3sm-to-cmip-master-20260526 # Use today's date
conda activate test-e3sm-to-cmip-master-20260526
python -m pip install .

# Set up e3sm_diags env
# https://github.com/E3SM-Project/e3sm_diags/commits/main/
# There has been 1 commit since `e3sm_diags v3.2.0`,
# so, we should use a dev env rather than the Unified env.
cd ~/ez/e3sm_diags
git status # Check that branch is `main`, and for "nothing to commit, working tree clean"
git fetch upstream main
git reset --hard upstream/main
git log --oneline | head -n 2 
# 9d5ebf19 Add targeted image-regression and E3SM Unified CI coverage (#1053)
# ff3ef00d Bump v3.2.0 (#1056)
# Good, matches https://github.com/E3SM-Project/e3sm_diags/commits/main
rm -rf build
conda clean --all --y
conda env create -f conda-env/dev.yml -n test-e3sm-diags-main-20260526 # Use today's date
conda activate test-diags-main-20260526
python -m pip install .

# Set up MPAS-Analysis env
# https://github.com/MPAS-Dev/MPAS-Analysis/commits/develop/
# There has been 2 commits since `mpas_analysis v1.15.0`,
# so, we should use a dev env rather than the Unified env.
cd ~/ez/MPAS-Analysis
git status # Check that branch is `develop`, and for "nothing to commit, working tree clean"
git fetch upstream develop
git reset --hard upstream/develop
git log --oneline | head -n 3
# 22956fd31 Merge pull request #1150 from MPAS-Dev/dependabot/github_actions/prefix-dev/setup-pixi-0.9.6
# aac7e79b0 Bump prefix-dev/setup-pixi from 0.9.5 to 0.9.6
# f8f8a5c1e Merge pull request #1148 from xylar/update-to-1.15.0
 # Good, matches https://github.com/MPAS-Dev/MPAS-Analysis/commits/develop/
rm -rf build
conda clean --all --y
conda create --name test-mpas-analysis-develop-20260526 --file dev-spec.txt
conda activate test-mpas-analysis-develop-20260526
python -m pip install .

# Set up zppy-interfaces env
# https://github.com/E3SM-Project/zppy-interfaces/commits/main
# There have been NO commits since `zppy-interfaces 0.2.1`,
# so we can keep using the Unified env
zppy run
# zppy itself #################################################################
cd ~/ez/zppy
git status # Check for uncommitted changes
git fetch upstream enable-e3sm_to_cmip-eamxx
git checkout -b test-enable-e3sm_to_cmip-eamxx upstream/enable-e3sm_to_cmip-eamxx
git log --oneline # Good, matches https://github.com/E3SM-Project/zppy/pull/827/commits
rm -rf build
conda clean --all --y
conda env create -f conda/dev.yml -n test-zppy-pr827
conda activate test-zppy-pr827
python -m pip install .
pytest tests/test_*.py # 44 passed in 1.06s

conda list python # Check Python version
# python                    3.14.5

# This PR changed e3sm_diags.bash and e3sm_to_cmip.bash,
# so, we will want to run the tests for those tasks and any tasks that use them.
git grep --name-only e3sm_to_cmip zppy/*.py
# zppy/__main__.py
# zppy/e3sm_to_cmip.py
# zppy/ilamb.py
# zppy/pcmdi_diags.py
# zppy/ts.py
# => ilamb, pcmdi_diags depend on e3sm_to_cmip
# ts includes e3sm_to_cmip in two file paths:
# AI suggests: So the path e3sm_to_cmip_data/grids/vert_L128.nc means "the vert_L128.nc file that ships with the e3sm_to_cmip software package," not "a file produced by the e3sm_to_cmip zppy task." The e3sm_to_cmip task uses the same path for the same reason.
git grep --name-only e3sm_diags zppy/*.py
# zppy/climo.py
# zppy/e3sm_diags.py
# zppy/e3sm_to_cmip.py
# (climo, e3sm_to_cmip are just mentioning it in a comment)

# We'll want to set tasks_to_run to be this reduced list of tasks.

# We will however run the legacy tests to make sure this EAMxx support doesn't break backwards compatiblity.

# Edit tests/integration/utils.py:
TEST_SPECIFICS: Dict[str, Any] = {
    # This is the NCO path.
    # Keep as "" to use the production-version NCO commands.
    # Set to a specific path to use development-version NCO commands.
    "nco_path": "",
    # These are custom environment_commands for specific tasks.
    # Never set these to "", because they will print the line
    # `environment_commands = ""` for the corresponding task,
    # thus overriding the value set higher up in the cfg.
    # That is, there will be no environment set.
    # (`environment_commands = ""` only redirects to Unified
    # if specified under the [default] task)
    "e3sm_to_cmip_environment_commands": "source /gpfs/fs1/home/ac.forsyth2/miniforge3/etc/profile.d/conda.sh; conda activate test-e3sm-to-cmip-master-20260526",
    "diags_environment_commands": "source /gpfs/fs1/home/ac.forsyth2/miniforge3/etc/profile.d/conda.sh; conda activate test-e3sm-diags-main-20260526",
    "mpas_analysis_environment_commands": "source /gpfs/fs1/home/ac.forsyth2/miniforge3/etc/profile.d/conda.sh; conda activate test-mpas-analysis-develop-20260526",
    "global_time_series_environment_commands": "source /lcrc/soft/climate/e3sm-unified/load_latest_e3sm_unified_chrysalis.sh", # Keep Unified
    "livvkit_environment_commands": "source /lcrc/soft/climate/e3sm-unified/load_latest_e3sm_unified_chrysalis.sh", # Keep Unified
    "pcmdi_diags_environment_commands": "source /lcrc/soft/climate/e3sm-unified/load_latest_e3sm_unified_chrysalis.sh", # Keep Unified
    # This is the environment setup for other tasks.
    # Leave as "" to use the latest Unified environment.
    "environment_commands": "source /lcrc/soft/climate/e3sm-unified/load_latest_e3sm_unified_chrysalis.sh", # Keep Unified
    # For a complete test, run the set of latest cfgs and at least one set of legacy cfgs
    "cfgs_to_run": [
        "weekly_bundles",
        "weekly_comprehensive_v2",
        "weekly_comprehensive_v3",
        "weekly_legacy_3.1.0_bundles",
        "weekly_legacy_3.1.0_comprehensive_v2",
        "weekly_legacy_3.1.0_comprehensive_v3",
        "weekly_legacy_3.0.0_bundles",
        "weekly_legacy_3.0.0_comprehensive_v2",
        "weekly_legacy_3.0.0_comprehensive_v3",
    ],
    "tasks_to_run": [
        "e3sm_diags",
        # "mpas_analysis",
        # "global_time_series",
        "ilamb",
        # "livvkit",
        "pcmdi_diags",
    ],
    "unique_id": "test_pr827_20260526",
}
git diff # Diff looks good
python tests/integration/utils.py
# CFG FILES HAVE BEEN GENERATED FROM TEMPLATES WITH THESE SETTINGS:
# UNIQUE_ID=test_pr827_20260526
# nco_path=
# Reminder: `nco_path=''` => the production-version NCO commands will be used
# e3sm_to_cmip_environment_commands=source /gpfs/fs1/home/ac.forsyth2/miniforge3/etc/profile.d/conda.sh; conda activate test-e3sm-to-cmip-master-20260526 
# diags_environment_commands=source /gpfs/fs1/home/ac.forsyth2/miniforge3/etc/profile.d/conda.sh; conda activate test-e3sm-diags-main-20260526
# mpas_analysis_environment_commands=source /gpfs/fs1/home/ac.forsyth2/miniforge3/etc/profile.d/conda.sh; conda activate test-mpas-analysis-develop-20260526
# global_time_series_environment_commands=source /lcrc/soft/climate/e3sm-unified/load_latest_e3sm_unified_chrysalis.sh
# livvkit_environment_commands=source /lcrc/soft/climate/e3sm-unified/load_latest_e3sm_unified_chrysalis.sh
# pcmdi_diags_environment_commands=source /lcrc/soft/climate/e3sm-unified/load_latest_e3sm_unified_chrysalis.sh
# environment_commands=source /lcrc/soft/climate/e3sm-unified/load_latest_e3sm_unified_chrysalis.sh
# Reminder: `environment_commands=''` => the latest E3SM Unified environment will be used


# For reference:
alias sq
# Perlmutter: alias sq='sqa -u forsyth'
# Compy:      alias sq='sqa -u fors729'
# Chrysalis:  alias sq='sqa -u ac.forsyth2'
alias sqa
# Perlmutter: alias sqa='squeue -o "%8u %.7a %.4D %.9P %8i %.2t %.10r %.10M %.10l %j" --sort=P,-t,-p'
# Compy:      alias sqa='squeue -o "%8u %.7a %.4D %.9P %7i %.2t %.10r %.10M %.10l %.8Q %j" --sort=P,-t,-p'
# Chrysalis:  alias sqa='squeue -o "%8u %.7a %.4D %.9P %7i %.2t %.10r %.10M %.10l %j" --sort=P,-t,-p'
sq
# No jobs currently queued
ls tests/integration/generated/test_weekly_*_chrysalis.cfg # See all cfgs

# Launch jobs from ALL cfgs; start with the most important one, comprehensive_v3
zppy -c tests/integration/generated/test_weekly_comprehensive_v3_chrysalis.cfg
zppy -c tests/integration/generated/test_weekly_bundles_chrysalis.cfg
zppy -c tests/integration/generated/test_weekly_comprehensive_v2_chrysalis.cfg

zppy -c tests/integration/generated/test_weekly_legacy_3.1.0_bundles_chrysalis.cfg
zppy -c tests/integration/generated/test_weekly_legacy_3.1.0_comprehensive_v2_chrysalis.cfg
zppy -c tests/integration/generated/test_weekly_legacy_3.1.0_comprehensive_v3_chrysalis.cfg

zppy -c tests/integration/generated/test_weekly_legacy_3.0.0_bundles_chrysalis.cfg
zppy -c tests/integration/generated/test_weekly_legacy_3.0.0_comprehensive_v2_chrysalis.cfg
zppy -c tests/integration/generated/test_weekly_legacy_3.0.0_comprehensive_v3_chrysalis.cfg


sq | wc -l # Tue 05/26 16:21 => 167 - header row = 166 jobs

# Picking up 2026-05-27 #######################################################
sq | wc -l # Wed 05/27 12:45 => 1 - header row = 0 jobs
Bundles part 2
# Check on bundles status
cd /lcrc/group/e3sm/ac.forsyth2/zppy_weekly_bundles_output/test_pr827_20260526/v3.LR.historical_0051/post/scripts
grep -v "OK" *status # Good, no errors

cd /lcrc/group/e3sm/ac.forsyth2/zppy_weekly_legacy_3.1.0_bundles_output/test_pr827_20260526/v3.LR.historical_0051/post/scripts
grep -v "OK" *status # Good, no errors

cd /lcrc/group/e3sm/ac.forsyth2/zppy_weekly_legacy_3.0.0_bundles_output/test_pr827_20260526/v3.LR.historical_0051/post/scripts
grep -v "OK" *status # Good, no errors

# Now, run bundles part 2
cd ~/ez/zppy
git status
# Good, correct branch: test-enable-e3sm_to_cmip-eamxx
# Need to re-establish dev env:
bash # Run bash so we're in an isolated subshell
source ~/.bashrc
lcrc_conda
conda activate test-zppy-pr827

sq | wc -l # Wed 05/27 12:50 => 1 - header row = 0 jobs
zppy -c tests/integration/generated/test_weekly_bundles_chrysalis.cfg
zppy -c tests/integration/generated/test_weekly_legacy_3.1.0_bundles_chrysalis.cfg
zppy -c tests/integration/generated/test_weekly_legacy_3.0.0_bundles_chrysalis.cfg
sq | wc -l # Wed 05/27 12:50 => 7 - header row = 6 jobs
Review finished runs
### v2  ###
cd /lcrc/group/e3sm/ac.forsyth2/zppy_weekly_comprehensive_v2_output/test_pr827_20260526/v2.LR.historical_0201/post/scripts
grep -v "OK" *status # Good, no errors

cd /lcrc/group/e3sm/ac.forsyth2/zppy_weekly_legacy_3.0.0_comprehensive_v2_output/test_pr827_20260526/v2.LR.historical_0201/post/scripts
grep -v "OK" *status
# e3sm_diags_atm_monthly_180x360_aave_model_vs_obs_1982-1983.status:RUNNING 1223458

cd /lcrc/group/e3sm/ac.forsyth2/zppy_weekly_legacy_3.1.0_comprehensive_v2_output/test_pr827_20260526/v2.LR.historical_0201/post/scripts
grep -v "OK" *status # Good, no errors

### v3 ###
cd /lcrc/group/e3sm/ac.forsyth2/zppy_weekly_comprehensive_v3_output/test_pr827_20260526/v3.LR.historical_0051/post/scripts
grep -v "OK" *status # Good, no errors

cd /lcrc/group/e3sm/ac.forsyth2/zppy_weekly_legacy_3.0.0_comprehensive_v3_output/test_pr827_20260526/v3.LR.historical_0051/post/scripts
grep -v "OK" *status # Good, no errors

cd /lcrc/group/e3sm/ac.forsyth2/zppy_weekly_legacy_3.1.0_comprehensive_v3_output/test_pr827_20260526/v3.LR.historical_0051/post/scripts
grep -v "OK" *status # Good, no errors

### bundles ###
sq | wc -l # Wed 05/27 13:00 => 1 - header row = 0 jobs

cd /lcrc/group/e3sm/ac.forsyth2/zppy_weekly_bundles_output/test_pr827_20260526/v3.LR.historical_0051/post/scripts
grep -v "OK" *status # Good, still no errors

cd /lcrc/group/e3sm/ac.forsyth2/zppy_weekly_legacy_3.0.0_bundles_output/test_pr827_20260526/v3.LR.historical_0051/post/scripts
grep -v "OK" *status # Good, still no errors

cd /lcrc/group/e3sm/ac.forsyth2/zppy_weekly_legacy_3.1.0_bundles_output/test_pr827_20260526/v3.LR.historical_0051/post/scripts
grep -v "OK" *status # Good, still no errors
Debugging
cd /lcrc/group/e3sm/ac.forsyth2/zppy_weekly_legacy_3.0.0_comprehensive_v2_output/test_pr827_20260526/v2.LR.historical_0201/post/scripts
grep -v "OK" *status
# e3sm_diags_atm_monthly_180x360_aave_model_vs_obs_1982-1983.status:RUNNING 1223458
tail e3sm_diags_atm_monthly_180x360_aave_model_vs_obs_1982-1983.o1223458 
# 2026-05-26 21:18:37,416 [WARNING]: warnings.py(_showwarnmsg:110) >> /gpfs/fs1/home/ac.forsyth2/miniforge3/envs/test-e3sm-diags-main-20260526/lib/python3.13/site-packages/cartopy/mpl/geoaxes.py:856: UserWarning: Attempting to set identical low and high xlims makes transformation singular; automatically expanding.
#   self.set_xlim([x1, x2])

# 2026-05-26 21:18:41,532 [INFO]: utils.py(_save_main_plot:122) >> Plot saved in: model_vs_obs_1982-1983/lat_lon/ERA5/ERA5-TREFHT-SON-land.png
# srun: Job step aborted: Waiting up to 92 seconds for job step to finish.
# slurmstepd: error: *** STEP 1223458.0 ON chr-0316 CANCELLED AT 2026-05-27T01:31:25 DUE TO TIME LIMIT ***
# slurmstepd: error: *** JOB 1223458 ON chr-0316 CANCELLED AT 2026-05-27T01:31:25 DUE TO TIME LIMIT ***
# srun: got SIGCONT
# srun: forcing job termination

grep "\-\-time" e3sm_diags_atm_monthly_180x360_aave_model_vs_obs_1982-1983.bash
#SBATCH  --time=5:00:00

# So, it hit a 5 hour time limit that ended at 5/26 21:18 When did we start?
grep -n 2026-05-26 e3sm_diags_atm_monthly_180x360_aave_model_vs_obs_1982-1983.o1223458  | head -n 1
# 24:2026-05-26 20:32:08,708 [INFO]: e3sm_diags_driver.py(_log_diagnostic_run_info:525) >> 
# We started at 20:32, so we actually ran for only about 46 minutes before hanging.

# This is the intermittent bug again.
# https://github.com/E3SM-Project/zppy/issues/822
sq | wc -l # Wed 05/27 13:01 => 1 - header row = 0 jobs
sbatch e3sm_diags_atm_monthly_180x360_aave_model_vs_obs_1982-1983.bash
sq | wc -l # Wed 05/27 15:12 => 1 - header row = 0 jobs
grep -v "OK" *status # No errors now!

post/scripts directory status: One error -- e3sm_diags once again hit the intermittent bug of #822. Rerunning with sbatch, as expected, resolved the issue.

Python tests
cd ~/ez/zppy
git status
# Good, correct branch: test-enable-e3sm_to_cmip-eamxx
# Good, correct env: Unified
ls tests/integration/test_*.py

# Passing
pytest tests/integration/test_last_year.py # 1 passed in 0.81s

# Failing, but errors look expected based on the PR
pytest tests/integration/test_bash_generation.py # 1 failed in 2.52s
pytest tests/integration/test_campaign.py # 6 failed in 3.86s
pytest tests/integration/test_defaults.py # 1 failed in 1.02s
pytest tests/integration/test_bundles.py # 2 failed in 0.67s
# This is ok because we didn't actually run all the jobs,
# so there's some missing info.

salloc --nodes=1 --partition=debug --time=02:00:00 --account=e3sm
# Re-establish Unified env
bash
source ~/.bashrc
lcrc_conda
conda activate test-zppy-pr827
pytest tests/integration/test_images.py 
# 1 passed in 699.34s (0:11:39)
cat test_images_summary.md

Complete summary table:

Test name Total images Correct images Missing images Mismatched images
bundles_e3sm_diags 1762 1762 0 0
bundles_ilamb 388 388 0 0
comprehensive_v2_e3sm_diags 3806 3806 0 0
comprehensive_v2_ilamb 776 776 0 0
comprehensive_v3_e3sm_diags 5369 5369 0 0
comprehensive_v3_ilamb 837 837 0 0
comprehensive_v3_pcmdi_diags 647 647 0 0
legacy_3.1.0_bundles_e3sm_diags 1762 1762 0 0
legacy_3.1.0_bundles_ilamb 388 388 0 0
legacy_3.1.0_comprehensive_v2_e3sm_diags 3806 3806 0 0
legacy_3.1.0_comprehensive_v2_ilamb 776 776 0 0
legacy_3.1.0_comprehensive_v3_e3sm_diags 5365 5365 0 0
legacy_3.1.0_comprehensive_v3_ilamb 837 837 0 0
legacy_3.0.0_bundles_e3sm_diags 1762 1762 0 0
legacy_3.0.0_bundles_ilamb 388 388 0 0
legacy_3.0.0_comprehensive_v2_e3sm_diags 3806 3806 0 0
legacy_3.0.0_comprehensive_v2_ilamb 776 776 0 0
legacy_3.0.0_comprehensive_v3_e3sm_diags 5365 5365 0 0
legacy_3.0.0_comprehensive_v3_ilamb 837 837 0 0

Results

Other than the re-run required by the #822-affected job, I didn't encounter any issues in testing. Note I only ran the jobs that would be affected by a change to e3sm_to_cmip or e3sm_diags.

Comment thread zppy/defaults/default.ini Outdated
# and written to a sibling directory `ts_vert_remap/` parallel to `ts/`, so users
# can use native-level and pressure-level data at the same time.
# If empty, no vertical regrid is performed.
vert_remap_vars = string(default="")

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.

It's a little inconsistent to use vrt for 2 of the parameters and vert for another. But I don't know if it's worth changing (and risking breaking something else) at this point.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Good catch! Let me try to rename vert_remap_vars → vrt_remap_vars — aligns everything with NCO's vrt_ prefix to be consistent. Since these are newly added parameters, it would be good to fix before releasing to the users.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@zhangshixuan1987 just a quick note on this: you may need to rename vert_remap_vars to vrt_remap_vars. This change was made to reduce variation in parameter names.

Comment thread zppy/e3sm_to_cmip.py
# EAM/CAM have vertical coord info embedded in the file already.
if c["prc_typ"] == "eamxx":
default_vrt_in_file = (
f"{c['diagnostics_base_path']}/e3sm_to_cmip_data/grids/vert_L128.nc"

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.

Prefixing with diagnostics_base_path should already imply the data are pre-installed

Yes I suppose looking at it again, that makes sense. It was just a little confusing when I was trying to figure out which tasks had e3sm_to_cmip as a dependency. A comment might be nice, but not necessary.

Align all vertical-remap naming with NCO's vrt_ prefix convention,
matching the existing vrt_remap_file and vrt_in_file parameters.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@chengzhuzhang
chengzhuzhang merged commit 7729b56 into main May 28, 2026
7 checks passed
@chengzhuzhang
chengzhuzhang deleted the enable-e3sm_to_cmip-eamxx branch May 28, 2026 21:01
@czender

czender commented May 29, 2026

Copy link
Copy Markdown

@chengzhuzhang Where can I find a comparable set of EAMv3 (not EAMxx) 3-hourly files so I can compare performance?

@chengzhuzhang

chengzhuzhang commented May 29, 2026

Copy link
Copy Markdown
Collaborator Author

@chengzhuzhang Where can I find a comparable set of EAMv3 (not EAMxx) 3-hourly files so I can compare performance?

@czender I have been using:/global/cfs/cdirs/e3sm/www/Tutorials/2024/simulations/extendedOutput.v3.LR.historical_0101/archive/atm/hist/extendedOutput.v3.LR.historical_0101.eam.h3.*nc. Thanks!

@czender

czender commented May 29, 2026

Copy link
Copy Markdown

@chengzhuzhang I have discovered the cause for the slowdown of the high frequency climos (HFCs) from EAMxx relative to EAMv3:
The EAMxx test dataset archives only 8 timesteps per file, 365 files per year. This requires opening and closing a vast number of files multiples times compared to EAMv3 which stores ~240 timesteps per file and ~12 files per year. When I rebin the EAMxx daily datasets into monthly datasets and run the HFC climos on those, the speed is order 100 times faster, and takes only ~1 minute to process 10 years of three hourly data on one node.

The only ways that I can think of to remedy this are:

  1. Rebin EAMxx 3-hourly data into fewer, larger files (e.g., monthly) prior to HFC climatology.
  2. Suggest that EAMxx output high frequency data into ~monthly files to begin with.
    Both of those options have downsides so ...feedback welcome!
    In the meantime, I will try a few more permutations of the ncclimo invocation to better characterize the issue.

@chengzhuzhang

chengzhuzhang commented May 29, 2026

Copy link
Copy Markdown
Collaborator Author

@czender thank you so much. I should have thought about this..
After taking a closer look at one of the 3ha_ne30pg2.AVERAGE.nhours_x3 files, I found that each file is around 1.1 GB and contains many variables (both 2D and 3D), at ne30pg2.

In this case, saving EAMxx high-frequency data into roughly monthly files would probably only be practical at ne30 resolution. Personally, between these two options, I would lean toward selecting variables and regridding first, prior to HFC climatology...

@czender

czender commented Jun 3, 2026

Copy link
Copy Markdown

@chengzhuzhang While investigating approaches to reduce the HFC time requirement on PM, I copied the entire 10-year daily dataset of 3-hourly values to my laptop and to Chrysalis, and I repeated the timing tests on these two machines. My laptop and Chrysalis both finish the entire 10-year 3-hourly HFC climo in 3 minutes or less (on only one node!). Thus it appears the problem is specific to Perlmutter which appears to deccelerate when operating on large numbers of small files (even when on compute nodes). This is the kind of issue that @ndkeen may be able to help with. Perhaps the srun command can be optimized, or something else. I think we should consult with Noel before rearranging the HFC workflow or modifying ncclimo. I will Slack with him and give him a reproducer soon.

@chengzhuzhang

Copy link
Copy Markdown
Collaborator Author

My laptop and Chrysalis both finish the entire 10-year 3-hourly HFC climo in 3 minutes or less (on only one node!).

@czender Super interesting, and somewhat surprising! Since the issue appears to be machine-specific, I’ll hold off on any attempts to change zppy for now. Hopefully this can be resolved on the system side without requiring workflow changes from us.

@czender

czender commented Jun 3, 2026

Copy link
Copy Markdown

p.s. the data are in /home/ac.zender/data/ne256/raw on Chrysalis if you want to verify that my tests are an accurate reflection of what zppy would get.

@forsyth2 forsyth2 mentioned this pull request Jun 19, 2026
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.

[Bug]: fragile component detection before vertical interpolation [Feature]: EAMxx cmorization via zppy

6 participants