Skip to content

Taylor Diagram with observations and multi-case - #435

Open
brianpm wants to merge 14 commits into
NCAR:mainfrom
brianpm:taylor_with_obs
Open

Taylor Diagram with observations and multi-case#435
brianpm wants to merge 14 commits into
NCAR:mainfrom
brianpm:taylor_with_obs

Conversation

@brianpm

@brianpm brianpm commented Jan 15, 2026

Copy link
Copy Markdown
Contributor

The main objective of this PR is to provide Taylor Diagrams with observation/reanalysis reference data, and thus to address #426.

Since Taylor Diagrams are especially useful for comparing multiple simulations to a common reference, I also prioritized being able to run with multiple test cases.

Refactoring the original cam_taylor_diagram.py mostly needed moving to use more of adf_dataset. The other key part was allowing for the potential need to regrid obs-to-obs for derived variables that the Taylor diagram uses. I also refactored the regrid_and_vert_interp.py module. Both use xESMF for regridding, defaulting to conservative methods. The regridding weights are currently being stored in a subdirectory of the regrid location.

For cases when we need to regrid from pressure levels to pressure levels, I added a plev_to_plev method in adf_utils. Elsewhere in that module I made some modifications to try to keep metadata.

I was running in to a hang when creating climo files, and I tracked it down to our use of multiprocessing. I think I've implemented a fix for that (which I think we've had to work around before). The main thing seems to be mp.get_context('spawn') but there's also some dask stuff to try to prevent zombie processes being made. This seems to work, but isn't crucial for this PR.

There are some updates to adf_info.py that help to deal with the multi-case configurations. This has to do with handling the specification of history files from the YAML file. This could break previously working configurations because it might make some assumptions about how those history files are specified.

To read nested lists correctly (i.e., [['h0a', 'h1a'],['h0a', 'h1a']]), I also needed to modify adf_config.py.

I added the ERA5 land fraction to the observations repository and included it in the variable defaults file.

A few modifications are included in adf_dataset, but mainly just trying to apply unit transformations more carefully. These might not be super necessary, but I don't think they will hurt anything.

@brianpm

brianpm commented Jan 15, 2026

Copy link
Copy Markdown
Contributor Author

I should also mention that I have a test configuration file here:
/glade/u/home/brianpm/drive_diagnostics/config_taylor_obs_MULTI.yaml

The web page generation does not work with this configuration... part of the reasoning for making it a draft PR for now.

Resolved conflicts in adf_dataset.py (kept apply_scaling kwargs on top of
main's hist_str-aware file lookup; factored the per-case regrid location
into a helper) and create_climo_files.py (main already carries the
spawn-context multiprocessing fix).
- regrid_and_vert_interp: source_grid was undefined for unstructured input
  (NameError); load_climo_da(...)['PS'] indexed a DataArray; the PS fallback
  passed a DataArray where a grid Dataset was expected; cam_regrid_loc as a
  plain string was indexed character-wise; weights dir needed parents=True;
  dropped two no-op 'final_ds = final_ds' blocks.
- _create_clean_grid: derive cell edges from centers when the file has no
  bounds, so conservative regridding works on CAM climo files.
- cam_taylor_diagram: reference data was only recognized when the label was
  literally 'Obs', so model-vs-model looked for the baseline in the test-case
  regrid files; route all reference loads through _is_ref/_load_ref_da, which
  falls back to the baseline climo when there is no regridded file. Obs weight
  directory was built from a case *name* rather than a path.
- adf_config: collapse the two near-identical recursion helpers into one, and
  cover nested-list keyword expansion in the unit tests.
- Remove leftover debug prints (create_climo_files, adf_info) and the
  regrid_and_vert_interp.py.OLD copy; repair the mangled comment at the end of
  hist_str_to_list.
