Skip to content

Improve TC analysis workflow robustness for both EAM and EAMxx#828

Merged
chengzhuzhang merged 8 commits into
mainfrom
zppy-tc
Jun 4, 2026
Merged

Improve TC analysis workflow robustness for both EAM and EAMxx#828
chengzhuzhang merged 8 commits into
mainfrom
zppy-tc

Conversation

@zhangshixuan1987

@zhangshixuan1987 zhangshixuan1987 commented May 15, 2026

Copy link
Copy Markdown
Collaborator

Summary

Objectives:

  • Improve the robustness of the TC analysis workflow.
  • Support both EAM and EAMxx variable naming conventions through an ordered tc_vars mapping.
  • Derive the TempestExtremes mesh resolution and pg2 flag directly from the configured grid name.
  • Replace hard-coded TempestExtremes variable names with configurable variables.
  • Improve error handling and avoid misleading downstream failures when no tracks are detected.

Issue resolution:

  • Closes #<ISSUE_NUMBER_HERE>

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

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.

configuration file for test with EAMXX data


# Directions to run:
# 1. Update <output>, <www>, <environment_commands_secondary> below.
# 2. Run with `zppy -c examples/post.v3.LR.amip.0101.cfg`.
# Direction to create stand-alone test data for zppy-interfaces:
# 3. Once the jobs finish, `cd <output>/post/scripts`.
# 4. Run `grep -n "Running a zi-pcmdi command" pcmdi_diags*.o*` to find the pcmdi_diags commands.
# 5. Then, you can run those lines stand-alone.
[default]
input = /pscratch/sd/z/zhan391/e3smv4_project/ne256pg2_ne256pg2.F20TR-SCREAMv1.July-1.spanc800.2xauto.acc150.n0032.test2.1
output = /pscratch/sd/z/zhan391/e3smv4_project/ne256pg2_ne256pg2.F20TR-SCREAMv1.July-1.spanc800.2xauto.acc150.n0032.test2.1
case = ne256pg2_ne256pg2.F20TR-SCREAMv1.July-1.spanc800.2xauto.acc150.n0032.test2.1
www = /global/cfs/cdirs/e3sm/www/zhan391/eamxx-pcmdi

partition = "debug"
account = "e3sm"
#account = "priority"
campaign = "water_cycle"
debug = False
environment_commands = "source /global/common/software/e3sm/anaconda_envs/load_latest_e3sm_unified_pm-cpu.sh"

[tc_analysis]
active = True
walltime = "02:00:00" # Example elapsed time: 3296 seconds (55 minutes)
years = "1995:2004:10",
input_component = "eamxx"
case = "6ha_ne30pg2"
input_files = "AVERAGE.nhours_x6"
#input_subdir = "archive/atm/hist"
input_subdir = "run/"
ts_grid = "ne30pg2"
# Note: Users must provide the variable list in the fixed sequence required by
# this TempestExtremes cyclone-detection workflow:
# SLP,T@200hPa,T@500hPa,U@model_bottom,V@model_bottom,U@850hPa,V@850hPa
# For example:
# EAM: vars="PSL,T200,T500,UBOT,VBOT,U850,V850"
# EAMxx: vars="SeaLevelPressure,T_mid_at_200hPa,T_mid_at_500hPa,U_at_model_bot,V_at_model_bot,U_at_850hPa,V_at_850hPa"
tc_vars = "SeaLevelPressure,T_mid_at_200hPa,T_mid_at_500hPa,U_at_model_bot,V_at_model_bot,U_at_850hPa,V_at_850hPa"

- Derive resolution and pg2 flag from grid name
- Add ordered tc_vars mapping for EAM and EAMxx variable conventions
- Replace hard-coded TempestExtremes variable names
- Add return-code checks for mesh generation and analysis commands
- Skip histograms gracefully when no tracks are detected
- Use mintime instead of deprecated minlength"
@zhangshixuan1987
zhangshixuan1987 marked this pull request as draft May 15, 2026 00:23
@zhangshixuan1987

Copy link
Copy Markdown
Collaborator Author

@chengzhuzhang : Please see the log file in the following directory for my test:

/pscratch/sd/z/zhan391/e3smv4_project/ne256pg2_ne256pg2.F20TR-SCREAMv1.July-1.spanc800.2xauto.acc150.n0032.test2.1/post/scripts/tc_analysis_1995-2004.*

I think the processing was successful, but I need to check the plots to see if the results are reasonable. Another thing is that the AEW tracking was empty, need to double-check if this is real or if something unexpected behavior was triggered for this part.

@kaizhangpnl kaizhangpnl left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@zhangshixuan1987 Thanks for working on this PR. It looks good to me. For the empty AEW tracking issue, I noticed that if we use higher resolution data, AEW tracking seems to be working.

@zhangshixuan1987

zhangshixuan1987 commented May 20, 2026

Copy link
Copy Markdown
Collaborator Author

@zhangshixuan1987 Thanks for working on this PR. It looks good to me. For the empty AEW tracking issue, I noticed that if we use higher resolution data, AEW tracking seems to be working.

For AEW tracking, there is also a physical/numerical question about how best to derive the vorticity used for tracking. Since AEW tracking depends on coherent vorticity maxima, it may matter whether vorticity is computed directly from U/V winds on the native SE grid or after remapping the winds to a regular latitude–longitude grid. In principle, deriving vorticity on the SE grid is reasonable if the calculation uses a grid-aware SE/spherical derivative operator. However, a simple finite-difference calculation that treats the SE grid as a regular latitude–longitude grid could introduce numerical artifacts, especially near element boundaries.

I am not sure whether TempestExtremes uses the appropriate grid-aware algorithm for calculating vorticity on the SE grid. I think this is the first thing we should clarify, to make sure the tracking results are physically and numerically reasonable rather than only making the workflow run successfully.

To me, a more robust and consistent alternative would be to use regridded latitude–longitude data for all TC and AEW tracking and statistics. This would make comparisons across simulations with different native grids fairer, because the tracking algorithm would be applied to fields on the same grid, with the same spatial resolution and grid geometry. A useful sensitivity test may be to compare AEW tracks from: (1) vorticity derived on the native grid and then remapped to latitude–longitude, and (2) U/V remapped first, followed by vorticity calculation and tracking on the common latitude–longitude grid. This is a question also for the developer of the tc tracking and diagnostics for e3sm-diags

@chengzhuzhang

Copy link
Copy Markdown
Collaborator

@zhangshixuan1987 TempestExtremes natively handles calculations on unstructured grids like the SE grid, and in my view, being able to track directly on native grids helps avoid artifacts introduced by remapping to lat-lon grids. For the model-vs-obs application, the target dataset IBTrACS provides individual storm tracks, so as a first check, it would be useful to evaluate results on the native grid.

