Skip to content

Conversation

@forsyth2
Copy link
Collaborator

@forsyth2 forsyth2 commented May 22, 2025

Summary

Objectives:

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

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.

@forsyth2 forsyth2 added the semver: new feature New feature (will increment minor version) label May 22, 2025
@forsyth2
Copy link
Collaborator Author

forsyth2 commented May 22, 2025

The first commit here, aee525a, was created with:

# 2025-05-22
git fetch zhangshixuan1987 zppy_pcmdi_new
git checkout -b zppy_pcmdi_202505 zhangshixuan1987/zppy_pcmdi_new
git log
# The latest commit not from this PR is "Test fixes (#646)", from 11/25
# So, this branch is now ~6 months out of date. 
# Very large changes to the zppy codebase have occurred over this time.
git rebase -i 69fe79dd718832f52d7a0fae31c0257b9ee70915
# Usually, we like to keep the commit history for big changes,
# but considering we need to catch up on 6 months of code changes,
# it will be much more practical to rebase the 24 commits into 1.
# Then, we only have to rebase 1 commit.
# So, setting "pick" for the 1st commit "Add PCMDI Diags to zppy"
# and "f" for "fixup" for the remaining 23 commits.
git log
# Now, we just have one commit on top of "Test fixes (#646)"
# So now let's address the conflicts from rebasing off the latest main.
git fetch upstream
git rebase upstream/main
# Luckily, only two files have conflicts
# Just did "Accept Incoming Change" on all conflicts.
git grep -n "<<<<<<<"
# No remaining conflicts
git add zppy/defaults/default.ini
git add zppy/templates/ts.bash
git status 
# All changes are ready to be committed
git rebase --continue
# Now, we have one commit on top of "Update to latest conda-incubator/setup-miniconda (#715)", from 5/21.
# So, we're up to date now!

@forsyth2
Copy link
Collaborator Author