vert_remap uses np.interp, which requires increasing x. Pressure-level data
often arrives surface-first (descending), which silently produced a constant
field equal to the topmost level.
load_regrid_da returns a DataArray, whose 'in' operator tests values, not
variable names, so 'hyam' not in dset was always True and the hybrid branch
could never run. Regridded files are already on pressure levels, so the
pressure path is the correct one; collapse the two identical branches and
drop the now-unused geocat/warnings imports. Also pass the Dataset (not the
DataArray) to _determine_vertical_coord_type so its hyam fallback can fire.
load_climo_ds and load_reference_climo_ds indexed the result of load_dataset
without checking for None, and load_reference_climo_ds looked up
variable_defaults[var] unguarded (KeyError for a variable with no defaults
entry). Also require new_unit to exist before treating a units match as
evidence the data were already transformed.
…posed

The rewritten regridder only wrote the test-case files, dropping the
{base}_{var}_baseline.nc products the old driver produced for every variable
in non-obs mode. zonal_mean, global_latlon_map and polar_map read them via
load_reference_regrid_da; meridional_mean and global_latlon_vect_map glob them
directly. All five degrade with a warning rather than crashing, so a
model-vs-model run silently lost the reference and difference panels from
every plot.

- Add _write_reference_files, run once before the test-case loop, reusing
  _determine_vertical_coord_type / _handle_vertical_interpolation /
  _apply_ocean_mask / save_to_nc. No horizontal regrid: the reference defines
  the target grid. Gated on 'not compare_obs', as the old driver was.
- Revert cam_regrid_loc to a single directory. The per-case list broke
  meridional_mean and global_latlon_vect_map (Path() of a list) and
  _regrid_loc_for_case (the baseline is not in case_names); regridded
  filenames already carry the case name.
- _find_surface_pressure: prefer the PS carried inside the climo file, as the
  old driver did, before a standalone PS climo. PS is often not in
  diag_var_list, which otherwise skipped every 3-D variable.

Bugs the run surfaced, all on paths shared with the test case:
- attrs['transformed'] = True could not be written; netCDF4 rejects a Python
  bool. Now 1.
- DEFAULT_PLEVS_Pa was a list; geocat's interp_hybrid_to_pressure needs .size
  and utils.vert_remap needs .shape, so every hybrid- and height-coordinate
  variable crashed.
- _create_clean_grid clipped the target grid CENTERS to +/-89.999999. Those
  become the output coordinate, so regridded files disagreed with the
  reference at the poles and xarray alignment silently dropped both polar rows
  from every difference field. Only the bounds are clamped now, to +/-90.
- load_climo_ds dropped attrs (xarray arithmetic does not keep them), so every
  regridded file lost 'units' and polar_map raised KeyError on it.
- cam_taylor_diagram advertised 'PRECT or PRECC+PRECL' but raised KeyError for
  either when PRECT was absent. Added _load_field/_load_field_ds, which return
  None for a variable that is not part of the run, and routed the loaders
  through them.

Verified by a full model-vs-model run (b.e23_alpha17f.BLT1850.ne30_t232.098 vs
.093, yrs 10-11): completes successfully, 16/16 baseline files written, 3-D
references on the same 21 pressure levels as the test case, difference fields
keep all 192 latitudes, and every plotting script produces its reference and
difference panels.
The ERA5 land-sea mask this PR adds as the LANDFRAC observation is a
single-timestamp file, not the 12-month climatology (time = 1..12) every other
file in ADF_obs uses. load_da's squeeze() collapses its length-1 time dimension
to a scalar coordinate, so data.time.dt.days_in_month succeeds but the
subsequent .sel(time=...) raises KeyError: no index found for coordinate
'time', which aborted global_latlon_map for every model-vs-obs run.

A time-invariant field has the same value in every season, so return it
unchanged (dropping the stale scalar time coordinate, matching what the
averaging path produces). Guarded on 'time' being a coordinate but not a
dimension, so the existing month-dimension and bare-numpy paths are untouched.
The Taylor diagram numbered its points with a counter that advanced only for
points actually drawn, while the figure legend numbers the full variable list.
Any variable without data therefore shifted every later label onto the wrong
name. In the model-vs-obs run just completed, three of ten variables have no
observational counterpart, so four of the seven plotted points were labelled
with the wrong variable -- the plot read as if TropicalLandPrecip were present
when point 4 was actually Land2mTemperature. Number by position in the frame
instead, which leaves gaps for missing variables and keeps labels correct; it
also removes the old requirement that all cases share the same row order.
Dropped the 'annos' list, which was accumulated and never used.

