CDAT Migration: Test `lat_lon` set with run script and debug any issues #794

tomvothecoder · 2024-02-26T22:20:02Z

Issues fixed

ValueError: Multiple 'Z' axis dims were found in this dataset, ['ilev', 'lev']. Please drop the unused dimension(s) before performing grid operations. -- affects _align_grids_to_lower_res() and _apply_land_sea_mask()
- Root Cause: Some datasets have ilev coordinates with pressure variables (e.g., hyam, hybm) that are not used
- Solution: Add _get_grid() and _drop_unused_ilev_axis()
KeyError: No attribute "units" -- affected cosp_bin_sum()
- Root Cause: In cosp_bin_sum(), calling .sum() results in Xarray dropping the original units attribute. The original units are required for the "convert_units()`" function to correctly convert units to "%". Otherwise, units are None and never set to %.
- Solution: Add keep_attrs=True to .sum() call
OSError: No file found for 'HadISST' and 'JJA' in /global/cfs/cdirs/e3sm/diagnostics/observations/Atm/climatologyand OSError: No file found for 'HadISST' and 'ANN' in /global/cfs/cdirs/e3sm/diagnostics/observations/Atm/climatology
- Root cause: Trying to open a non-existent ref dataset to get the global attr to set on the parameter object.
- Solution: Use a try and except in Dataset._get_global_attr_from_climo_dataset() to cover cases where ref files are missing and the test dataset is being used as the ref dataset
x and y nan location mismatching -- affects CLDTOT_TAU1.3_9.4_ISCCP, CLDTOT_TAU1.3_ISCCP, CLDTOT_TAU9.4_ISCCP, CLDTOT_TAU9.4_ISCCP, CLDLOW_TAU1.3_9.4_MISR, CLDLOW_TAU1.3_MISR, CLDLOW_TAU9.4_MISR, CLDTOT_TAU1.3_9.4_MISR, CLDTOT_TAU1.3_MISR, CLDTOT_TAU9.4_MISR, CLDHGH_TAU1.3_9.4_MODIS, CLDHGH_TAU1.3_MODIS, CLDHGH_TAU9.4_MODIS, CLDTOT_TAU1.3_9.4_MODIS, CLDTOT_TAU1.3_MODIS, CLDTOT_TAU9.4_MODIS
- Root cause: cosp_bin_sum() call to .sum() results in np.nan being replaced with 0s
- Solution: Pass skipna=True and min_count=1 to .sum() (related post)
- Closes [Bug]: CDAT Migration - Fix large relative diff and x and y nan location mismatch for some variables in zonal_mean_xy set #790
PminusE - large relative differences
- Root cause: Attributes are not preserved due to lambda arithmetic with xarray objects
- Solution: Define individual formula functions (pminuse_1(), pminuse_2(), pminuse_3()) and move arithmetic into those functions wrapped by with xr.set_options(keep_attrs=True):
QREFHT - large relative difference and shape mismatch (180, 360), (721, 1440)
- Root cause: Missing derivation definitions in derivations.py from acme.py on main (related PR).
- Solution: Add derivation definitions from acme.py
ALBEDOC (reference dataset) - x and y nan location mismatching
- Root Cause: The albedoc() formula function divides rsutcs / rsdt. rsdt contains 0s, which results in divide by zeros. Xarray and NumPy replaces these values with np.inf instead of throwing a "Divide By Zero" error. Meanwhile CDAT replaces these values with the floats from rsutcs, but they are masked so the output will np.nan.
- Solution: Add _replace_inf_with_nan() to update the array values for this case
CRU-TREFHT -- land mask is not being applied
- Root cause: The conditional in subset_and_align_datasets() was incorrect and did not consider "land_60S90N"
- Solution: Update the conditional from if region == "land" or region == "ocean": to if "land" in region or "ocean" in region:"
CRU-TREFHT -- after applying land mask, region subsetting is not being applied
- Root cause: The conditional in subset_and_align_datasets() was incorrect and ignored subsetting
- Solution: Update the conditional from elif region == "global": to if "global" not in region:

Differences we can accept

