CDAT Migration Phase 2: Refactor `qbo` set #826

tomvothecoder · 2024-07-23T21:58:58Z

Closes CDAT Migration Phase 2: Refactor qbo set #664

Checklist

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)

tomvothecoder · 2024-07-29T22:08:38Z

@chengzhuzhang Here are the plots. The diff plot shows a really small diff. I think this PR is good to merge after I fix the plot titles and unit tests.

Dev	Main	Diff

Source: https://portal.nersc.gov/project/e3sm/cdat-migration-fy24/664-qbo/qbo/

tomvothecoder

Final code review

e3sm_diags/driver/qbo_driver.py

tomvothecoder · 2024-07-29T22:52:36Z

e3sm_diags/driver/utils/dataset_xr.py

+        all_vars_keys = list(ds.data_vars.keys())
+        hybrid_var_keys = set(list(sum(HYBRID_SIGMA_KEYS.values(), ())))
+        keep_vars = [
+            var
+            for var in all_vars_keys
+            if "bnd" in var or "bounds" in var or var in hybrid_var_keys
+        ]
+        ds = ds[[self.var] + keep_vars]


The build breaks in this PR because we didn't keep hybrid variables to reconstruct pressure from hybrid in PR #750. This should fix the build.

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 <[email protected]> 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 <[email protected]> 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 <[email protected]> [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 <[email protected]> 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 <[email protected]> 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)

tomvothecoder added the cdat-migration-fy24 CDAT Migration FY24 Task label Jul 23, 2024

tomvothecoder self-assigned this Jul 23, 2024

tomvothecoder added this to the FY24 Q4 (07/01/24 - 9/30/24) milestone Jul 23, 2024

tomvothecoder added 2 commits July 25, 2024 15:04

Initial refactor of qbo

b39c7b8

Add subsetting prototype code

0c319f6

tomvothecoder force-pushed the refactor/664-qbo branch from eda00de to 0c319f6 Compare July 25, 2024 22:04

Add final regression test

394ac8e

tomvothecoder added 2 commits July 29, 2024 15:20

Fix unit tests

a3b9948

Fix plot title

5044253

tomvothecoder marked this pull request as ready for review July 29, 2024 22:32

Remove deprecated plotter

63880ba

tomvothecoder commented Jul 29, 2024

View reviewed changes

e3sm_diags/driver/qbo_driver.py Outdated Show resolved Hide resolved

e3sm_diags/driver/qbo_driver.py Outdated Show resolved Hide resolved

tomvothecoder added 4 commits July 29, 2024 15:39

Update e3sm_diags/driver/qbo_driver.py

b18396c

Update e3sm_diags/driver/qbo_driver.py

1c16c2f

Keep hybrid sigma variables

a66058f

Keep hybrid sigma variables

6a319a3

tomvothecoder commented Jul 29, 2024

View reviewed changes

tomvothecoder added 5 commits July 29, 2024 15:53

Remove whitespace

340c4d7

Add future annotations import for plotter

e754dc7

Add _subset_vars_and_load()

bd7e9e6

Add exception for ref name AttributeError

b4e03d3

Add MetricsDict type annotation

d4b8ca3

tomvothecoder merged commit 4dc44a0 into cdat-migration-fy24 Jul 30, 2024
4 checks passed

tomvothecoder deleted the refactor/664-qbo branch July 30, 2024 17:23

tomvothecoder mentioned this pull request Jul 30, 2024

CDAT Migration Phase 2: Refactor qbo set #664

Closed

1 task

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

CDAT Migration Phase 2: Refactor qbo set (#826)

eed8a29

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

CDAT Migration Phase 2: Refactor qbo set (#826)

c7ef34e

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

CDAT Migration Phase 2: Refactor qbo set (#826)

da7ff2d

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

CDAT Migration Phase 2: Refactor qbo set (#826)

7eb0092

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

CDAT Migration Phase 2: Refactor qbo set (#826)

907d2f0

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

CDAT Migration Phase 2: Refactor qbo set (#826)

f6c6eef

tomvothecoder added a commit that referenced this pull request Nov 11, 2024

CDAT Migration Phase 2: Refactor qbo set (#826)

bff1359

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

CDAT Migration Phase 2: Refactor qbo set (#826)

f2a701b

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CDAT Migration Phase 2: Refactor `qbo` set #826

CDAT Migration Phase 2: Refactor `qbo` set #826

tomvothecoder commented Jul 23, 2024 •

edited

Loading

tomvothecoder commented Jul 29, 2024 •

edited

Loading

tomvothecoder left a comment

tomvothecoder Jul 29, 2024

CDAT Migration Phase 2: Refactor qbo set #826

CDAT Migration Phase 2: Refactor qbo set #826

Conversation

tomvothecoder commented Jul 23, 2024 • edited Loading

Checklist

tomvothecoder commented Jul 29, 2024 • edited Loading

tomvothecoder left a comment

Choose a reason for hiding this comment

tomvothecoder Jul 29, 2024

Choose a reason for hiding this comment

CDAT Migration Phase 2: Refactor `qbo` set #826

CDAT Migration Phase 2: Refactor `qbo` set #826

tomvothecoder commented Jul 23, 2024 •

edited

Loading

tomvothecoder commented Jul 29, 2024 •

edited

Loading