cam_taylor_diagram carried its own copy of _create_clean_grid that still
clipped the grid CENTERS to +/-89.999999 and never inferred bounds -- so the
regridding behind its derived variables ran without bounds (hence the
'Latitude is outside of [-90, 90]' warnings) and would reintroduce the polar
misalignment fixed in the regridder. Hoisted one implementation into
adf_utils.create_clean_grid, used by both, and deleted both copies.
The Taylor diagram's EquatorialPacificStress entry could never resolve against
observations because TAUX had no obs_file. The TAUX climatologies already
existed in ADF_obs (built from ERA5 iews) but were referenced only by the
era5-1deg defaults; the default file referenced neither TAUX nor TAUY, and
TAUY had no observation at all.

TAUY has now been built from the sibling ERA5 field (inss, monthly means from
RDA ds633.1, 1979-2022) at 0.25 degree native and conservatively regridded to
1 degree, by AMP_toolbox/make_era5_tau_obs.py. Point both defaults files at the
matching resolution: the default file at the native climatologies, as its other
ERA5 entries are, and the era5-1deg file at the 1 degree pair.

scale_factor: -1 is deliberately left untouched. ERA5 stores stress positive
eastward/northward on the surface, CAM uses the opposite sign, and ADF applies
that factor to the *model* when its climo is loaded for regridding while
observations are read unscaled -- verified over 5S-5N: ERA5 -0.0228 N/m2, CAM
raw +0.0283, CAM regridded -0.0283. Adding obs_scale_factor would double-flip.
The Taylor diagram accepts 'PRECT or PRECC+PRECL' for its two tropical
precipitation entries. That is right against a baseline simulation, but PRECC
and PRECL have no observational counterpart, so under compare_obs the check
passed and then both entries silently vanished from the diagram -- which is
how the obs run came out at 8 of 10 variables with no indication why.

Warn explicitly in that case, and note that ADF derives PRECT from PRECC +
PRECL when it builds the time series, so adding it costs the user nothing.
Verified: with PRECT in diag_var_list the derivation runs, the derived field
is exactly PRECC + PRECL, and it regrids onto ERAI's grid in mm/day matching
the observations. The obs Taylor diagram now shows all 10 variables.
@brianpm
brianpm marked this pull request as ready for review August 7, 2026 03:35
@brianpm

brianpm commented Aug 7, 2026

Copy link
Copy Markdown
Contributor Author

This comment authored by Claude, and so is very verbose. But it is comprehensive about what was changed in this latest update. At this point, the code seems to run and is improved. I might do another round of refactoring, but at this time I think it is ready for review/testing.

Branch taylor_with_obs was 42 commits behind main and conflicting. Merged
main in, reviewed the code, and verified with a full model-vs-model ADF run
(b.e23_alpha17f.BLT1850.ne30_t232.098 vs .093, years 10–11, 16 variables).
That run now reports "ADF diagnostics has completed successfully." Unit
tests pass (18); the two CI-gated files clear the 9.5 pylint bar
(adf_config.py 9.52, adf_info.py 9.71).

The blocker: the rewritten regridder stopped producing the reference

The old driver wrote two products per variable; the rewrite wrote only one.

output old (main) new (as submitted)
{target}_{case}_{var}_regridded.nc yes yes
{base}_{var}_baseline.nc yes never

The baseline file is not limited to 3-D fields — in the old driver a 2-D
variable passes straight through and is still written, so the file existed for
every variable in non-obs mode. Five scripts read it and nothing else produces
it: zonal_mean, global_latlon_map and polar_map via
load_reference_regrid_da; meridional_mean and global_latlon_vect_map by
globbing it directly. All five degrade with a warning rather than crashing, so
a model-vs-model run would have silently lost the reference and difference
panels from every plot.

Fixed by adding _write_reference_files, run once before the test-case loop
(the reference does not depend on the test case, and doing it first also fixes
an old-code quirk where the baseline was skipped whenever the test regridded
file already existed). It reuses the module's existing
_determine_vertical_coord_type / _handle_vertical_interpolation /
_apply_ocean_mask / save_to_nc. No horizontal regrid — the reference is
the target grid. Gated on not compare_obs, as the old driver was.