The TC and AEW diagnostics are developed based on Karthik’s script (https://agupubs.onlinelibrary.wiley.com/doi/full/10.1029/2019MS002024). I agree that additional sensitivity studies would be valuable to better understand grid sensitivity. I haven’t looked closely at your implementation yet, but I’m wondering whether the analyses would differ when using native ne256 output versus output that has been remapped online to ne30.

@zhangshixuan1987

zhangshixuan1987 commented May 20, 2026

Copy link
Copy Markdown
Collaborator Author

@zhangshixuan1987 TempestExtremes natively handles calculations on unstructured grids like the SE grid, and in my view, being able to track directly on native grids helps avoid artifacts introduced by remapping to lat-lon grids. For the model-vs-obs application, the target dataset IBTrACS provides individual storm tracks, so as a first check, it would be useful to evaluate results on the native grid.

The TC and AEW diagnostics are developed based on Karthik’s script (https://agupubs.onlinelibrary.wiley.com/doi/full/10.1029/2019MS002024). I agree that additional sensitivity studies would be valuable to better understand grid sensitivity. I haven’t looked closely at your implementation yet, but I’m wondering whether the analyses would differ when using native ne256 output versus output that has been remapped online to ne30.

@chengzhuzhang I agree that, for TC tracking, using PSL directly output by the model is better done on the native model grid. By the way, since this application will likely involve remapping ne256 output to ne30 (or something else) as well, the argument about avoiding artifacts introduced by remapping to lat-lon grids seems somewhat weaker to me, or we should always ask for the native ne256 or ne1024 output for the TC diagnostics to better reserve your statement here.

My point here is specifically about AEW tracking, which uses vorticity rather than a variable directly output by the model. In this case, the workflow involves two steps: first, deriving vorticity from the U and V wind fields at a specific pressure level, such as 850 hPa; and second, tracking AEWs using the derived vorticity field. My concern is mainly about the first step: how the vorticity is calculated from U/V winds on the SE grid, and whether that calculation is using a grid-aware derivative operator appropriate for the native grid.

Overall, I should clarify that I do not have any objections to, or specific questions about, the current TC and AEW diagnostics developed based on Karthik’s script. I am simply trying to share some thoughts on whether we could have a more robust and physically consistent treatment of the feature tracking, especially when applying the diagnostics across different grids and resolutions.

@rljacob

rljacob commented May 20, 2026

Copy link
Copy Markdown
Member

I believe UXarray can calculate vorticity directly on the unstructured grid with a grid-aware operator.

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

This PR updates the tc_analysis workflow template to be more configurable and
robust, particularly for supporting both EAM and EAMxx variable naming and for
deriving TempestExtremes settings from a configured grid name.

Changes:

  • Derives res and pg2 directly from a configured ts_grid value.
  • Replaces hard-coded TempestExtremes variable names with an ordered tc_vars
    mapping and validates its length.
  • Adds additional checks / early-exit behavior intended to avoid misleading
    failures when no tracks are detected.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.

File Description
zppy/templates/tc_analysis.bash Refactors grid handling and variable naming, and adds additional error handling / skipping logic.
zppy/defaults/default.ini Introduces ts_grid/tc_vars defaults for tc_analysis and removes res from defaults.

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

Comment thread zppy/templates/tc_analysis.bash Outdated
Comment on lines +29 to +33
# Derive numeric res and pg2 from grid.
# Expected forms: ne30pg2, ne30np4, ne120pg2, ne120np4
grid="{{ ts_grid }}"
if [[ "${grid}" =~ ^ne([0-9]+)(pg2|np4)$ ]]; then
res="${BASH_REMATCH[1]}"

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 can confirm that using topography file to infer grids is not reliable. By point checking a eamxx file, I see:topography_file = "NONE"

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 is a good idea to implement a fall-back for backward-compatibiliby though.

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.

26af251 rename ts_grid → input_grid; restore res back-compat

Comment on lines +95 to +105
GenerateCSMesh \
--res "${res}" \
--alt \
--file "${result_dir}outCSMeshne${res}.g"

if [ $? != 0 ]; then
echo "ERROR: GenerateCSMesh failed."
cd {{ scriptDir }}
echo 'ERROR (3)' > {{ prefix }}.status
exit 3
fi

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.

c91ff87 stop errexit after env setup

Comment thread zppy/templates/tc_analysis.bash Outdated
Comment on lines +177 to +186
eval ls "${drc_in}/${caseid}.${input_files}."*{${start}..${end}}*"*.nc" > "${result_dir}inputfile_${file_name}.txt"
if [ $? != 0 ] || [ ! -s "${result_dir}inputfile_${file_name}.txt" ]; then
echo "ERROR: no input files found for ${caseid}.${input_files} between ${start} and ${end}."
cd {{ scriptDir }}
echo 'ERROR (8)' > {{ prefix }}.status
exit 8
fi

eval ls "${caseid}.${input_files}."*{${start}..${end}}*"*.nc" > "${result_dir}outputfile_${file_name}.txt"
if [ $? != 0 ] || [ ! -s "${result_dir}outputfile_${file_name}.txt" ]; then
Comment thread zppy/templates/tc_analysis.bash Outdated
Comment on lines +177 to +186
eval ls "${drc_in}/${caseid}.${input_files}."*{${start}..${end}}*"*.nc" > "${result_dir}inputfile_${file_name}.txt"
if [ $? != 0 ] || [ ! -s "${result_dir}inputfile_${file_name}.txt" ]; then
echo "ERROR: no input files found for ${caseid}.${input_files} between ${start} and ${end}."
cd {{ scriptDir }}
echo 'ERROR (8)' > {{ prefix }}.status
exit 8
fi

eval ls "${caseid}.${input_files}."*{${start}..${end}}*"*.nc" > "${result_dir}outputfile_${file_name}.txt"
if [ $? != 0 ] || [ ! -s "${result_dir}outputfile_${file_name}.txt" ]; then

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.

d81d36e restore eval ls glob pattern

Comment thread zppy/defaults/default.ini Outdated
Comment on lines +186 to +187
# explicitly set the model grid such as ne30pg2, ne120pg2 etc.
# so that res and grid setup will be inferred from the ts_grid
@chengzhuzhang
chengzhuzhang marked this pull request as ready for review May 26, 2026 20:49
chengzhuzhang and others added 4 commits May 26, 2026 14:39
- Address review (Copilot, chengzhuzhang): the new key collided with
  [e3sm_diags].ts_grid (the post-remap target grid name); rename to
  input_grid which parallels input_files/input_subdir in the same
  section and unambiguously refers to the native grid of the input.
- Restore res in [tc_analysis] defaults and reintroduce the legacy
  topography-file inference fallback so existing EAM configs
  (e.g. template_min_case_tc_analysis_res.cfg and
  template_min_case_tc_analysis_only.cfg) keep working without
  modification. For EAMxx, set input_grid explicitly (EAMxx output
  sets topography_file="NONE", so inference cannot work).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Address review (Copilot): with set -e on for the whole script, the
newly added "if [ $? != 0 ]; then ... exit N; fi" error-handling
blocks were dead code -- the shell exited at the failed command
before reaching them, leaving the job in RUNNING with no status
file written.

Every other zppy template (climo, ts, e3sm_diags, e3sm_to_cmip,
global_time_series, ilamb, livvkit, mpas_analysis, pcmdi_diags) uses
"set -e" only to guard environment_commands and then turns it off
with "set +e". Follow that convention so the existing error checks
in this template run as intended.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Address review (Copilot, x2): the quoting around "${drc_in}/..."
and "*.nc" turned the trailing ".nc" suffix into a literal-match
character class for the first round of pathname expansion. While
eval re-parses and globbing usually succeeds anyway on a default
shell, the pattern breaks under failglob/nullglob and is brittle.
Restore the original unquoted glob form (pre-PR style), which works
reliably.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Pre-commit cleanup; no functional change.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@zhangshixuan1987

zhangshixuan1987 commented May 26, 2026

Copy link
Copy Markdown
Collaborator Author

@chengzhuzhang, as I discussed with @kaizhangpnl on the TC analysis confluence page, for TC tracking it would be helpful to enable the threshold parameter zs — or PHIS/9.8 for E3SM — to reduce false alarms that often appear over regions with rough topography due to sea-level pressure correction.

This is especially useful for high-resolution simulations, where lakes or rivers over land can be resolved and may be identified as ocean-like regions. As a result, some unintended tracks may be detected and retained unless an elevation/topography threshold is applied. I just brought it here as a discussion for your reference @chengzhuzhang , and you may discuss with @kaizhangpnl and Karthik to see whether extra actions should be taken on this.

@chengzhuzhang

Copy link
Copy Markdown
Collaborator

@zhangshixuan1987 Thanks for the notes! The current tracking script was developed mainly for ne30 and applied to ne120 in several occations, but we shouldn’t expect it to apply to higher resolutions out of the box. The suggested constraints seem reasonable. It would also be helpful to include some discussion on how threshold/constraint choices may need to change with resolution.

@zhangshixuan1987

Copy link
Copy Markdown
Collaborator Author

@zhangshixuan1987 Thanks for the notes! The current tracking script was developed mainly for ne30 and applied to ne120 in several occations, but we shouldn’t expect it to apply to higher resolutions out of the box. The suggested constraints seem reasonable. It would also be helpful to include some discussion on how threshold/constraint choices may need to change with resolution.

@chengzhuzhang Thank you for the clarification. The surface geopotential height guard is mainly intended to provide a cleaner separation between TC-like vortices over land and over ocean in the TempestExtremes tracking. I agree that it would be valuable to discuss how the threshold and constraint choices may need to vary with resolution. For the tracking workflow itself, I think adding an optional input parameter that allows users to specify a surface-height threshold should be sufficient. Users may need to adjust this threshold to include or exclude tracks near coastal regions, depending on the analysis target, for example, whether the focus is on landfalling TCs or open-ocean-only TCs. This is more of an analysis-design choice than a core tracking-workflow issue.

@chengzhuzhang

Copy link
Copy Markdown
Collaborator

@chengzhuzhang : Please see the log file in the following directory for my test:

/pscratch/sd/z/zhan391/e3smv4_project/ne256pg2_ne256pg2.F20TR-SCREAMv1.July-1.spanc800.2xauto.acc150.n0032.test2.1/post/scripts/tc_analysis_1995-2004.*

I think the processing was successful, but I need to check the plots to see if the results are reasonable. Another thing is that the AEW tracking was empty, need to double-check if this is real or if something unexpected behavior was triggered for this part.

@zhangshixuan1987 I have addressed copilot-review, fixed the CI/CD tests. Also my test seems to run okay. Could you open up the permission for /pscratch/sd/z/zhan391/e3smv4_project/ne256pg2_ne256pg2.F20TR-SCREAMv1.July-1.spanc800.2xauto.acc150.n0032.test2.1/post/scripts/tc_analysis_1995-2004.* so that I can compare with my results?

The only user_facing change I made based on your code is to rename ts_grid to input_grid, because the former name is already used for a parameter under e3sm_diags task for a different purpose.

I’m not sure whether you plan to refine the TC tracking script now, or if it would be better handled in a separate PR later. With #827 and #828, EAMxx developers should be able to use zppy for end-to-end processing more readily.

@zhangshixuan1987

zhangshixuan1987 commented May 27, 2026

Copy link
Copy Markdown
Collaborator Author

@zhangshixuan1987 I have addressed copilot-review, fixed the CI/CD tests. Also my test seems to run okay. Could you open up the permission for /pscratch/sd/z/zhan391/e3smv4_project/ne256pg2_ne256pg2.F20TR-SCREAMv1.July-1.spanc800.2xauto.acc150.n0032.test2.1/post/scripts/tc_analysis_1995-2004.* so that I can compare with my results?

@chengzhuzhang : I have changed the permission, and you should have access to files.

The only user_facing change I made based on your code is to rename ts_grid to input_grid, because the former name is already used for a parameter under e3sm_diags task for a different purpose.

@chengzhuzhang : This sounds good to me.

I’m not sure whether you plan to refine the TC tracking script now, or if it would be better handled in a separate PR later. With #827 and #828, EAMxx developers should be able to use zppy for end-to-end processing more readily.

@chengzhuzhang: I think it is better for you to check with @kaizhangpnl and Karthik first, since I want to be respectful of their ongoing work on the TC tracking script and avoid making major changes without their input. My current thought is that #827 and #828 already make it easier for EAMxx developers to use zppy for end-to-end processing. If the TC tracking refinement requires more discussion or broader changes, it may be cleaner to handle it in a separate PR

When case is overridden in [tc_analysis], the output filenames should
still use the top-level case name (default_case) so that downstream
consumers (e.g. e3sm_diags tc_analysis set) can locate the files with
the expected naming convention.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@chengzhuzhang

chengzhuzhang commented May 27, 2026

Copy link
Copy Markdown
Collaborator

@zhangshixuan1987 Thank you! I just confirmed that our tc files produce same results for TC counts.
And I agree that we should spend more time to discuss the refinement for the tracking script with folks who are developers and familiar with features, and the update can go in as separate PRs for e3sm_diags/zppy.

P.S. I submit this issue E3SM-Project/e3sm_diags#1058 to e3sm_diags, so that when AEW file was not created ,e3sm_diags can still run for other TC diags.

@chengzhuzhang
chengzhuzhang requested a review from forsyth2 May 27, 2026 20:47
@forsyth2

Copy link
Copy Markdown
Collaborator

PR is also ready to review. I think we can plan to merge #827 ( a much bigger PR) first and then this one.

@chengzhuzhang Thanks, I think you saw I just approved #827. Hopefully I'll be able to review and test this PR tomorrow. Once both #827 and #828 are merged, we should definitely add in a new EAMxx test cfg too (part of #837).

@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.

From visual inspection alone, this looks alright. I'm also trying to start the tests on tc_analysis (and e3sm_diags using the tc_analysis set), rebased off main now that #827 has merged. (I'm having some trouble connecting to Chrysalis but hopefully that resolves soon).

@forsyth2

Copy link
Copy Markdown
Collaborator

I'm also trying to start the tests

I got the tests going, but it's been slow to get nodes today. I'll check in on the results on Monday.

@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.

Testing

Determine which tests to run

This PR touches zppy/templates/tc_analysis.bash

cd ~/ez/zppy
git status # Check for uncommitted changes
git fetch upstream zppy-tc
git checkout -b test-pr828-zppy-tc upstream/zppy-tc
git log --oneline | head -n 7
# ce784b76 tc_analysis: use default_case for output filename
# 511b0343 tc_analysis: trim trailing whitespace
# d81d36e3 tc_analysis: restore eval ls glob pattern
# c91ff873 tc_analysis: stop errexit after env setup, matching other templates
# 26af251d tc_analysis: rename ts_grid to input_grid; restore res back-compat
# 9b84b5d6 Improve TC analysis workflow robustness
# 17ce9335 Bump to 3.2.0 (#826)
# Good, matches https://github.com/E3SM-Project/zppy/pull/828/commits
git grep --name-only tc_analysis zppy/*.py
# zppy/__main__.py
# zppy/e3sm_diags.py
# zppy/tc_analysis.py

e3sm_diags depends on tc_analysis, so let's run the tc_analysis jobs and the e3sm_diags jobs that use it.

# Rebase off the latest `main` to get the changes from #827
git fetch upstream main
git rebase upstream/main
git log --oneline | head -n 9
# 99490512 tc_analysis: use default_case for output filename
# a87257bd tc_analysis: trim trailing whitespace
# 0d2b4c9d tc_analysis: restore eval ls glob pattern
# 9a48dfac tc_analysis: stop errexit after env setup, matching other templates
# 47520713 tc_analysis: rename ts_grid to input_grid; restore res back-compat
# 5d582b59 Improve TC analysis workflow robustness
# 7729b56f Enable EAMxx CMORization, E3SM Diags and add ts-level vertical regrid (#827)
# d57b263c Rewrite AGENTS.md in second person (#795)
# 17ce9335 Bump to 3.2.0 (#826)

# tc_analysis is turned off in tests/integration/template_weekly_comprehensive_v3.cfg:

# TODO: Add "tc_analysis" back in after empty dat is resolved.
# [tc_analysis]
# active = True
# walltime = "00:30:00"

# Let's turn that back on and see if this PR (or one of the other recent ones) have resolved this issue
git diff
# -# TODO: Add "tc_analysis" back in after empty dat is resolved.
# -# [tc_analysis]
# -# active = True
# -# walltime = "00:30:00"
# +[tc_analysis]
# +active = #expand active_e3sm_diags#
# +walltime = "00:30:00"

# -sets = "lat_lon","zonal_mean_xy","zonal_mean_2d","polar","cosp_histogram","meridional_mean_2d","annual_cycle_zonal_mean","zonal_mean_2d_stratosphere","enso_diags","qbo","diurnal_cycle","streamflow","tropical_subseasonal","aerosol_aeronet",
# +sets = "lat_lon","zonal_mean_xy","zonal_mean_2d","polar","cosp_histogram","meridional_mean_2d","annual_cycle_zonal_mean","zonal_mean_2d_stratosphere","enso_diags","qbo","diurnal_cycle","streamflow","tc_analysis","tropical_subseasonal","aerosol_aeronet",

We've rebased off main to include the changes from #827. We'll run the tc_analysis jobs and the e3sm_diags jobs, since they use tc_analysis.

Set up environments

We just need to set up the environment for e3sm_diags:

lcrc_conda # Bash function to activate conda.

# 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 
# 5/18: 9d5ebf19 Add targeted image-regression and E3SM Unified CI coverage (#1053)
# 5/12: ff3ef00d Bump v3.2.0 (#1056)
# Good, matches https://github.com/E3SM-Project/e3sm_diags/commits/main
# We can reuse the existing env:
conda activate test-e3sm-diags-main-20260526
python -m pip install .
zppy run
# zppy itself #################################################################
cd ~/ez/zppy
git status 
# Branch: test-pr828-zppy-tc
git diff
git add -A
rm -rf build
conda clean --all --y
conda env create -f conda/dev.yml -n test-zppy-pr828
conda activate test-zppy-pr828
git commit -m "Turn on tc_analysis in v3 cfg"
python -m pip install .
pytest tests/test_*.py # 44 passed in 0.50s

conda list python # Check Python version
# python                    3.14.5

git grep --name-only tc_analysis tests/integration/template_weekly_*.cfg
# tests/integration/template_weekly_bundles.cfg
# tests/integration/template_weekly_comprehensive_v2.cfg
# tests/integration/template_weekly_comprehensive_v3.cfg
# tests/integration/template_weekly_legacy_3.0.0_bundles.cfg
# tests/integration/template_weekly_legacy_3.0.0_comprehensive_v2.cfg
# tests/integration/template_weekly_legacy_3.0.0_comprehensive_v3.cfg
# tests/integration/template_weekly_legacy_3.1.0_bundles.cfg
# tests/integration/template_weekly_legacy_3.1.0_comprehensive_v2.cfg
# tests/integration/template_weekly_legacy_3.1.0_comprehensive_v3.cfg

# All cfgs use tc_analysis, so we should run them all.
# More might have tc_analysis commented out though
git grep "\[tc_analysis\]" tests/integration/template_weekly_*.cfg
# tests/integration/template_weekly_bundles.cfg:# [tc_analysis]
# tests/integration/template_weekly_comprehensive_v2.cfg:[tc_analysis]
# tests/integration/template_weekly_comprehensive_v3.cfg:[tc_analysis]
# tests/integration/template_weekly_legacy_3.0.0_bundles.cfg:# [tc_analysis]
# tests/integration/template_weekly_legacy_3.0.0_comprehensive_v2.cfg:[tc_analysis]
# tests/integration/template_weekly_legacy_3.0.0_comprehensive_v3.cfg:# [tc_analysis]
# tests/integration/template_weekly_legacy_3.1.0_bundles.cfg:# [tc_analysis]
# tests/integration/template_weekly_legacy_3.1.0_comprehensive_v2.cfg:[tc_analysis]
# tests/integration/template_weekly_legacy_3.1.0_comprehensive_v3.cfg:# [tc_analysis]

# Enable those as well
git diff tests/integration/template_weekly_*.cfg # Check the diff
git grep "\[tc_analysis\]" tests/integration/template_weekly_*.cfg # None are commented out now
git grep -B 1 "\[tc_analysis\]" tests/integration/template_weekly_*.cfg
# Check that we haven't accidentally included an uncommented version of the TODO line
git add -A
pre-commit run --all-files
git add -A
python -m pip install .

# We previously determined we need to run tc_analysis, and e3sm_diags since it uses tc_analysis.

# 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 /lcrc/soft/climate/e3sm-unified/load_latest_e3sm_unified_chrysalis.sh", # Keep Unified
    "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 /lcrc/soft/climate/e3sm-unified/load_latest_e3sm_unified_chrysalis.sh", # Keep Unified
    "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_pr828_20260529_try3",
}
git diff # Diff looks good
python tests/integration/utils.py
# CFG FILES HAVE BEEN GENERATED FROM TEMPLATES WITH THESE SETTINGS:
# UNIQUE_ID=test_pr828_20260529_try3
# nco_path=
# Reminder: `nco_path=''` => the production-version NCO commands will be used
# e3sm_to_cmip_environment_commands=source /lcrc/soft/climate/e3sm-unified/load_latest_e3sm_unified_chrysalis.sh
# 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 /lcrc/soft/climate/e3sm-unified/load_latest_e3sm_unified_chrysalis.sh
# 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 # Fri 05/29 14:46 => 127 - header row = 126 jobs

# Picking up 2026-06-01 #######################################################
sq | wc -l # Mon 06/01 10:39 => 1 - header row = 0 jobs
Bundles part 2
# Check on bundles status
cd /lcrc/group/e3sm/ac.forsyth2/zppy_weekly_bundles_output/test_pr828_20260529_try3/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_pr828_20260529_try3/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_pr828_20260529_try3/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-pr828-zppy-tc
# Need to re-establish dev env:
bash # Run bash so we're in an isolated subshell
source ~/.bashrc
lcrc_conda
conda activate test-zppy-pr828

sq | wc -l # Mon 06/01 10:40 => 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
Review finished runs
### v2  ###
cd /lcrc/group/e3sm/ac.forsyth2/zppy_weekly_comprehensive_v2_output/test_pr828_20260529_try3/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_pr828_20260529_try3/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_pr828_20260529_try3/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_pr828_20260529_try3/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_pr828_20260529_try3/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_pr828_20260529_try3/v3.LR.historical_0051/post/scripts
grep -v "OK" *status # Good, no errors

### bundles ###
sq | wc -l # Wed 05/27 10:55 => 4 - header row = 3 jobs

cd /lcrc/group/e3sm/ac.forsyth2/zppy_weekly_bundles_output/test_pr828_20260529_try3/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_pr828_20260529_try3/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_pr828_20260529_try3/v3.LR.historical_0051/post/scripts
grep -v "OK" *status # Good, still no errors

post/scripts directory status: NO ERRORS

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

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

# Failing, but errors look expected based on the PR
pytest tests/integration/test_bash_generation.py # 1 failed in 1.30s
pytest tests/integration/test_campaign.py # 6 failed in 2.72s
pytest tests/integration/test_defaults.py # 1 failed in 0.57s
pytest tests/integration/test_bundles.py # 2 failed in 0.30s
# 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 dev env
bash
source ~/.bashrc
lcrc_conda
conda activate test-zppy-pr828
pytest tests/integration/test_images.py 
# 1 failed in 679.90s (0:11:19)
cat test_images_summary.md

Output:

------------------------------------- Captured stdout call --------------------------------------
Preparing weekly cfg tests
Preparing legacy 3.1.0 cfg tests
Preparing legacy 3.0.0 cfg tests
Running 9 tests in parallel
Individual test logs will be written to test_<name>.log files
✓ Completed: legacy_3.1.0_bundles (1 tasks) (log: test_legacy_3.1.0_bundles.log)
✓ Completed: bundles (1 tasks) (log: test_bundles.log)
✓ Completed: legacy_3.1.0_comprehensive_v2 (1 tasks) (log: test_legacy_3.1.0_comprehensive_v2.log)
✓ Completed: comprehensive_v2 (1 tasks) (log: test_comprehensive_v2.log)
✓ Completed: legacy_3.1.0_comprehensive_v3 (1 tasks) (log: test_legacy_3.1.0_comprehensive_v3.log)
✓ Completed: comprehensive_v3 (1 tasks) (log: test_comprehensive_v3.log)
✓ Completed: legacy_3.0.0_comprehensive_v2 (1 tasks) (log: test_legacy_3.0.0_comprehensive_v2.log)
✓ Completed: legacy_3.0.0_bundles (1 tasks) (log: test_legacy_3.0.0_bundles.log)
✓ Completed: legacy_3.0.0_comprehensive_v3 (1 tasks) (log: test_legacy_3.0.0_comprehensive_v3.log)
Copy the output of test_images_summary.md to a Pull Request comment

Test Summary:
Test                                                    Total    Correct     Status
----------------------------------------------------------------------------------
bundles_e3sm_diags                                       1762       1762     ✓ PASS
comprehensive_v2_e3sm_diags                              3806       3794     ✗ FAIL
comprehensive_v3_e3sm_diags                              5369       5369     ✓ PASS
legacy_3.1.0_bundles_e3sm_diags                          1762       1762     ✓ PASS
legacy_3.1.0_comprehensive_v2_e3sm_diags                 3806       3794     ✗ FAIL
legacy_3.1.0_comprehensive_v3_e3sm_diags                 5365       5365     ✓ PASS
legacy_3.0.0_bundles_e3sm_diags                          1762       1762     ✓ PASS
legacy_3.0.0_comprehensive_v2_e3sm_diags                 3806       3794     ✗ FAIL
legacy_3.0.0_comprehensive_v3_e3sm_diags                 5365       5365     ✓ PASS
----------------------------------------------------------------------------------

⚠ Some tests had mismatched or missing images. Check individual log files for details.
==================================== short test summary info ====================================
FAILED tests/integration/test_images.py::test_images - assert 3806 == 3794
================================= 1 failed in 679.90s (0:11:19) =================================

Complete summary table:

Test name Total images Correct images Missing images Mismatched images
bundles_e3sm_diags 1762 1762 0 0
comprehensive_v2_e3sm_diags 3806 3794 12 (list) 0
comprehensive_v3_e3sm_diags 5369 5369 0 0
legacy_3.1.0_bundles_e3sm_diags 1762 1762 0 0
legacy_3.1.0_comprehensive_v2_e3sm_diags 3806 3794 12 (list) 0
legacy_3.1.0_comprehensive_v3_e3sm_diags 5365 5365 0 0
legacy_3.0.0_bundles_e3sm_diags 1762 1762 0 0
legacy_3.0.0_comprehensive_v2_e3sm_diags 3806 3794 12 (list) 0
legacy_3.0.0_comprehensive_v3_e3sm_diags 5365 5365 0 0

Summary table -- failing tests only

e3sm_diags

Test name Total images Correct images Missing images Mismatched images
comprehensive_v2_e3sm_diags 3806 3794 12 (list) 0
legacy_3.1.0_comprehensive_v2_e3sm_diags 3806 3794 12 (list) 0
legacy_3.0.0_comprehensive_v2_e3sm_diags 3806 3794 12 (list) 0

Results Analysis

Missing images from comprehensive_v2_e3sm_diags:

e3sm_diags/atm_monthly_180x360_aave/model_vs_obs_1982-1983/tc_analysis/ace-distribution.png
e3sm_diags/atm_monthly_180x360_aave/model_vs_obs_1982-1983/tc_analysis/aew-density-map.png
e3sm_diags/atm_monthly_180x360_aave/model_vs_obs_1982-1983/tc_analysis/cyclone-density-map.png
e3sm_diags/atm_monthly_180x360_aave/model_vs_obs_1982-1983/tc_analysis/tc-frequency-annual-cycle.png
e3sm_diags/atm_monthly_180x360_aave/model_vs_obs_1982-1983/tc_analysis/tc-frequency.png
e3sm_diags/atm_monthly_180x360_aave/model_vs_obs_1982-1983/tc_analysis/tc-intensity.png
e3sm_diags/atm_monthly_180x360_aave_mvm/model_vs_model_1980-1981/tc_analysis/ace-distribution.png
e3sm_diags/atm_monthly_180x360_aave_mvm/model_vs_model_1980-1981/tc_analysis/aew-density-map.png
e3sm_diags/atm_monthly_180x360_aave_mvm/model_vs_model_1980-1981/tc_analysis/cyclone-density-map.png
e3sm_diags/atm_monthly_180x360_aave_mvm/model_vs_model_1980-1981/tc_analysis/tc-frequency-annual-cycle.png
e3sm_diags/atm_monthly_180x360_aave_mvm/model_vs_model_1980-1981/tc_analysis/tc-frequency.png
e3sm_diags/atm_monthly_180x360_aave_mvm/model_vs_model_1980-1981/tc_analysis/tc-intensity.png

We did run tc_analysis though:

cd /lcrc/group/e3sm/ac.forsyth2/zppy_weekly_comprehensive_v2_output/test_pr828_20260529_try3/v2.LR.historical_0201/post/scripts
grep -v "OK" *status # Good, no errors
tail tc_analysis_1980-1981.o1224994
tail tc_analysis_1982-1983.o1224995
# Doesn't look like there were errors in either job

grep www ~/ez/zppy/tests/integration/generated/test_weekly_comprehensive_v2_chrysalis.cfg
# www = "/lcrc/group/e3sm/public_html/diagnostic_output/ac.forsyth2/zppy_weekly_comprehensive_v2_www/test_pr828_20260529_try3"
cd /lcrc/group/e3sm/public_html/diagnostic_output/ac.forsyth2/zppy_weekly_comprehensive_v2_www/test_pr828_20260529_try3/v2.LR.historical_0201/e3sm_diags
ls -R | grep tc_analysis # No results

v3 won't actually have expected results to compare against, because we have previously turned off tc_analysis (TODO: Add "tc_analysis" back in after empty dat is resolved.). Let's look at v3 as well.

cd /lcrc/group/e3sm/ac.forsyth2/zppy_weekly_comprehensive_v3_output/test_pr828_20260529_try3/v3.LR.historical_0051/post/scripts
grep -v "OK" *status # Good, no errors
tail tc_analysis_1985-1986.o1224973 
tail tc_analysis_1987-1988.o1224974
# Doesn't look like there were errors in either job

grep www ~/ez/zppy/tests/integration/generated/test_weekly_comprehensive_v3_chrysalis.cfg
# www = "/lcrc/group/e3sm/public_html/diagnostic_output/ac.forsyth2/zppy_weekly_comprehensive_v3_www/test_pr828_20260529_try3"
cd /lcrc/group/e3sm/public_html/diagnostic_output/ac.forsyth2/zppy_weekly_comprehensive_v3_www/test_pr828_20260529_try3/v3.LR.historical_0051/e3sm_diags
ls -R | grep tc_analysis # No results

So, for both E3SMv2 and E3SMv3, tc_analysis completes successfully, but the e3sm_diags sets dependent on it fail to produce output on the web server.

Conclusion

This is a regression on E3SMv2. tc_analysis testing for E3SMv3 had previously been turned off, so there's nothing expected to compare to. In both cases, the tc_analysis set of e3sm_diags doesn't produce plots.

@forsyth2

forsyth2 commented Jun 1, 2026

Copy link
Copy Markdown
Collaborator

In both cases, the tc_analysis set of e3sm_diags doesn't produce plots.

@chengzhuzhang From what I can tell, tc_analysis is working, but e3sm_diags needs to be updated to account for these changes. That said, I'd need to do more debugging to know if the fix is in zppy's tc_analysis code, zppy's e3sm_diags code, or the actual e3sm_diags codebase.

@chengzhuzhang

Copy link
Copy Markdown
Collaborator

@forsyth2 I checked the v2 results, It looks like for the v2 test, there are TC tracks, but no AEW tracks detected, and no plots in tc analysis set were created in e3sm_diags. This issue should be resolved in E3SM-Project/e3sm_diags#1059.
For v3, it is likely that neither TC and AEW tracks were detected, no e3sm_diags TC results should be expected.

@chengzhuzhang

Copy link
Copy Markdown
Collaborator

Wait, it does look like for earlier version, there is aew-density-map.png:
e3sm_diags/atm_monthly_180x360_aave/model_vs_obs_1982-1983/tc_analysis/aew-density-map.png
Maybe there is a change in aew detection in this PR that I previously didn't notice.

@forsyth2

forsyth2 commented Jun 1, 2026

Copy link
Copy Markdown
Collaborator

@chengzhuzhang Ok I can re-test using this PR AND E3SM-Project/e3sm_diags#1059, to see if that fixes things. Just let me know once you have that PR ready to test as well.

@chengzhuzhang

chengzhuzhang commented Jun 1, 2026

Copy link
Copy Markdown
Collaborator

@forsyth2 I looked at the test results, and the behavior change is actually expected based on this PR. Also the e3sm_diags's issue is also caused by this behavoir change: before this PR, the script always attempted to generate the AEW histogram after StitchNodes, even if no AEW tracks were found. Now, this PR explicitly adds the warning path in zppy/templates/tc_analysis.bash: if AEW candidate nodes are empty, it prints WARNING: AEW DetectNodes produced no candidate nodes. Skipping AEW StitchNodes and HistogramNodes. and if stitching produces no tracks, it prints WARNING: No AEW tracks found. Skipping AEW histogram.

In previous zppy test for v2, the AEW map actually shows no data.

@forsyth2

forsyth2 commented Jun 1, 2026

Copy link
Copy Markdown
Collaborator

@chengzhuzhang Oh I see, but there's not much point in a test that expects no output. Do we just need to run on a different year set, data set, something else? Or should we just turn off testing the tc_analysis set for now?

@chengzhuzhang

Copy link
Copy Markdown
Collaborator

@chengzhuzhang Oh I see, but there's not much point in a test that expects no output. Do we just need to run on a different year set, data set, something else? Or should we just turn off testing the tc_analysis set for now?

It is fine to keep it as is, at least the test caught the behavior change caused by this PR, and is still useful.

@forsyth2

forsyth2 commented Jun 2, 2026

Copy link
Copy Markdown
Collaborator

It is fine to keep it as is, at least the test caught the behavior change caused by this PR, and is still useful.

@chengzhuzhang That's true, but shouldn't we try to have a test that actually has real output for tc_analysis plots? If it's not feasible to implement (because of data availability or other reasons), I guess we can just leave it as is. In that case, I can change my review to "approved" and merge this PR.

Drop the empty-input guards on the TC and AEW paths so that
cyclones_hist_*.nc and aew_hist_*.nc are produced on every successful
run, matching the pre-PR behavior. Older e3sm_diags versions expect
these files to exist and crash with FileNotFoundError when they are
absent (E3SM-Project/e3sm_diags#1059). HistogramNodes failure checks
are retained so real errors still surface.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@chengzhuzhang

Copy link
Copy Markdown
Collaborator

@forsyth2 thanks for testing and review. I added a commit 7a09d69, the TC and AEW paths now always run StitchNodes and HistogramNodes (pre-PR behavior), keeping only the exit 13 / exit 16 on real HistogramNodes failures. cyclones_hist_.nc and aew_hist_.nc will be produced on every successful run, so older e3sm_diags versions won't crash. This should address the failed test in your zppy run without requiring a fix in e3sm_diags E3SM-Project/e3sm_diags#1059. Could you test again?

@forsyth2

forsyth2 commented Jun 2, 2026

Copy link
Copy Markdown
Collaborator

Thanks @chengzhuzhang I'll get another test started. Should I run the e3sm_diags jobs based off E3SM-Project/e3sm_diags#1059 though or is that still in progress?

@chengzhuzhang

Copy link
Copy Markdown
Collaborator

Thanks @chengzhuzhang I'll get another test started. Should I run the e3sm_diags jobs based off E3SM-Project/e3sm_diags#1059 though or is that still in progress?

The e3sm_diags PR is ready. But I would prefer this zppy test to base off e3sm_diags main branch or the latest e3sm-unified. Thanks!

@forsyth2

forsyth2 commented Jun 2, 2026

Copy link
Copy Markdown
Collaborator

Alright, sounds good!

@forsyth2 forsyth2 mentioned this pull request Jun 2, 2026
7 tasks
@forsyth2

forsyth2 commented Jun 4, 2026

Copy link
Copy Markdown
Collaborator

I've been waiting on Chrysalis nodes all day today, which is unusual. In any case, I just have a few jobs left and once they finish, I'll post test results here.

@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.

Visual inspection

I've done a high level visual inspection. The only thing that stood out to me is an error in the error-number order. That's a simple enough fix; I'll just push a commit for it.

Testing

Re-enable tc_analysis tests: set up #838

Determine which tests to run: already done in earlier test -- e3sm_diags depends on tc_analysis, so let's run the tc_analysis jobs and the e3sm_diags jobs that use it.

Set up environments

We just need to set up the environment for e3sm_diags:

lcrc_conda # Bash function to activate conda.

# 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 
# 5/18: 9d5ebf19 Add targeted image-regression and E3SM Unified CI coverage (#1053)
# 5/12: ff3ef00d Bump v3.2.0 (#1056)
# Good, matches https://github.com/E3SM-Project/e3sm_diags/commits/main
# We can just reuse the existing env then:
conda activate test-e3sm-diags-main-20260526
python -m pip install .
zppy run
# zppy itself #################################################################
cd ~/ez/zppy
git status # Check for uncommitted changes
git fetch upstream zppy-tc
git checkout -b test-pr828-try2-zppy-tc upstream/zppy-tc
git log --oneline | head -n 8
# 7a09d697 tc_analysis: always run StitchNodes and HistogramNodes
# ce784b76 tc_analysis: use default_case for output filename
# 511b0343 tc_analysis: trim trailing whitespace
# d81d36e3 tc_analysis: restore eval ls glob pattern
# c91ff873 tc_analysis: stop errexit after env setup, matching other templates
# 26af251d tc_analysis: rename ts_grid to input_grid; restore res back-compat
# 9b84b5d6 Improve TC analysis workflow robustness
# 17ce9335 Bump to 3.2.0 (#826)

# Rebase off the latest `main` to get the changes from #827:
git fetch upstream main
git rebase upstream/main
# AND cherry-pick #838:
git fetch upstream re-enable-tc-analysis-tests
git cherry-pick 7e8f15a11211c885a77e9164ea84de12f0952417
git log --oneline | head -n 11
# fed83659 Re-enable tc_analysis tests
# 3848d531 tc_analysis: always run StitchNodes and HistogramNodes
# 92d913df tc_analysis: use default_case for output filename
# a1500cf1 tc_analysis: trim trailing whitespace
# 7905ff73 tc_analysis: restore eval ls glob pattern
# a7c92817 tc_analysis: stop errexit after env setup, matching other templates
# 1ad98cd1 tc_analysis: rename ts_grid to input_grid; restore res back-compat
# 4d4e8563 Improve TC analysis workflow robustness
# 7729b56f Enable EAMxx CMORization, E3SM Diags and add ts-level vertical regrid (#827)
# d57b263c Rewrite AGENTS.md in second person (#795)
# 17ce9335 Bump to 3.2.0 (#826)


rm -rf build
conda clean --all --y
conda env create -f conda/dev.yml -n test-zppy-pr828-try2
conda activate test-zppy-pr828-try2
git add -A
pre-commit run --all-files # Good, all pass
git add -A
python -m pip install .

pytest tests/test_*.py # 44 passed in 0.62s

conda list python # Check Python version
# python                    3.14.5

# We previously determined we need to run tc_analysis, and e3sm_diags since it uses tc_analysis.

# 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 /lcrc/soft/climate/e3sm-unified/load_latest_e3sm_unified_chrysalis.sh", # Keep Unified
    "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 /lcrc/soft/climate/e3sm-unified/load_latest_e3sm_unified_chrysalis.sh", # Keep Unified
    "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_pr828_20260602",
}
git diff # Diff looks good
python tests/integration/utils.py
# CFG FILES HAVE BEEN GENERATED FROM TEMPLATES WITH THESE SETTINGS:
# UNIQUE_ID=test_pr828_20260602
# nco_path=
# Reminder: `nco_path=''` => the production-version NCO commands will be used
# e3sm_to_cmip_environment_commands=source /lcrc/soft/climate/e3sm-unified/load_latest_e3sm_unified_chrysalis.sh
# 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 /lcrc/soft/climate/e3sm-unified/load_latest_e3sm_unified_chrysalis.sh
# 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 06/02 15:08 => 127 - header row = 126 jobs
sq | wc -l # Tue 06/02 18:10 => 1 - header row = 0 jobs
Bundles part 2
# Check on bundles status
cd /lcrc/group/e3sm/ac.forsyth2/zppy_weekly_bundles_output/test_pr828_20260602/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_pr828_20260602/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_pr828_20260602/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-pr828-try2-zppy-tc
# Good, correct env: test-zppy-pr828-try2

sq | wc -l # Tue 06/02 18:11 => 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
Review finished runs
### v2  ###
cd /lcrc/group/e3sm/ac.forsyth2/zppy_weekly_comprehensive_v2_output/test_pr828_20260602/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_pr828_20260602/v2.LR.historical_0201/post/scripts
grep -v "OK" *status # Good, no errors

cd /lcrc/group/e3sm/ac.forsyth2/zppy_weekly_legacy_3.1.0_comprehensive_v2_output/test_pr828_20260602/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_pr828_20260602/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_pr828_20260602/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_pr828_20260602/v3.LR.historical_0051/post/scripts
grep -v "OK" *status # Good, no errors

### bundles ###
sq | wc -l # Tue 06/02 18:13 => 4 - header row = 3 jobs

# Picking up 2026-06-03 #######################################################
sq | wc -l # Wed 06/03 09:45 => 4 - header row = 3 jobs
sq
# None of the 3 jobs have gotten nodes yet.

# Picking up 2026-06-04 #######################################################
sq | wc -l # Thu 06/04 08:20 => 1 - header row = 0 jobs

cd /lcrc/group/e3sm/ac.forsyth2/zppy_weekly_bundles_output/test_pr828_20260602/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_pr828_20260602/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_pr828_20260602/v3.LR.historical_0051/post/scripts
grep -v "OK" *status # Good, still no errors

post/scripts directory status: NO ERRORS

Python tests
cd ~/ez/zppy
git status
# Good, correct branch: test-pr828-try2-zppy-tc
# Need to re-establish dev env:
bash # Run bash so we're in an isolated subshell
source ~/.bashrc
lcrc_conda
conda activate test-zppy-pr828-try2
ls tests/integration/test_*.py

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

# Failing, but errors look expected based on the PR (or on #827, which is new on `main`)
pytest tests/integration/test_bash_generation.py # 1 failed in 2.74s
pytest tests/integration/test_campaign.py # 6 failed in 5.01s
pytest tests/integration/test_defaults.py # 1 failed in 1.52s

# Failing simply because we didn't run all jobs on this run:
pytest tests/integration/test_bundles.py # 2 failed in 0.77s

salloc --nodes=1 --partition=debug --time=02:00:00 --account=e3sm
# Re-establish dev env
bash
source ~/.bashrc
lcrc_conda
conda activate test-zppy-pr828-try2
pytest tests/integration/test_images.py 
# 1 passed in 668.02s (0:11:08)
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
comprehensive_v2_e3sm_diags 3806 3806 0 0
comprehensive_v3_e3sm_diags 5369 5369 0 0
legacy_3.1.0_bundles_e3sm_diags 1762 1762 0 0
legacy_3.1.0_comprehensive_v2_e3sm_diags 3806 3806 0 0
legacy_3.1.0_comprehensive_v3_e3sm_diags 5365 5365 0 0
legacy_3.0.0_bundles_e3sm_diags 1762 1762 0 0
legacy_3.0.0_comprehensive_v2_e3sm_diags 3806 3806 0 0
legacy_3.0.0_comprehensive_v3_e3sm_diags 5365 5365 0 0

✅ No non-expected failures in Python tests. No failures in image check tests at all.

Conclusion

After I merge the error-number fix, I think this should be good to merge.

Comment thread zppy/templates/tc_analysis.bash Outdated

@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.

After I merge the error-number fix, I think this should be good to merge.

I've added the fix here: 6cf1439

@zhangshixuan1987 @chengzhuzhang Thanks for working on this. I think this is good to merge.

@chengzhuzhang

Copy link
Copy Markdown
Collaborator

@forsyth2 thanks for confirming with testing.

@chengzhuzhang
chengzhuzhang merged commit 17d5459 into main Jun 4, 2026
7 checks passed
@chengzhuzhang
chengzhuzhang deleted the zppy-tc branch June 4, 2026 20:17
chengzhuzhang added a commit that referenced this pull request Jul 13, 2026
* Add tc_analysis and local e3sm_diags env to EAMxx example

Wire tc_analysis into examples/post.v3.eamxx.cfg using the EAMxx support
from PR #828: a [tc_analysis] section reading the 6-hourly AVERAGE.nhours_x6
stream with input_grid and the EAMxx-ordered tc_vars, plus "tc_analysis"
added to the e3sm_diags sets. Point [e3sm_diags] at a local e3sm_diags dev
build (main, which has EAMxx COSP support) via environment_commands.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* Point EAMxx example env/paths at Perlmutter

Set e3sm_diags environment_commands to the Perlmutter conda hosting
e3sm_diags_eamxx_dev (built from e3sm_diags main, has EAMxx COSP);
keep the Chrysalis path as a comment. Bump output/www folders to
07062026. Validated with a dry run (dry_run=True).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* Rename Perlmutter e3sm_diags env to e3sm_diags_main

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* Use input_component=eamxx for atm_monthly_glb in EAMxx example

glb is a 2D global/region-average, so no EAMxx-specific vertical remap is
exercised; eam vs eamxx produce byte-identical output (verified). Switch to
eamxx for consistency with the other EAMxx-fed subsections.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
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.

6 participants