There seems to be a minor difference in nan locations for the regridded land-sea mask produced using xCDAT + xESMF vs. CDAT + ESMF (method="bilinear"). xCDAT has a little bit more nan values than CDAT. This causes x and y nan location mismatch when comparing array values. I don't know the exact reason why there are differences, but they seem insignificant enough to accept.

The absolute sum, mean, and nan count values are similar across variables. The diff plots also show no visible differences. All of the scripts for debugging these variables can be found here.

TAUXY (ocean)

xCDAT	CDAT	Diff

ERA5-TREFHT (land)

xCDAT	CDAT	Diff

MERRA2-TREFHT (land)

xCDAT	CDAT	Diff

Issues on `main`

Some reference variable names don't align with main - affects
AODVIS, FLUT, FLUTC, LCWF, RESTOM, SOLIN, SWCF, FLDS, FLDSC, FSDS, FSDSC, FSNS, FSNSC, LHFLX, SHFLX, T, U, U10, PRECT, SST
- Root Cause: main saves the original variable name, while cdat-migration-fy24 saves the target (derived) variable name. cdat-migration-fy24 is doing the right thing here.
- Solution: Use the derived variables dictionary in the template notebook to align variable keys before comparing.
The following variables have large diffs due to a bug on main: MISRCOSP-CLDLOW_TAU1.3_9.4_MISR, MISRCOSP-CLDLOW_TAU1.3_MISR, MISRCOSP-CLDLOW_TAU9.4_MISR. I believe this branch is doing the correct thing. We can revisit the comparison for these variables once the issue below is fixed on main.
- [Bug]: Silent bug in adjust_prs_val_units() conditional where if prs_val0: will be False if prs_val0 is 0 #797

Checklist

Closes CDAT Migration - Update regression testing notebook for lat_lon set using run script #792
My code follows the style guidelines of this project
I have performed a self-review of my own code
My changes generate no new warnings
Any dependent changes have been merged and published in downstream modules

If applicable:

New and existing unit tests pass with my changes (locally and CI/CD build)
I have added tests that prove my fix is effective or that my feature works
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
I have noted that this is a breaking change for a major release (fix or feature that would cause existing functionality to not work as expected)

- Fix multiple Z axis dims by dropping ilev with `_drop_unused_ilev_axis()` - Fix `cosp_bin_sum()` `.sum()` call not preserving units for unit conversion

- Fix unused ilev axis not being dropped in `_apply_land_sea_mask()`

The mask and subsetting was not being applied properly for all cases

auxiliary_tools/cdat_regression_testing/utils.py

tomvothecoder · 2024-03-12T20:35:21Z

Hi @chengzhuzhang, I'm just tagging you so you're aware of the issues that were fixed in this PR, differences that we can accept, and issues found on the main branch. I will merge this PR to continue moving forward.

With the list of issues fixed, the cdat-migration-fy24 branch will be even more robust.

chengzhuzhang · 2024-03-12T20:58:53Z

Hi @chengzhuzhang, I'm just tagging you so you're aware of the issues that were fixed in this PR, differences that we can accept, and issues found on the main branch. I will merge this PR to continue moving forward.

With the list of issues fixed, the cdat-migration-fy24 branch will be even more robust.

Got it. Thank you for working on this!