Bugs the end-to-end run exposed — all on paths shared with the test case

These were latent in the PR; the reference pass just hit them first.

  1. attrs['transformed'] = True could not be written. netCDF4 rejects a
    Python bool, so any variable carrying a scale factor failed to save. Now 1.
  2. DEFAULT_PLEVS_Pa was a Python list. geocat's
    interp_hybrid_to_pressure needs .size and utils.vert_remap needs
    .shape, so every hybrid- and height-coordinate variable crashed. Now an
    ndarray.
  3. _create_clean_grid clipped the target grid centers to ±89.999999.
    Those centers become the output coordinate, so regridded files disagreed
    with the reference at the poles by 1e-6°, and xarray alignment silently
    dropped both polar rows from every difference field — surfacing as
    IndexError: shape mismatch ... (190,1) (192,289) in cartopy. Only the
    bounds are clamped now, to ±90.
  4. load_climo_ds dropped attrs. ds[v] = ds[v] * scale + offset does not
    preserve them, so every regridded file lost units and polar_map raised
    KeyError: 'units'. (main's load_da always forced a units attribute;
    this PR removed that safety net, so the loss became visible.)
  5. PS resolution. ADF climo files for a 3-D variable carry PS alongside
    hyam/hybm, and the old driver preferred it. The rewrite only looked for
    a standalone PS climo, which exists only when PS is in diag_var_list — it
    is not in the standard config, so every 3-D variable was skipped. Added
    _find_surface_pressure to match the old precedence.
  6. cam_taylor_diagram could not honour its own requirement check. It
    accepts "PRECT or PRECC+PRECL", but both the reference and test branches
    raised KeyError: 'PRECT' instead of returning None when PRECT was
    absent. Added _load_field/_load_field_ds, which return None for a
    variable that is not part of the run, and routed every loader through them.

Other defects fixed while reviewing

Regridder: source_grid was undefined for unstructured input (NameError,
caught by pylint E0606); load_climo_da(case,'PS')['PS'] indexed a DataArray;
the PS fallback passed a DataArray where a grid Dataset was expected;
conservative regridding had no bounds to work with because CAM climo files
carry none, so cell edges are now inferred from centers; two no-op
final_ds = final_ds blocks and the committed 753-line
regrid_and_vert_interp.py.OLD removed.

cam_taylor_diagram: reference data was recognized only when the label was
literally 'Obs', so model-vs-model looked for the baseline among test-case
regrid files; the obs weights directory was built from Path(case_list[0]),
a case name rather than a path; get_var_at_plev's hybrid branch was
unreachable ('hyam' not in dataarray tests values, not variable names) and
was collapsed into the correct pressure-level path.

adf_utils: plev_to_plev was silently wrong for surface-first data
vert_remap uses np.interp, which needs increasing x, so a descending
pressure coordinate (the CMIP convention) returned a constant field equal to
the topmost level. Verified both orderings now agree.

adf_dataset: None-guards on the new Dataset loaders and an unguarded
variable_defaults[var] lookup; new_unit must exist before a units match
counts as "already transformed".

adf_config: collapsed __expand_dict_refs + __expand_list_refs (35 largely
duplicated lines) into one recursive helper, with a unit test for nested-list
keyword expansion — the actual new capability, previously uncovered.

cam_regrid_loc reverted to a single directory. The per-case list broke
meridional_mean.py:28 and global_latlon_vect_map.py:62 (Path() of a list)
and _regrid_loc_for_case (the baseline is not in case_names). Regridded
filenames already carry the case name, so one directory suffices, and it is
what config_cam_baseline_example.yaml:84 documents.

Housekeeping: removed leftover CHECK ON INPUT and HIST_STR_USE debug
prints, and repaired the comment glued onto the assignment at the end of
hist_str_to_list.