Second commit, f769b93, created by iteratively fixing errors brought up by running pre-commit run --all-files. Many of these errors were because of function renamings during parameter inference refactoring (#682)

@forsyth2
Copy link
Collaborator Author

3rd commit, dff9099, cleans up the file space.

@forsyth2 forsyth2 mentioned this pull request May 22, 2025
15 tasks
@forsyth2 forsyth2 force-pushed the zppy_pcmdi_202505 branch from d527e31 to cab9aa2 Compare May 22, 2025 19:23
@forsyth2
Copy link
Collaborator Author

4th commit, cab9aa2, moves the ts task changes to the e3sm_to_cmip task.

@forsyth2
Copy link
Collaborator Author

5th commit, 0e2ef3d removes unused parameters.

6th commit, a0c9c5f, puts pcmdi_diags task at bottom of default.ini, after ilamb, since that's the order it appears in __main__.py.

Copy link
Collaborator Author

@forsyth2 forsyth2 left a comment

Choose a reason for hiding this comment

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

I've done a high-level code read-through, leaving some comments.

I'd also like to:

  1. Run a sample cfg producing PCMDI plots
  2. Run the integration tests to make sure the other tasks haven't been broken by these changes.

cmip_vars = string(default="")
# Model component having generated input files (eam, eamxx, elm, mosart, ...)
input_component = string(default="")
# TBD: description here
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Add a description of what interp_vars does

[pcmdi_diags]
# Title of zppy-pcmdi diagnostics
pcmdi_webtitle = string(default="E3SM-PMP Diagnostics")
# Version of zppy-pcmdi code
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Should this just be "Version of pcmdi code"?

# The years increment for test model data
ts_num_years = integer(default=5)
##########################################################################################
# below followed the setup in e3sm_diag but used for PCMDI workflow
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

"below follows the setup in e3sm_diag but using for PCMDI workflow"

Comment on lines +434 to +439
# See url<need to work on document later>
multiprocessing = boolean(default=True)
# See url<need to work on document later>
num_workers = integer(default=24)
# See url<need to work on document later>
figure_format = string(default="png")
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

What document?

Comment on lines +46 to +50
# procedure type for e3sm_to_cmip
c["cmor_tables_prefix"] = c["diagnostics_base_path"]

# check and set parameter for pcmdi
c["pcmdi_external_prefix"] = c["diagnostics_base_path"]
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

For these parameters defined internally (in the .py file), I usually like to include them as comments in the default.ini, e.g.:

# machine -- NOTE: this parameter is created internally

Comment on lines +51 to +62
{% if input_files.split(".")[0] == 'clm2' or input_files.split(".")[0] == 'elm' -%}
--var-list \
'snd, mrsos, mrso, mrfso, mrros, mrro, prveg, evspsblveg, evspsblsoi, tran, tsl, lai, cLitter, cProduct, cSoilFast, cSoilMedium, cSoilSlow, fFire, fHarvest, cVeg, nbp, gpp, ra, rh' \
--realm \
lnd \
{% endif -%}
{% if input_files.split(".")[0] == 'cam' or input_files.split(".")[0] == 'eam' -%}
--var-list \
{{ cmip_vars }} \
'ua, va, ta, wa, zg, hur, pr, prc, prsn, ts, tas, prw, psl, sfcWind, tasmax, tasmin, tauu, tauv, rtmt, rsdt, rsds, rsdscs, rlds, rldscs, rsus, rsuscs, rsut, rsutcs, rlus, rlut, rlutcs, clivi, clwvi, clt, evspsbl, hfls, hfss, huss' \
--realm \
{{ component }} \
atm \
{% endif -%}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We've gotten rid of {{ cmip_vars }} and {{ component }} here, opting for hard-coded choices, so I imagine that will break other use cases of e3sm_to_cmip. Currently default.ini has cmip_vars = string(default=""), so we could add these as a default. That said, I see there are two defaults based on the if-statements, so that will be more difficult to handle.

@@ -0,0 +1,1308 @@
#!/bin/bash
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This file is 1,308 lines long. Ideally the .bash files are extremely simple -- just some boilerplate around a call to an external package.

Otherwise, scope creep is introduced into zppy -- that is, zppy is a workflow coordinator and so we should not be putting analysis code inside it. That code can be moved into zppy-interfaces if it's not enough code to warrant its own package.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Currently, the only analysis code in zppy-interfaces is the global-time-series codebase. Most of that is contained in https://github.com/E3SM-Project/zppy-interfaces/tree/main/zppy_interfaces/global_time_series, but it's also important to note that it has its own distinct "entry point". In https://github.com/E3SM-Project/zppy-interfaces/blob/main/pyproject.toml#L118, we have:

[project.scripts]
zi-global-time-series = "zppy_interfaces.global_time_series.__main__:main

That is, when someone (notably the zppy-generated global_time_series.bash calls zi-global-time-series, it's calling zppy_interfaces.global.time_series.__main__.py's main function.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I will say that e3sm_diags.bash is quite a long file. However, many of the lines are just for filling out the template of e3sm.py. We can analyze the line count for that: (451-157)/537 = 294/537 = 55% of that file is the just filling out that template, with the remaining 243 lines helping to set it up.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

And here, we actually fill out three templates:

link_observation.py: lines 428-471
parameterfile.py: lines 508-768
pcmdi.py: lines 777-1224

So, all together, that's [(472-428) + (769-508) + (1225-777)]/1308 = [44+261+448]/1308 = 753/1308 = 57.6% of the file is just filling out these templates. So that's actually pretty close to e3sm_diags.bash.

But why are 3 files being created? That feels like it's entering into Scope Creep.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The knowledge of these 3 files adds context to this block in default.ini:

# File of specified regions for mean climate calculation
regions_specs = string(default="pcmdi_data/region/regions_specs.json")
# File of observation data name for mean climate calculation
reference_alias = string(default="pcmdi_data/reference/reference_alias.json")
# Utility file with functions for zppy-pcmdi data processing
pcmdi_zppy_util = string(default="pcmdi_data/utility/pcmdi_zppy_util.py")
# Utility file with functions for zppy-pcmdi viewer page processing
pcmdi_viewer_util = string(default="pcmdi_data/utility/pcmdi_viewer_util.py")
# Template files for zppy-pcmdi viewer page processing
pcmdi_viewer_template = string(default="pcmdi_data/viewer")
# File of variable list to generate synthetic metrics plot
synthetic_metrics = string(default="pcmdi_data/synthetic_metrics/synthetic_metrics_list.json")

As an example, let's follow pcmdi_zppy_util. In zppy/templates/pcmdi_diags.bash, we have:

cp -r '{{pcmdi_external_prefix}}/{{pcmdi_zppy_util}}' .

In zppy/pcmdi_diags.py, we defined:

c["pcmdi_external_prefix"] = c["diagnostics_base_path"]

So, on Chrysalis, according to https://github.com/E3SM-Project/mache/blob/main/mache/machines/chrysalis.cfg#L28, that's going to be:

base_path = /lcrc/group/e3sm/diagnostics

So, we're expecting /lcrc/group/e3sm/diagnostics/pcmdi_data/utility/pcmdi_zppy_util.py to exist.

I now see two important issues that we must address:

  1. We can't be relying on .py files in an arbitrary directory. First, they might not get synced across machines -- for example, sure this exists on Chrysalis, but is it there on Perlmutter, Compy, every other machine? Second, there's no way to track updates to them; here's no git version history.
  2. This file in particular I can see is doing plotting, like global_time_series.py. That's outside the scope of zppy. We should follow the example of global_time_series.py and move these analysis scripts into a directory of zppy-interfaces and set them up with a new entry point.

Comment on lines 428 to 471
cat > link_observation.py << EOF
import os
import re
import glob
import json
try:
from pcmdi_zppy_util import ObservationLinker
except ImportError as e:
raise ImportError("Module 'pcmdi_zppy_util' not found. Make sure it's installed and accessible.") from e
# Inputs populated via templating
MODEL_NAME = '${model_name_ref}.${tableID_ref}'
VARIABLES = '{{ vars }}'.split(",")
OBS_SETS = '{{ obs_sets }}'.split(",")
OBS_TS_DIR = '{{ obs_ts }}'
OBS_TMP_DIR = '${obstmp_dir}'
OBS_ALIAS_FILE = "reference_alias.json"
# Mapping from observational variable names to CMIP-standard
ALT_OBS_MAP = {
"pr": "PRECT",
"sst": "ts",
"sfcWind": "si10",
"taux": "tauu",
"tauy": "tauv",
"rltcre": "toa_cre_lw_mon",
"rstcre": "toa_cre_sw_mon",
"rtmt": "toa_net_all_mon"
}
linker = ObservationLinker(
model_name=MODEL_NAME,
variables=VARIABLES,
obs_sets=OBS_SETS,
ts_dir_ref_source=OBS_TS_DIR,
obstmp_dir=OBS_TMP_DIR,
altobs_dic=ALT_OBS_MAP,
obs_alias_file=OBS_ALIAS_FILE
)
linker.link_obs_data()
linker.process_derived_variables()
EOF
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Note to self -- this block fills out link_observation.py.

Comment on lines 508 to 785
cat > parameterfile.py << EOF
import os
import sys
import json
#####################
# Basic Information
#####################
start_yr = int('${Y1}')
end_yr = int('${Y2}')
num_years = end_yr - start_yr + 1
period = f"{start_yr:04d}01-{end_yr:04d}12"
model_parts = '${model_name}'.split('.')
mip, exp, product, realm = model_parts[:4]
##############################################
# Configuration Shared with PCMDI Diagnostics
##############################################
# Whether to generate NetCDF outputs for observations and model results
nc_out_obs = {{ mov_nc_out_obs }}
nc_out_model = {{ mov_nc_out_model }}
# Output file extension: use .nc if either output is enabled,
# otherwise default to .xml
ext = ".nc" if nc_out_model or nc_out_obs else ".xml"
# User annotation and debug flag
user_notes = 'Provenance and results'
debug = {{ pcmdi_debug }}
# Enable plot generation for model and observation
plot_model = {{ mov_plot_model }}
plot_obs = {{ mov_plot_obs }} # optional
# Execution mode and output format
run_type = '{{ run_type }}'
figure_format = '{{ figure_format }}'
# Save interpolated model climatologies?
save_test_clims = {{ save_test_clims }}
# Save all metrics results in a single file?
# Set to 'n' as metrics are computed per variable
metrics_in_single_file = 'n'
# Custom values for land/sea masking
regions_values = {
"land": 100.0,
"ocean": 0.0
}
# Template path for land/sea mask file (fixed input)
modpath_lf = os.path.join(
'fixed',
'sftlf.%(model).nc'
)
{% if "mean_climate" in subsection %}
############################################
# Setup Specific for Mean Climate Metrics
############################################
modver = "${case_id}"
parallel = False
generate_sftlf = False
sftlf_filename_template = modpath_lf
# Target grid: can be '2.5x2.5' or a CDMS2 grid object string
target_grid = '{{ target_grid }}'
targetGrid = target_grid # for backward compatibility
target_grid_string = '{{ target_grid_string }}'
# Regridding tool and method (general use)
# OPTIONS: 'regrid2' or 'esmf'
regrid_tool = '{{ regrid_tool }}'
# OPTIONS: 'linear' or 'conservative' (only for 'esmf')
regrid_method = '{{ regrid_method }}'
# Regridding tool and method for ocean diagnostics
regrid_tool_ocn = '{{ regrid_tool_ocn }}' # 'regrid2' or 'esmf'
regrid_method_ocn = ('{{ regrid_method_ocn }}') # 'linear' or 'conservative'
# Model realization(s) to consider
realization = "*"
# Model product name from input
test_data_set = [product]
# Path to model climatology files
test_data_path = '${climo_dir_primary}'
# Template for model climatology filenames
filename_template = '.'.join([
mip,
exp,
'%(model)',
'%(realization)',
'${tableID}',
'%(variable)',
period,
'AC',
'${case_id}',
'nc'
])
# Path to reference climatology files
reference_data_path = '${climo_dir_ref}'
# Observation catalogue file (dynamic by subsection)
custom_observations = os.path.join(
'pcmdi_diags',
'{}_{}_catalogue.json'.format(
'${climo_dir_ref}',
'{{subsection}}'
)
)
# Load variable-specific region definitions
regions = json.load(open('regions.json'))
# Load predefined region specifications and normalize domain lat/lon as tuples
regions_specs = json.load(open('regions_specs.json'))
for key in regions_specs:
domain = regions_specs[key].get('domain', {})
if 'latitude' in domain:
domain['latitude'] = tuple(domain['latitude'])
regions_specs[key]['domain']['latitude'] = domain['latitude']
if 'longitude' in domain:
domain['longitude'] = tuple(domain['longitude'])
regions_specs[key]['domain']['longitude'] = domain['longitude']
# METRICS OUTPUT
metrics_output_path = os.path.join(
'pcmdi_diags',
'metrics_results',
'mean_climate',
mip,
exp,
'%(case_id)'
)
#INTERPOLATED MODELS' CLIMATOLOGIES
diagnostics_output_path = os.path.join(
'pcmdi_diags',
'diagnostic_results',
'mean_climate',
mip,
exp,
'%(case_id)'
)
test_clims_interpolated_output = diagnostics_output_path
{% endif %}
{% if "variability_modes" in subsection %}
# Setup for Mode Variability Diagnostics
msyear = int(start_yr)
meyear = int(end_yr)
# Seasons to analyze (comma-separated string to list)
seasons = '{{ seasons }}'.split(",")
# Data frequency (e.g., monthly, seasonal)
frequency = '{{ frequency }}'
# Variables to analyze (comma-separated string or space-separated)
varModel = '{{ vars }}'
# Unit conversion flags for model and observations
ModUnitsAdjust = {{ ModUnitsAdjust }}
ObsUnitsAdjust = {{ ObsUnitsAdjust }}
# Mask out land regions (consider ocean-only if True)
landmask = {{ landmask }}
# If True, remove domain mean from each time step
RmDomainMean = {{ RmDomainMean }}
# If True, normalize EOFs to unit variance
EofScaling = {{ EofScaling }}
# Conduct Combined EOF/CBF analysis (if True)
CBF = {{ CBF }}
# Conduct Conventional EOF analysis (if True)
ConvEOF = {{ ConvEOF }}
# Skip CMEC output (hardcoded for now)
cmec = False
# Whether to overwrite existing diagnostic output
update_json = False
# Template for model input file paths
modnames = [product]
realization = "*"
modpath = os.path.join(
'${ts_dir_primary}',
'{}.{}.%(model).%(realization).{}.%(variable).{}.nc'.format(
mip, exp, '${tableID}', period
)
)
# Output results directory
results_dir = os.path.join(
'pcmdi_diags',
'%(output_type)',
'variability_modes',
'%(mip)',
'%(exp)',
'${case_id}',
'%(variability_mode)',
'%(reference_data_name)',
)
{% endif %}
{% if "enso" in subsection %}
# Parameter Setup for ENSO Metrics
modnames = [product]
realization = realm
modpath = os.path.join(
'${ts_dir_primary}',
'{}.{}.%(model).%(realization).{}.%(variable).{}.nc'.format(
mip, exp, '${tableID}', period
)
)
# Observation/Reference settings
obs_cmor = True
obs_cmor_path = '${ts_dir_ref}'
obs_catalogue = 'obs_catalogue.json'
# Land/Sea mask for reference data
reference_data_lf_path = json.load(open('obs_landmask.json'))
# Metrics collection type (e.g., ENSO_perf, ENSO_tel, ENSO_proc)
# Defined externally via metricsCollection
# Output directory structure
results_dir = os.path.join(
'pcmdi_diags',
'%(output_type)',
'enso_metric',
'%(mip)',
'%(exp)',
'${case_id}',
'%(metricsCollection)',
)
# Output filenames for JSON and NetCDF
json_name = "%(mip)_%(exp)_%(metricsCollection)_${case_id}_%(model)_%(realization)"
netcdf_name = json_name
{% endif %}
EOF
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Note to self -- this block fills out parameterfile.py

Comment on lines 777 to 1224
cat > pcmdi.py << EOF
import os
import glob
import re
import json
import time
import datetime
import xcdat as xc
import numpy as np
import pandas as pd
import shutil
import collections
from collections import OrderedDict
import psutil
import subprocess
from itertools import chain
from subprocess import Popen, PIPE, call
import pcmdi_metrics
from pcmdi_metrics.graphics import (
normalize_by_median,
)
from pcmdi_zppy_util import(
count_child_processes,
run_serial_jobs,
run_parallel_jobs,
derive_missing_variable,
save_variable_regions,
generate_mean_clim_cmds,
generate_varmode_cmds,
build_enso_obsvar_catalog,
build_enso_obsvar_landmask,
generate_enso_cmds,
shift_row_to_bottom,
check_badvals,
archive_data,
drop_vars,
enso_plot_driver,
variability_modes_plot_driver,
mean_climate_plot_driver,
parcoord_metric_plot,
portrait_metric_plot,
ObservationLinker,
DataCatalogueBuilder,
LandSeaMaskGenerator,
ClimMetricsReader,
ClimMetricsMerger,
MeanClimateMetricsCollector,
VariabilityMetricsCollector,
EnsoDiagnosticsCollector,
SyntheticMetricsPlotter
)
from pcmdi_viewer_util import(
collect_config,
setup_jinja_env,
create_section,
add_section,
generate_methodology_html,
generate_data_html,
generate_viewer_html
)
# Determine multiprocessing usage
num_workers = {{ num_workers }}
multiprocessing = {{ multiprocessing }} if num_workers >= 2 else False
{% if "synthetic_plots" not in subsection %}
# Time range
start_yr = int('${Y1}')
end_yr = int('${Y2}')
num_years = end_yr - start_yr + 1
# Set data paths based on diagnostic type
{% if "mean_climate" in subsection %}
test_data_path = '${climo_dir_primary}'
reference_data_path = '${climo_dir_ref}'
{% elif "variability_modes" in subsection or "enso" in subsection %}
test_data_path = '${ts_dir_primary}'
reference_data_path = '${ts_dir_ref}'
{% endif %}
# Dataset identifiers
test_data_set = ['${model_name}'.split(".")[1]]
{% if run_type == "model_vs_obs" %}
reference_data_set = '{{ obs_sets }}'.split(",")
{% elif run_type == "model_vs_model" %}
reference_data_set = ['${model_name_ref}'.split(".")[1]]
{% endif %}
variables = '{{ vars }}'.split(",")
###############################################################
#check and process derived quantities, these quantities are
#likely not included as default in e3sm_to_cmip module
###############################################################
for var in variables:
varin = re.split(r"[_-]", var)[0] if "_" in var or "-" in var else var
test_fpaths = sorted(glob.glob(os.path.join(test_data_path, f"*.{var}.*.nc")))
if not test_fpaths:
derive_missing_variable(varin, test_data_path, '${model_name}.${tableID}')
{% if run_type == "model_vs_model" %}
ref_fpaths = sorted(glob.glob(os.path.join(reference_data_path, f"*.{var}.*.nc")))
if not ref_fpaths:
derive_missing_variable(varin, reference_data_path, '${model_name_ref}.${tableID_ref}')
{% endif %}
#######################################################
#collect and document data info in a dictionary
# for convenience of pcmdi processing
#######################################################
builder = DataCatalogueBuilder(
test_data_path, test_data_set,
reference_data_path, reference_data_set,
variables, '{{subsection}}', 'pcmdi_diags'
)
test_dic, obs_dic = builder.build_catalogues()
##########################################################
# land/sea mask is needed in PCMDI diagnostics, check and
# generate it here as these data are not always available
# for model or observations
##########################################################
# Whether to generate the land/sea mask
generate_flag = {{ generate_sftlf }}
# Instantiate and run
mask_generator = LandSeaMaskGenerator(
test_path=test_data_path,
ref_path=reference_data_path,
subsection='{{subsection}}',
fixed_dir='fixed'
)
mask_generator.run(generate_flag)
# Diagnostic input file templates
input_template = os.path.join(
'pcmdi_diags',
'%(output_type)',
'%(metric_type)',
'${model_name}'.split(".")[0],
'${model_name}'.split(".")[1],
'${case_id}'
)
# Diagnostic output path templates
out_path = os.path.join('${results_dir}', '%(group_type)')
{% endif %}
{% if "mean_climate" in subsection %}
regions = '{{regions}}'.split(",")
#assiagn region to each variable
save_variable_regions(variables, regions)
# generate the command list
lstcmd = generate_mean_clim_cmds(
variables=variables,
obs_dic=obs_dic,
case_id='${case_id}'
)
####################################################
# call pcmdi mean climate diagnostics
####################################################
if (len(lstcmd) > 0 ) and multiprocessing:
try:
results = run_parallel_jobs(lstcmd, num_workers)
for i, (stdout, stderr, return_code) in enumerate(results):
print(f"\nCommand {i+1} finished:")
print(f"STDOUT: {stdout}")
print(f"STDERR: {stderr}")
print(f"Return code: {return_code}")
except RuntimeError as e:
print(f"Execution failed: {e}")
elif (len(lstcmd) > 0 ):
try:
results = run_serial_jobs(lstcmd)
for i, (stdout, stderr, return_code) in enumerate(results):
print(f"\nCommand {i+1} finished:")
print(f"STDOUT: {stdout}")
print(f"STDERR: {stderr}")
print(f"Return code: {return_code}")
except RuntimeError as e:
print(f"Execution failed: {e}")
else:
print("no jobs to run,continue....")
print("successfully finish all jobs....")
#time delay to ensure process completely finished
time.sleep(5)
#orgnize diagnostic output
collector = MeanClimateMetricsCollector(
regions=regions,
variables=variables,
fig_format='{{figure_format}}',
model_info=tuple('${model_name}'.split(".")), # (mip, exp, model, relm)
case_id='${case_id}',
input_template=input_template,
output_dir=out_path
)
collector.collect()
{% endif %}
{% if "variability_modes" in subsection %}
##########################################
# call pcmdi mode variability diagnostics
##########################################
print("calculate mode variability metrics")
{% if subsection == "variability_modes_atm" %}
var_modes = '{{ atm_modes }}'.split(",")
{% elif subsection == "variability_modes_cpl" %}
var_modes = '{{ cpl_modes }}'.split(",")
{% endif %}
#from configuration file
varOBS = '{{vars}}'
refset = obs_dic[varOBS]['set']
refname = obs_dic[varOBS][refset]
refpath = obs_dic[varOBS][refname]['file_path']
reftyrs = int(str(obs_dic[varOBS][refname]['yymms'])[0:4])
reftyre = int(str(obs_dic[varOBS][refname]['yymme'])[0:4])
# Call the function
lstcmd = generate_varmode_cmds(
modes=var_modes,
varOBS=varOBS,
reftyrs=reftyrs,
reftyre=reftyre,
refname=refname,
refpath=refpath,
case_id='${case_id}'
)
if (len(lstcmd) > 0 ) and multiprocessing:
try:
results = run_parallel_jobs(lstcmd, num_workers)
for i, (stdout, stderr, return_code) in enumerate(results):
print(f"\nCommand {i+1} finished:")
print(f"STDOUT: {stdout}")
print(f"STDERR: {stderr}")
print(f"Return code: {return_code}")
except RuntimeError as e:
print(f"Execution failed: {e}")
elif (len(lstcmd) > 0 ):
try:
results = run_serial_jobs(lstcmd)
for i, (stdout, stderr, return_code) in enumerate(results):
print(f"\nCommand {i+1} finished:")
print(f"STDOUT: {stdout}")
print(f"STDERR: {stderr}")
print(f"Return code: {return_code}")
except RuntimeError as e:
print(f"Execution failed: {e}")
else:
print("no jobs to run,continue...")
print("successfully finish all jobs....")
#time delay to ensure process completely finished
time.sleep(5)
# Create the collector instance
collector = VariabilityMetricsCollector(
modes=var_modes,
fig_format='{{figure_format}}',
mip='${model_name}'.split(".")[0],
exp='${model_name}'.split(".")[1],
model='${model_name}'.split(".")[2],
relm='${model_name}'.split(".")[3],
case_id='${case_id}',
input_dir=input_template,
output_dir=out_path
)
# Run the collection process
collector.collect()
{% endif %}
{% if "enso" in subsection %}
#############################################
# call enso_driver.py to process diagnostics
#############################################
build_enso_obsvar_catalog(obs_dic, variables)
build_enso_obsvar_landmask(obs_dic, variables)
#now start enso driver
lstcmd = generate_enso_cmds('{{ enso_groups }}', '${case_id}')
if (len(lstcmd) > 0 ) and multiprocessing:
try:
results = run_parallel_jobs(lstcmd, num_workers)
for i, (stdout, stderr, return_code) in enumerate(results):
print(f"\nCommand {i+1} finished:")
print(f"STDOUT: {stdout}")
print(f"STDERR: {stderr}")
print(f"Return code: {return_code}")
except RuntimeError as e:
print(f"Execution failed: {e}")
elif (len(lstcmd) > 0 ) and not multiprocessing:
try:
results = run_serial_jobs(lstcmd)
for i, (stdout, stderr, return_code) in enumerate(results):
print(f"\nCommand {i+1} finished:")
print(f"STDOUT: {stdout}")
print(f"STDERR: {stderr}")
print(f"Return code: {return_code}")
except RuntimeError as e:
print(f"Execution failed: {e}")
else:
print("no jobs to run...")
print("successfully finish all jobs....")
#time delay to ensure process completely finished
time.sleep(5)
# Initialize and run collector
obs_dict = json.load(open('obs_catalogue.json'))
obs_name = list(obs_dict.keys())[0]
collector = EnsoDiagnosticsCollector(
fig_format='{{figure_format}}',
refname=obs_name,
model_name_parts='${model_name}'.split("."),
case_id='${case_id}',
input_dir=input_template,
output_dir=out_path
)
enso_groups = '{{ enso_groups }}'.split(",")
collector.run(enso_groups)
{% endif %}
{% if "synthetic_plots" in subsection %}
#########################################
#plot synthetic figures for pcmdi metrics
#########################################
print("generate synthetic metrics plot ...")
metric_sets = '{{sub_sets}}'.split(",")
figure_sets = '{{synthetic_sets}}'.split(",")
figure_format = '{{figure_format}}'
test_input_path = os.path.join(
'${www}',
'%(model_name)',
'pcmdi_diags',
'${results_dir}',
'metrics_data',
'%(group_type)'
)
metric_dict = json.load(open('synthetic_metrics_list.json'))
plotter = SyntheticMetricsPlotter(
case_name='{{case}}',
test_name='{{model_name}}',
table_id='{{model_tableID}}',
figure_format=figure_format,
figure_sets=figure_sets,
metric_dict=metric_dict,
save_data=True,
base_test_input_path=test_input_path,
results_dir='${web_dir}/${results_dir}',
cmip_clim_dir='{{cmip_clim_dir}}',
cmip_clim_set='{{cmip_clim_set}}',
cmip_movs_dir='{{cmip_movs_dir}}',
cmip_movs_set='{{cmip_movs_set}}',
atm_modes='{{ atm_modes }}',
cpl_modes='{{ cpl_modes }}',
cmip_enso_dir='{{cmip_enso_dir}}',
cmip_enso_set='{{cmip_enso_set}}'
)
# Generate Summary Metrics plots
# e.g., "climatology,enso,variability"
groups = '{{sub_sets}}'.split(',')
plotter.generate(groups)
print("Generating viewer page for diagnostics...")
# Extract template values (assumes substitution happens before execution)
title = "{{pcmdi_webtitle}}"
version = "{{pcmdi_version}}"
subtitle = "${run_type}".replace('_', ' ').capitalize()
case_id = "${case}"
model_name = "{{model_name}}"
table_id = "{{model_tableID}}"
# ts_years is assumed to be a list via string_list(default=list(""))
ts_years = {{ts_years}}
ts_periods = ts_years if isinstance(ts_years, list) else []
# Validate and unpack periods
if len(ts_periods) == 3:
clim_period, emov_period, enso_period = [p.strip() for p in ts_periods]
else:
raise ValueError(
f"Expected 3 periods (climatology, EMoV, ENSO), "
f"but got {len(ts_periods)}: {ts_periods}"
)
# Set up paths
obs_dir = os.path.join('{{pcmdi_external_prefix}}', 'observations', 'Atm', 'time-series')
pmp_dir = os.path.join('{{pcmdi_external_prefix}}', 'pcmdi_data')
out_dir = os.path.join("${web_dir}", "${results_dir}", "viewer")
os.makedirs(out_dir, exist_ok=True)
# Copy logo
web_logo_src = os.path.join(
'{{pcmdi_external_prefix}}',
'{{pcmdi_viewer_template}}',
'e3sm_pmp_logo.png'
)
web_logo_dst = os.path.join(out_dir, 'e3sm_pmp_logo.png')
shutil.copy(web_logo_src, web_logo_dst)
# Build config
config = collect_config(
title=title,
subtitle=subtitle,
version=version,
case_id=case_id,
diag_dir="${web_dir}",
obs_dir=obs_dir,
pmp_dir=pmp_dir,
out_dir=out_dir,
clim_period=clim_period,
emov_period=emov_period,
enso_period=enso_period
)
# Render viewer
generate_methodology_html(config)
generate_data_html(config)
generate_viewer_html(config)
{% endif %}
EOF
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Note to self -- this block fills out pcmdi.py

@chengzhuzhang
Copy link
Collaborator

chengzhuzhang commented May 29, 2025

@zhangshixuan1987 thank you for the great effort to add this new capability. @forsyth2 thanks for the initial review.
I think one big challenge for integrating PMP is that the package is still developing an end-to-end workflow. The original design was focused on flexibility (lots of parameter options) rather than automation. To enable the end-to-end workflow, we need lots of additional code to produce the results we desire. I think we should prioritize having clean and modular code but can still leave them in zppy because they are part of PMP analysis task.

Some minor point, we need the parameter to set custom environment variable for PMP version, streamlined integration for vertical remapping, and update this branch based on latest zppy code on main, which Ryan is already working on.

@forsyth2
Copy link
Collaborator Author

Met with @zhangshixuan1987 & @chengzhuzhang. My action items:

  • Run per instructions at Zppy pcmdi new #656 (comment) to test code. Those instructions were for code as of Zppy pcmdi new #656, so check if it works with updates in this PR.
  • Try to pull the scripts out of the pcmdi_data directory and into either zppy-interfaces or maybe just the zppy/inclusions directory.
  • Consider design decision of moving code to zppy-interfaces. If we pulled code into zppy-interfaces, we might need to run zi-pmp, then pmp itself, and then zi-pmp again. That may or may not be acceptable. (Note: global-time-series assumes all input [output from previous tasks] has been created, but this script generates input as we go. So that is another complication.)

@forsyth2
Copy link
Collaborator Author

6c6818e adds the example cfg listed in #656 (comment)

@forsyth2
Copy link
Collaborator Author

5003ca0 adds some initial edits to the example cfg. I'm still not able to run it, because the new task requires both the Unified environment for the nc-file operators and an environment with the PCMDI code. I think it will be easier to reason about the environments post-refactor (moving some of the code to zppy-interfaces), so I'm going to move onto prototyping that work.

@forsyth2 forsyth2 mentioned this pull request May 30, 2025
14 tasks
@forsyth2
Copy link
Collaborator Author

ca63656 passes the parameters to zppy-interfaces (using the new code in E3SM-Project/zppy-interfaces#25).


[[ atm_monthly_180x360_aave ]]
# list of variables to be interpolated to pressure level; see e3sm_to_cmip/vrt_remap_plev19.nc
cmip_plevdata = "inclusions/e3sm_to_cmip/vrt_remap_plev19.nc"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This file doesn't exist in zppy's zppy/templates/inclusions/e3sm_to_cmip path. Is it inclusions elsewhere?

@forsyth2
Copy link
Collaborator Author

82d4888 iterates on the example cfg, but with limited success

@forsyth2
Copy link
Collaborator Author

@zhangshixuan1987 @chengzhuzhang I refactored the code to use zppy-interfaces -- see E3SM-Project/zppy-interfaces#25.

I'm trying to get the example cfg to work, but so far I haven't had much luck there. My latest attempt:

> cd /lcrc/group/e3sm/ac.forsyth2/zppy_pr719_output/unique_id_12/v3.LR.amip_0101/post/scripts

> grep -v "OK" *status
pcmdi_diags_enso_model_vs_obs_2005-2015.status:ERROR (11)
pcmdi_diags_mean_climate_model_vs_obs_2005-2014.status:ERROR (11)
pcmdi_diags_variability_modes_atm_model_vs_obs_2005-2014.status:ERROR (11)
pcmdi_diags_variability_modes_cpl_model_vs_obs_2005-2014.status:ERROR (11)

# Final errors in the `.o` files:
# KeyError: "Variable key 'psl' not found in observation dictionary."
# ValueError: not enough values to unpack (expected 4, got 1)
# KeyError: 'psl'
# KeyError: 'ts'

I imagine it's just a matter of making sure all the file paths are chained correctly.

My remaining action items:

  • Get the example cfg to actually work (on all 4 subtasks), with shorter test year periods
  • Run example cfg on original full year set
  • Run integration tests to make sure other tasks don't break
  • Clean up parameters (e.g., make sure we're only introducing parameters we really need)
  • Add unit/integration tests for this PR

Comment on lines +574 to +642
# Path to reference climatology files
reference_data_path = '${climo_dir_ref}'
# Observation catalogue file (dynamic by subsection)
custom_observations = os.path.join(
'pcmdi_diags',
'{}_{}_catalogue.json'.format(
'${climo_dir_ref}',
'{{subsection}}'
)
)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

> cd /lcrc/group/e3sm/ac.forsyth2/zppy_pr719_output/unique_id_12/v3.LR.amip_0101/post/scripts
> emacs pcmdi_diags_enso_model_vs_obs_2005-2015.o753616
# Warning: Could not extract dates from *.nc
# Warning: Catalogue not found at pcmdi_diags/ts_enso_catalogue.json
# Warning: Catalogue not found at pcmdi_diags/ts_ref_enso_catalogue.json
# KeyError: "Variable key 'psl' not found in observation dictionary."

> emacs pcmdi_diags_mean_climate_model_vs_obs_2005-2014.o753613 
# Warning: Could not extract dates from *.nc
# Warning: Catalogue not found at pcmdi_diags/climo_mean_climate_catalogue.json
# Warning: Catalogue not found at pcmdi_diags/climo_ref_mean_climate_catalogue.json
# ValueError: not enough values to unpack (expected 4, got 1)

> emacs pcmdi_diags_variability_modes_atm_model_vs_obs_2005-2014.o753615 
# Warning: Could not extract dates from *.nc
# Warning: Catalogue not found at pcmdi_diags/ts_variability_modes_atm_catalogue.json
# Warning: Catalogue not found at pcmdi_diags/ts_ref_variability_modes_atm_catalogue.json
# KeyError: 'psl'

> emacs pcmdi_diags_variability_modes_cpl_model_vs_obs_2005-2014.o753614
# Warning: Could not extract dates from *.nc
# Warning: Catalogue not found at pcmdi_diags/ts_variability_modes_cpl_catalogue.json
# Warning: Catalogue not found at pcmdi_diags/ts_ref_variability_modes_cpl_catalogue.json
# KeyError: 'ts'

YYYYS="${BASH_REMATCH[1]}"
YYYYE="${BASH_REMATCH[2]}"
else
echo "Warning: Could not extract dates from ${fname}"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Copy of https://github.com/E3SM-Project/zppy/pull/719/files#r2141246726:

> cd /lcrc/group/e3sm/ac.forsyth2/zppy_pr719_output/unique_id_12/v3.LR.amip_0101/post/scripts
> emacs pcmdi_diags_enso_model_vs_obs_2005-2015.o753616
# Warning: Could not extract dates from *.nc
# Warning: Catalogue not found at pcmdi_diags/ts_enso_catalogue.json
# Warning: Catalogue not found at pcmdi_diags/ts_ref_enso_catalogue.json
# KeyError: "Variable key 'psl' not found in observation dictionary."

> emacs pcmdi_diags_mean_climate_model_vs_obs_2005-2014.o753613 
# Warning: Could not extract dates from *.nc
# Warning: Catalogue not found at pcmdi_diags/climo_mean_climate_catalogue.json
# Warning: Catalogue not found at pcmdi_diags/climo_ref_mean_climate_catalogue.json
# ValueError: not enough values to unpack (expected 4, got 1)

> emacs pcmdi_diags_variability_modes_atm_model_vs_obs_2005-2014.o753615 
# Warning: Could not extract dates from *.nc
# Warning: Catalogue not found at pcmdi_diags/ts_variability_modes_atm_catalogue.json
# Warning: Catalogue not found at pcmdi_diags/ts_ref_variability_modes_atm_catalogue.json
# KeyError: 'psl'

> emacs pcmdi_diags_variability_modes_cpl_model_vs_obs_2005-2014.o753614
# Warning: Could not extract dates from *.nc
# Warning: Catalogue not found at pcmdi_diags/ts_variability_modes_cpl_catalogue.json
# Warning: Catalogue not found at pcmdi_diags/ts_ref_variability_modes_cpl_catalogue.json
# KeyError: 'ts'

YYYYS="${BASH_REMATCH[1]}"
YYYYE="${BASH_REMATCH[2]}"
else
echo "Warning: Could not extract dates from ${fname}"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Copy of https://github.com/E3SM-Project/zppy/pull/719/files#r2141246726:

> cd /lcrc/group/e3sm/ac.forsyth2/zppy_pr719_output/unique_id_12/v3.LR.amip_0101/post/scripts
> emacs pcmdi_diags_enso_model_vs_obs_2005-2015.o753616
# Warning: Could not extract dates from *.nc
# Warning: Catalogue not found at pcmdi_diags/ts_enso_catalogue.json
# Warning: Catalogue not found at pcmdi_diags/ts_ref_enso_catalogue.json
# KeyError: "Variable key 'psl' not found in observation dictionary."

> emacs pcmdi_diags_mean_climate_model_vs_obs_2005-2014.o753613 
# Warning: Could not extract dates from *.nc
# Warning: Catalogue not found at pcmdi_diags/climo_mean_climate_catalogue.json
# Warning: Catalogue not found at pcmdi_diags/climo_ref_mean_climate_catalogue.json
# ValueError: not enough values to unpack (expected 4, got 1)

> emacs pcmdi_diags_variability_modes_atm_model_vs_obs_2005-2014.o753615 
# Warning: Could not extract dates from *.nc
# Warning: Catalogue not found at pcmdi_diags/ts_variability_modes_atm_catalogue.json
# Warning: Catalogue not found at pcmdi_diags/ts_ref_variability_modes_atm_catalogue.json
# KeyError: 'psl'

> emacs pcmdi_diags_variability_modes_cpl_model_vs_obs_2005-2014.o753614
# Warning: Could not extract dates from *.nc
# Warning: Catalogue not found at pcmdi_diags/ts_variability_modes_cpl_catalogue.json
# Warning: Catalogue not found at pcmdi_diags/ts_ref_variability_modes_cpl_catalogue.json
# KeyError: 'ts'

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fname=$(basename "$file") is just getting the regex pattern *.nc as a string, so it's not actually matching individual files. To check -- is that accounted for elsewhere in the code or is this a bug?

@forsyth2 forsyth2 force-pushed the zppy_pcmdi_202505 branch 2 times, most recently from 53718fa to 42114e4 Compare June 13, 2025 22:36
This was referenced Aug 8, 2025
@forsyth2 forsyth2 added the priority: high High priority task (for next release) label Aug 9, 2025
@forsyth2
Copy link
Collaborator Author

forsyth2 commented Sep 17, 2025

Closing in favor of #732. Note the zppy-interfaces PR (E3SM-Project/zppy-interfaces#25) remains unchanged.

@forsyth2 forsyth2 closed this Sep 17, 2025
@forsyth2 forsyth2 deleted the zppy_pcmdi_202505 branch October 20, 2025 17:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

priority: high High priority task (for next release) semver: new feature New feature (will increment minor version)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add task for PMP

4 participants