…es (#794)

Refer to the PR for more information because the changelog is massive. Update build workflow to run on `cdat-migration-fy24` branch CDAT Migration Phase 2: Add CDAT regression test notebook template and fix GH Actions build (#743) - Add Makefile for quick access to multiple Python-based commands such as linting, testing, cleaning up cache and build files - Fix some lingering unit tests failure - Update `xcdat=0.6.0rc1` to `xcdat >=0.6.0` in `ci.yml`, `dev.yml` and `dev-nompi.yml` - Add `xskillscore` to `ci.yml` - Fix `pre-commit` issues CDAT Migration Phase 2: Regression testing for `lat_lon`, `lat_lon_land`, and `lat_lon_river` (#744) - Add Makefile that simplifies common development commands (building and installing, testing, etc.) - Write unit tests to cover all new code for utility functions - `dataset_xr.py`, `metrics.py`, `climo_xr.py`, `io.py`, `regrid.py` - Metrics comparison for `cdat-migration-fy24` `lat_lon` and `main` branch of `lat_lon` -- `NET_FLUX_SRF` and `RESTOM` have the highest spatial average diffs - Test run with 3D variables (`_run_3d_diags()`) - Fix Python 3.9 bug with using pipe command to represent Union -- doesn't work with `from __future__ import annotations` still - Fix subsetting syntax bug using ilev - Fix regridding bug where a single plev is passed and xCDAT does not allow generating bounds for coordinates of len <= 1 -- add conditional that just ignores adding new bounds for regridded output datasets, fix related tests - Fix accidentally calling save plots and metrics twice in `_get_metrics_by_region()` - Fix failing integration tests pass in CI/CD - Refactor `test_diags.py` -- replace unittest with pytest - Refactor `test_all_sets.py` -- replace unittest with pytest - Test climatology datasets -- tested with 3d variables using `test_all_sets.py` CDAT Migration Phase 2: Refactor utilities and CoreParameter methods for reusability across diagnostic sets (#746) - Move driver type annotations to `type_annotations.py` - Move `lat_lon_driver._save_data_metrics_and_plots()` to `io.py` - Update `_save_data_metrics_and_plots` args to accept `plot_func` callable - Update `metrics.spatial_avg` to return an optionally `xr.DataArray` with `as_list=False` - Move `parameter` arg to the top in `lat_lon_plot.plot` - Move `_set_param_output_attrs` and `_set_name_yr_attrs` from `lat_lon_driver` to `CoreParameter` class Regression testing for lat_lon variables `NET_FLUX_SRF` and `RESTOM` (#754) Update regression test notebook to show validation of all vars Add `subset_and_align_datasets()` to regrid.py (#776) Add template run scripts CDAT Migration Phase: Refactor `cosp_histogram` set (#748) - Refactor `cosp_histogram_driver.py` and `cosp_histogram_plot.py` - `formulas_cosp.py` (new file) - Includes refactored, Xarray-based `cosp_histogram_standard()` and `cosp_bin_sum()` functions - I wrote a lot of new code in `formulas_cosp.py` to clean up `derivations.py` and the old equivalent functions in `utils.py` - `derivations.py` - Cleaned up portions of `DERIVED_VARIABLES` dictionary - Removed unnecessary `OrderedDict` usage for `cosp_histogram` related variables (we should do this for the rest of the variables in in #716) - Remove unnecessary `convert_units()` function calls - Move cloud levels passed to derived variable formulas to `formulas_cosp.CLOUD_BIN_SUM_MAP` - `utils.py` - Delete deprecated, CDAT-based `cosp_histogram` functions - `dataset_xr.py` - Add `dataset_xr.Dataset._open_climo_dataset()` method with a catch for dataset quality issues where "time" is a scalar variable that does not match the "time" dimension array length, drops this variable and replaces it with the correct coordinate - Update `_get_dataset_with_derivation_func()` to handle derivation functions that require the `xr.Dataset` and `target_var_key` args (e.g., `cosp_histogram_standardize()` and `cosp_bin_sum()`) - `io.py` - Update `_write_vars_to_netcdf()` to write test, ref, and diff variables to individual netCDF (required for easy comparison to CDAT-based code that does the same thing) - Add `cdat_migration_regression_test_netcdf.ipynb` validation notebook template for comparing `.nc` files CDAT Migration Phase 2: Refactor `zonal_mean_2d()` and `zonal_mean_2d_stratosphere()` sets (#774) Refactor 654 zonal mean xy (#752) Co-authored-by: Tom Vo <tomvothecoder@gmail.com> CDAT Migration - Update run script output directory to NERSC public webserver (#793) [PR]: CDAT Migration: Refactor `aerosol_aeronet` set (#788) CDAT Migration: Test `lat_lon` set with run script and debug any issues (#794) CDAT Migration: Refactor `polar` set (#749) Co-authored-by: Tom Vo <tomvothecoder@gmail.com> Align order of calls to `_set_param_output_attrs` CDAT Migration: Refactor `meridional_mean_2d` set (#795) CDAT Migration: Refactor `aerosol_budget` (#800) Add `acme.py` changes from PR #712 (#814) * Add `acme.py` changes from PR #712 * Replace unnecessary lambda call Refactor area_mean_time_series and add ccb slice flag feature (#750) Co-authored-by: Tom Vo <tomvothecoder@gmail.com> [Refactor]: Validate fix in PR #750 for #759 (#815) CDAT Migration Phase 2: Refactor `diurnal_cycle` set (#819) CDAT Migration: Refactor annual_cycle_zonal_mean set (#798) * Refactor `annual_cycle_zonal_mean` set * Address PR review comments * Add lat lon regression testing * Add debugging scripts * Update `_open_climo_dataset()` to decode times as workaround to misaligned time coords - Update `annual_cycle_zonal_mean_plot.py` to convert time coordinates to month integers * Fix unit tests * Remove old plotter * Add script to debug decode_times=True and ncclimo file * Update plotter time values to month integers * Fix slow `.load()` and multiprocessing issue - Due to incorrectly updating `keep_bnds` logic - Add `_encode_time_coords()` to workaround cftime issue `ValueError: "months since" units only allowed for "360_day" calendar` * Update `_encode_time_coords()` docstring * Add AODVIS debug script * update AODVIS obs datasets; regression test results --------- Co-authored-by: Tom Vo <tomvothecoder@gmail.com> CDAT Migration Phase 2: Refactor `qbo` set (#826) CDAT Migration Phase 2: Refactor tc_analysis set (#829) * start tc_analysis_refactor * update driver * update plotting * Clean up plotter - Remove unused variables - Make `plot_info` a constant called `PLOT_INFO`, which is now a dict of dicts - Reorder functions for top-down readability * Remove unused notebook --------- Co-authored-by: tomvothecoder <tomvothecoder@gmail.com> CDAT Migration Phase 2: Refactor `enso_diags` set (#832) CDAT Migration Phase 2: Refactor `streamflow` set (#837) [Bug]: CDAT Migration Phase 2: enso_diags plot fixes (#841) [Refactor]: CDAT Migration Phase 3: testing and documentation update (#846) CDAT Migration Phase 3 - Port QBO Wavelet feature to Xarray/xCDAT codebase (#860) CDAT Migration Phase 2: Refactor arm_diags set (#842) Add performance benchmark material (#864) Add function to add CF axis attr to Z axis if missing for downstream xCDAT operations (#865) CDAT Migration Phase 3: Add Convective Precipitation Fraction in lat-lon (#875) CDAT Migration Phase 3: Fix LHFLX name and add catch for non-existent or empty TE stitch file (#876) Add support for time series datasets via glob and fix `enso_diags` set (#866) Add fix for checking `is_time_series()` property based on `data_type` attr (#881) CDAT migration: Fix African easterly wave density plots in TC analysis and convert H20LNZ units to ppm/volume (#882) CDAT Migration: Update `mp_partition_driver.py` to use Dataset from `dataset_xr.py` (#883) CDAT Migration - Port JJB tropical subseasonal diags to Xarray/xCDAT (#887) CDAT Migration: Prepare branch for merge to `main` (#885) [Refactor]: CDAT Migration - Update dependencies and remove Dataset._add_cf_attrs_to_z_axes() (#891) CDAT Migration Phase 2: Refactor core utilities and `lat_lon` set (#677) Refer to the PR for more information because the changelog is massive. Update build workflow to run on `cdat-migration-fy24` branch CDAT Migration Phase 2: Add CDAT regression test notebook template and fix GH Actions build (#743) - Add Makefile for quick access to multiple Python-based commands such as linting, testing, cleaning up cache and build files - Fix some lingering unit tests failure - Update `xcdat=0.6.0rc1` to `xcdat >=0.6.0` in `ci.yml`, `dev.yml` and `dev-nompi.yml` - Add `xskillscore` to `ci.yml` - Fix `pre-commit` issues CDAT Migration Phase 2: Regression testing for `lat_lon`, `lat_lon_land`, and `lat_lon_river` (#744) - Add Makefile that simplifies common development commands (building and installing, testing, etc.) - Write unit tests to cover all new code for utility functions - `dataset_xr.py`, `metrics.py`, `climo_xr.py`, `io.py`, `regrid.py` - Metrics comparison for `cdat-migration-fy24` `lat_lon` and `main` branch of `lat_lon` -- `NET_FLUX_SRF` and `RESTOM` have the highest spatial average diffs - Test run with 3D variables (`_run_3d_diags()`) - Fix Python 3.9 bug with using pipe command to represent Union -- doesn't work with `from __future__ import annotations` still - Fix subsetting syntax bug using ilev - Fix regridding bug where a single plev is passed and xCDAT does not allow generating bounds for coordinates of len <= 1 -- add conditional that just ignores adding new bounds for regridded output datasets, fix related tests - Fix accidentally calling save plots and metrics twice in `_get_metrics_by_region()` - Fix failing integration tests pass in CI/CD - Refactor `test_diags.py` -- replace unittest with pytest - Refactor `test_all_sets.py` -- replace unittest with pytest - Test climatology datasets -- tested with 3d variables using `test_all_sets.py` CDAT Migration Phase 2: Refactor utilities and CoreParameter methods for reusability across diagnostic sets (#746) - Move driver type annotations to `type_annotations.py` - Move `lat_lon_driver._save_data_metrics_and_plots()` to `io.py` - Update `_save_data_metrics_and_plots` args to accept `plot_func` callable - Update `metrics.spatial_avg` to return an optionally `xr.DataArray` with `as_list=False` - Move `parameter` arg to the top in `lat_lon_plot.plot` - Move `_set_param_output_attrs` and `_set_name_yr_attrs` from `lat_lon_driver` to `CoreParameter` class CDAT Migration Phase 2: Refactor `zonal_mean_2d()` and `zonal_mean_2d_stratosphere()` sets (#774) CDAT Migration Phase 2: Refactor `qbo` set (#826)

Update order of conditionals when checking axes

3e56557

tomvothecoder changed the title ~~CDAT Migration Phase 2: Refactor core utilities and lat_lon set (#677)~~ CDAT Migration: Debug lat_lon set with run script Feb 26, 2024

tomvothecoder self-assigned this Feb 26, 2024

tomvothecoder added the cdat-migration-fy24 CDAT Migration FY24 Task label Feb 26, 2024

tomvothecoder added 4 commits February 28, 2024 14:25

Add fixes for missing variables

778c5aa

- Fix multiple Z axis dims by dropping ilev with `_drop_unused_ilev_axis()` - Fix `cosp_bin_sum()` `.sum()` call not preserving units for unit conversion

Fix multiple Z axis error in _apply_land_sea_mask()

0ff0adb

Update notebooks

eb7475b

Fix OSError when getting global attr from ref file in model-only run

534dab5

- Fix unused ilev axis not being dropped in `_apply_land_sea_mask()`

tomvothecoder changed the title ~~CDAT Migration: Debug lat_lon set with run script~~ CDAT Migration: Test lat_lon set with run script and debug any issues Mar 4, 2024

tomvothecoder added 6 commits March 4, 2024 14:09

Fix .sum() replacing np.nan with 0s

d19be0b

Fix PminusE derived var not preserving units

72a4878

Update albedoc() to replace np.inf with np.nan

e01974d

Add missing derivations for QREFHT

53c600d

Add debug directories for tauxy and trefht

6b76c39

Fix region mask and subsetting conditional

e1ebe90

The mask and subsetting was not being applied properly for all cases

tomvothecoder mentioned this pull request Mar 6, 2024

[Bug]: Silent bug in adjust_prs_val_units() conditional where if prs_val0: will be False if prs_val0 is 0 #797

Closed

tomvothecoder added 2 commits March 12, 2024 09:58

Start debugging land sea mask diffs

5af92c5

Add debug scripts for remaining variables

5a976bf

tomvothecoder linked an issue Mar 12, 2024 that may be closed by this pull request

CDAT Migration - Update regression testing notebook for lat_lon set using run script #792

Closed

3 tasks

tomvothecoder removed a link to an issue Mar 12, 2024

CDAT Migration - Update regression testing notebook for lat_lon set using run script #792

Closed

3 tasks

This was linked to issues Mar 12, 2024

CDAT Migration - Update regression testing notebook for lat_lon set using run script #792

Closed

[Bug]: CDAT Migration - Fix large relative diff and x and y nan location mismatch for some variables in zonal_mean_xy set #790

Closed

tomvothecoder added this to the FY24 Q2 (01/01/24 - 03/31/24) milestone Mar 12, 2024

Update notebooks

d412de2

tomvothecoder commented Mar 12, 2024

View reviewed changes

auxiliary_tools/cdat_regression_testing/utils.py Outdated Show resolved Hide resolved

Update auxiliary_tools/cdat_regression_testing/utils.py

91127b2

Fix unit tests

21289cd

tomvothecoder merged commit dd4694c into cdat-migration-fy24 Mar 12, 2024
4 checks passed

tomvothecoder deleted the refactor/792-lat-lon-debug branch March 12, 2024 21:50

This was referenced Mar 14, 2024

CDAT Migration - Update regression testing notebook for lat_lon set using run script #792

Closed

[Bug]: CDAT Migration - Fix large relative diff and x and y nan location mismatch for some variables in zonal_mean_xy set #790

Closed

tomvothecoder added a commit that referenced this pull request May 1, 2024

CDAT Migration: Test lat_lon set with run script and debug any issu…

267a66f

…es (#794)

This was referenced Jun 3, 2024

[Refactor]: Validate fix in PR #750 for #759 #815

Merged

[Bug]: Figure out why TREFHT regridded reference variable is not regridding on main with ESMF bilinear #817

Closed

tomvothecoder added a commit that referenced this pull request Jul 15, 2024

CDAT Migration: Test lat_lon set with run script and debug any issu…

aeeef18

…es (#794)

tomvothecoder added a commit that referenced this pull request Jul 15, 2024

CDAT Migration: Test lat_lon set with run script and debug any issu…

8f3899c

…es (#794)

tomvothecoder added a commit that referenced this pull request Aug 21, 2024

CDAT Migration: Test lat_lon set with run script and debug any issu…

43647f3

…es (#794)

tomvothecoder added a commit that referenced this pull request Oct 1, 2024

CDAT Migration: Test lat_lon set with run script and debug any issu…

b7bafb3

…es (#794)

tomvothecoder mentioned this pull request Oct 21, 2024

[Refactor]: CDAT Migration Phase 3 - Port changes from main to cdat-migration-fy24 #871

Closed

tomvothecoder added a commit that referenced this pull request Oct 25, 2024

CDAT Migration: Test lat_lon set with run script and debug any issu…

a9163f7

…es (#794)

tomvothecoder added a commit that referenced this pull request Oct 29, 2024

CDAT Migration: Test lat_lon set with run script and debug any issu…

9682e23

…es (#794)

tomvothecoder added a commit that referenced this pull request Oct 29, 2024

CDAT Migration: Test lat_lon set with run script and debug any issu…

1c3d2f5

…es (#794)

tomvothecoder added a commit that referenced this pull request Dec 4, 2024

CDAT Migration: Test lat_lon set with run script and debug any issu…

34453fe

…es (#794)

This was referenced Jan 28, 2025

Enable CDAT-migrated E3SM Diags E3SM-Project/zppy#651

Merged

[Bug]: Debug zppy diffs for v3.0.0 #931

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CDAT Migration: Test `lat_lon` set with run script and debug any issues #794

CDAT Migration: Test `lat_lon` set with run script and debug any issues #794

tomvothecoder commented Feb 26, 2024 •

edited

Loading

tomvothecoder commented Mar 12, 2024 •

edited

Loading

chengzhuzhang commented Mar 12, 2024

CDAT Migration: Test lat_lon set with run script and debug any issues #794

CDAT Migration: Test lat_lon set with run script and debug any issues #794

Conversation

tomvothecoder commented Feb 26, 2024 • edited Loading

Issues fixed

Differences we can accept

Issues on main

Checklist

tomvothecoder commented Mar 12, 2024 • edited Loading

chengzhuzhang commented Mar 12, 2024

CDAT Migration: Test `lat_lon` set with run script and debug any issues #794

CDAT Migration: Test `lat_lon` set with run script and debug any issues #794

tomvothecoder commented Feb 26, 2024 •

edited

Loading

Issues on `main`

tomvothecoder commented Mar 12, 2024 •

edited

Loading