Verified by the run

  1. 16/16 variables have a {base}_{var}_baseline.nc, 2-D included.
  2. Every 3-D reference is on the identical 21 pressure levels (hPa) as its
    test counterpart; difference fields keep all 192 latitudes.
  3. Exactly one "did not find reference" warning in the whole log — the U,V
    vector pair, because V is not in this config's diag_var_list. Correct
    behaviour, not a regression.
  4. Spot-checked plots (PSL_ANN_LatLon, U_ANN_Zonal) show all four panels —
    test, baseline, %diff, difference — with correct units and full pole-to-pole
    coverage.
  5. Ocean masking reaches the reference: all 27,237 land cells in
    {base}_SST_baseline.nc are set to −999 with missing_value recorded.
    (Check this with decode_cf=False; xarray's CF decoding turns −999 into NaN
    and strips the attribute, which makes a correctly masked file look unmasked.)
  6. zonal_mean, global_latlon_map, global_latlon_vect_map,
    meridional_mean, polar_map, cam_taylor_diagram and qbo all complete;
    the Taylor diagram derives PRECT from PRECC+PRECL and places all 10
    variables at ratio ≈ 1, correlation ≈ 0.99+, as expected for two near-identical runs.

Worth deciding before merge

  1. hist_str_to_list is ambiguous. With N cases, ["h0","h1"] means "one
    stream per case" when N==2 and "both streams for every case" otherwise — the
    interpretation flips on the case count. Also [conf_val for _ in range(n)]
    hands every case the same list object; use list(conf_val).
  2. The cam_climo_info scalar broadcast ([conf_val]
    [conf_val] * num_cases) is the change the PR description warns could break
    existing configs. Worth a note in the example config / release notes.
  3. load_reference_climo_da picks the wrong scale factors for obs.
    get_value_converters compares case == ref_labels[var]; called with
    ref_case_label ("Obs") while ref_labels[var] is the source name (e.g.
    "ERA5"), it selects the model scale_factor rather than obs_scale_factor.
  4. taylor_stats_single reports bias as a percentage of the reference mean.
    For EquatorialPacificStress the 5°S–5°N TAUX mean can be near zero, so the
    percentage — and hence the marker size — becomes meaningless.
  5. weighted_correlation assumes matching NaN masks. Land/ocean-masked
    fields are masked with each case's own LANDFRAC, so case and reference can
    have different valid points. Relatedly, the test side masks with its
    conservatively-regridded OCNFRAC, which perturbs values near the 0.9
    threshold: ~205 of 27,237 land cells classify differently from the
    reference. Pre-existing behaviour, but a common mask would be cleaner.
  6. _handle_vertical_interpolation sources PMID from the climo Dataset
    itself
    , whereas the old driver could read a separate {target}_PMID_climo.nc.
    Same limitation already applies to the test side; height coordinates are
    rare.
  7. polar_map.py:131-134 picks base_name with the compare_obs test inverted
    relative to zonal_mean/global_latlon_map. Harmless today, but asymmetric.
  8. meridional_mean and global_latlon_vect_map still glob files by hand
    rather than using adf.data. Pre-existing; not this PR's job.

Model-vs-obs run — the PR's headline feature

Also run end to end (compare_obs: true, same test case against ERA5 / CERES /
ERAI), on a Casper node: completes successfully, Taylor diagram included.
All 12 variables that have obs regridded onto three different observational
grids — ERA5 721×1440, CERES 180×360, ERAI 121×240 — none of which carry
lat/lon bounds, so this is a real workout for the inferred-bounds conservative
regridding. The four variables with no obs (PRECC, PRECL, TAUX, TAUY) skip with
warnings rather than raising, which is _load_field doing its job.

Three further defects it exposed, all fixed:

  1. seasonal_mean crashed on the PR's own ERA5 land mask, aborting every
    model-vs-obs run.
    ERA5_LSM_1deg_conservativeregrid.nc is a
    single-timestamp file (2013-11-29, hours since 1900-01-01), not the
    12-month climatology (time = 1..12) every other file in ADF_obs uses.
    load_da's squeeze() leaves a scalar time coordinate, so
    data.time.dt.days_in_month succeeds and sails past the existing guard,
    then .sel(time=...) raises KeyError: no index found for coordinate 'time'. Time-invariant fields now pass through unchanged.
    Consider regenerating that obs file as a proper 12-month climatology so
    it matches the convention of every other file in the directory; the code fix
    is worth keeping either way.
  2. The Taylor diagram mislabelled its points whenever a variable was
    missing.
    Points were numbered by a counter that advanced only for points
    actually drawn, while the figure legend numbers the full 10-variable list.
    In the obs run, 3 of 10 variables have no counterpart, so the 7 plotted
    points were numbered 1–7 and four carried the wrong name — the figure read
    as if TropicalLandPrecip were present when point 4 was really
    Land2mTemperature. Silent, and it only appears when variables are missing,
    which is the normal case in obs mode (model-vs-model had all 10, so it
    looked fine). Now numbered by position, leaving gaps; this also retires the
    "requires cases to have same order in DataFrame" caveat.
  3. cam_taylor_diagram carried a duplicate _create_clean_grid that still
    clipped grid centers to ±89.999999 and never inferred bounds, so the
    regridding behind its derived variables ran without bounds (the Latitude is outside of [-90, 90] warnings) and would have reintroduced the polar
    misalignment fixed in the regridder. One implementation now lives in
    adf_utils.create_clean_grid.

Two obs-mode limitations that are not bugs but constrain the feature:

  • EquatorialPacificStress can never work against obsresolved. ERA5
    surface wind stress is now wired in: the TAUX climatologies already existed in
    ADF_obs but were referenced only by the era5-1deg defaults, and TAUY did
    not exist. TAUY was built from ERA5 inss (RDA ds633.1, 1979-2022) at 0.25°
    and 1° by AMP_toolbox/make_era5_tau_obs.py, and both defaults files now
    point at the matching resolution. The obs Taylor diagram went from 7 to 8 of
    10 variables; EquatorialPacificStress scores correlation 0.712, stddev ratio
    1.347, bias +26.2% (model -0.0287 vs ERA5 -0.0228 N/m² over 5°S-5°N — the
    model's easterly trades are ~26% stronger).

    Two things that fix uncovered, both worth knowing:

    • The pre-existing TAUX obs file stored latitude north-to-south, unlike
      every other file in ADF_obs. adf_utils.domain_stats (lib/adf_utils.py:435)
      slices with sel(lat=slice(lo, hi)), which returns an empty selection on
      a descending axis, so polar_map died in nanmin with
      a.size==0. Fixed in the data (a lossless sort, verified statistics
      bit-identical), but domain_stats silently mis-slicing any north-to-south
      dataset remains a live footgun in ADF.
    • The percentage bias is ambiguous for a signed quantity. +26.2% here means
      the model stress is 26% larger in magnitude in the same (negative)
      direction, not weaker. See the taylor_stats_single item below.
  • The two tropical-precip entries need PRECT in diag_var_list
    resolved. The derivation pathway itself works: ADF derives PRECT from
    PRECC + PRECL when building the time series (verified numerically identical
    to their sum), and it regrids onto ERAI's grid in mm/day matching the obs
    (model 3.101 vs ERAI 2.884 mm/day, a +7.5% wet bias). The entries were
    missing only because PRECT was absent from diag_var_list.

    The real defect was the requirement check: it accepts "PRECT or
    PRECC+PRECL", which is right against a baseline simulation but wrong under
    compare_obs, where neither PRECC nor PRECL has an observational
    counterpart. The check passed and both entries then vanished with no
    indication why. It now warns explicitly and points at PRECT.

    The obs Taylor diagram is now complete — all 10 variables.
    TropicalLandPrecip corr 0.851 / ratio 0.882 / bias +2.3%;
    TropicalOceanPrecip corr 0.858 / ratio 1.001 / bias +1.1%;
    EquatorialPacificStress corr 0.712 / ratio 1.347 / bias +26.2%.

Not exercised

Unstructured (ncol) input: the source_grid NameError fix on that path is
reasoned, not run — both test cases here are regular lat/lon.

Memory note: regridding a 3-D CAM field to ERA5 0.25° needs ~3.2 GB per
variable, and the horizontal regrid runs before the vertical interpolation,
so it inflates to 721×1440 while still carrying all 32 hybrid levels. The run
peaked near 60 GB and is killed outright on a login node. Same ordering as the
old driver, so not a regression, but obs comparisons against high-resolution
reanalysis need a batch job with real memory.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants