CDAT Migration Phase 2: Refactor core utilities and `lat_lon` set by tomvothecoder · Pull Request #677 · E3SM-Project/e3sm_diags

tomvothecoder · 2023-03-17T18:38:17Z

Description

Closes CDAT Migration Phase 2: Refactor lat_lon set #658

This PR refactors most core utilities for e3sm_diags to use Xarray/xCDAT since CDAT is deprecated at the end of the year. The lat_lon set is also refactored to use these core utilities.

How to Review

This is a large PR that involves many changes. I am interested in code review for areas of improvement, optimization, potential bugs, etc. The functionality itself is unchanged and working (refer to viewer outputs at the bottom of the description).

To help streamline the review process, there is section below for the main focus areas. If you are interested and have time, feel free to review other files and functions that are not listed.

I anticipate review will take the next few weeks (targeting end of Sep/2023). I'll try my best to respond to any comments, suggestions, or questions you might have. Thanks for the help!

Main Focus Areas

Each focus area includes the most important files and functions to review.

Most refactored functions include a docstring with a "Notes" section at the bottom. The "Notes" section lists the old function it replaces (if you wanted to compare them). Here's an example (look at the bottom of the docstring):

e3sm_diags/e3sm_diags/driver/utils/regrid.py

Lines 180 to 200 in 3e0a087

    
           def _subset_on_region(ds: xr.Dataset, var_key: str, region: str) -> xr.Dataset: 
        
               """Subset a variable in the dataset based on the region. 
        
               Parameters 
        
               ---------- 
        
               ds : xr.Dataset 
        
                   The dataset. 
        
               var_key : str 
        
                   The variable to subset. 
        
               region : str 
        
                   The region 
        
               Returns 
        
               ------- 
        
               xr.Dataset 
        
                   The dataest with the subsetted variable. 
        
               Notes 
        
               ----- 
        
               Replaces `e3sm_diags.utils.general.select_region`. 
        
               """

(1) Dataset class

driver/dataset_xr.py
- Methods have been reordered to make the class more readable
- Methods have been renamed, refactored, and updated with docstrings. I tried to group related time series and climatology functions together.
- Most methods now return xr.Dataset objects, which are crucial because they contain bounds for downstream operations (e.g., generating weights). Variables in xr.Dataset objects should be operated on (regridding, regional subsetting, metrics calculation) until the last step, plotting (which will use the variables directly as xr.DataArray).

(2) Computation, Regridding, and I/O Utilities

driver/utils/climo_xr.py
- climo() function has the same logic as the older version but operates on Xarray objects
driver/utils/regrid.py
- align_grids_to_lower_res (formerly regrid_to_lower_res())
- regrid_z_axis_to_plevs() (formerly convert_to_plevs())
  - _hybrid_to_plevs()
  - _pressure_to_plevs()
driver/metrics/metrics.py
- spatial_avg() (formerly mean())
- std()
- correlation()
- rmse()
driver/utils/io.py
- Moved _write_vars_to_netcdf() and _get_output_dir() from general.py to this file

(3) Lat Lon Driver

driver/lat_lon_driver.py
- Moved run_diag() to the top level of the file
- Most of the code is similar. I replaced function calls to use the new refactored functions, renamed things, and tried to refactor logic to make it more readable.
- xr.Dataset objects are mainly used because they contain bounds. The test/reference variable gets updated in the xr.Dataset objects when applying land sea mask, regridding, etc. These objects are passed to _create_and_save_metrics().

(4) Lat Lon Plotter

plot/lat_lon_plot.py
- Refactored arguments to expect xr.DataArray type
- Extract code into separate utility functions (added to plot/utils.py), which reduced the length of the file significantly (compare to plot/cartopy/deprecated_lat_lon_plot.py)

(5) Derived Variables

derivations/acme.py
- Split acme.py file into derivations.py, formulas.py, and utils.py
- derivations.py -- stores derived variables dictionary.
- derivations/formulas.py -- stores formulas for deriving variables, all functions have been updated to operate on xr.DataArray objects
- utils.py -- a general file for storing utility functions (e.g,. unit conversion)
  - _apply_land_sea_mask() replaces the old mask_by() function (removed unnecessary logic and renamed for clarity)
  - TODO: Some functions have not been refactored (e.g., determine_tau())
derivations/default_regions_xr.py
- Updated structure of REGION_SPECS dict (formerly region_specs)
- Updated structure of ARM_SITE_SPECS dict (formerly points_specs)

(6) Misc.

e3sm_diags/driver/utils/general.py
- Removed the mask_by() function because it was overly complex and select_region() only uses one part of the code.
e3sm_diags/driver/utils/core_parameter.py
- Updated self.backend="mpl" to self.backend=cartopy because "mpl" gets updated to "cartopy" in e3sm_diags/plot/__init__.py anyways

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:

I have added tests that prove my fix is effective or that my feature works
New and existing unit tests pass with my changes (locally and CI/CD build)
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)

Viewer Outputs

Future Items

CDAT Migration Phase 3: Refactor utility functions and legacy code #734

e3sm_diags/driver/utils/climo.py

tomvothecoder · 2023-05-15T16:40:39Z

Hi @chengzhuzhang, I'm spending more time managing the effort and progress for this PR.

Here are a few notes:

This is a large and high priority task so we might need to pull another person in to help refactor other components if we want it merged within the next 1-2 months. xCDAT and other projects are currently taking up a lot of my bandwidth.
- Workaround -- I refactor the core driver and utilities, and somebody else refactors the plotter and viewer
I'm sinking too much time on investigating the diffs between xCDAT's climatology vs. E3SM diags climatology
- Workaround -- Refactor E3SM diags climatology to operate on xr.DataArray instead of TransientVariable. Open up a new issue to integrate xCDAT's climatology later, and attach my investigation Jupyter Notebook
I don't have a clear timeline on when Jason can work on incorporating a version of cdutil.reconstructPressureFromHybrid into cf-xarray.
- Temporary Workaround -- Create a function in e3sm_diags that is a copy of this code and convert it to xarray directly

chengzhuzhang · 2023-05-16T19:18:00Z

Hi @chengzhuzhang, I'm spending more time managing the effort and progress for this PR.

Here are a few notes:

1. This is a large and high priority task so we might need to pull another person in to help refactor other components if we want it merged within the next 1-2 months. xCDAT and other projects are currently taking up a lot of my bandwidth.
   
   * **Workaround -- I refactor the core driver and utilities, and somebody else refactors the plotter and viewer**

2. I'm sinking too much time on investigating the diffs between xCDAT's climatology vs. E3SM diags climatology
   
   * **Workaround -- Refactor E3SM diags climatology to operate on xr.DataArray instead of TransientVariable. Open up a new issue to integrate xCDAT's climatology later, and attach my investigation Jupyter Notebook**

3. I don't have a clear timeline on when Jason can work on incorporating a version of [cdutil.reconstructPressureFromHybrid](https://github.com/CDAT/cdutil/blob/b823b69db46bb76536db7d435e72075fc3975c65/cdutil/vertical.py#L8) into `cf-xarray`.
   
   * **Temporary Workaround -- Create a function in e3sm_diags that is a copy of this code and convert it to xarray directly**

Hey @tomvothecoder thank you for the thoughtful suggestions. This is a high priority PR, since we have a crunching time line for the CDAT support. The workaround for climatology and and pressure level construction sounds good to me! Both @forsyth2 and I are familiar with the plotter and viewer code, so I think distribute the refactor work to either would make sense. We can talk about how to get start splitting the work.

e3sm_diags/derivations/derivations.py

pyproject.toml

- Refactor `get_climo_variable()`, `get_static_variable()`, `_get_climo_var()`, and `_get_var_from_timeseries_file()` from using cdms2 to xarray - Add acme_new.py` with refactoreed derivation functions to operate on `xarray.DataArray`

- Remove `self.climo_fcn` and replace with direct call to `climo` - Add new `climo` function based on xarray and xcdat - Refactor `get_timeseries_variable()` and `get_climo_variable()` with cleaner conditionals, updated docstrings, and type annotations - Rename `_get_timeseries_file_path()` to `_get_timeseries_filepath()` - Add type annotations to various private methods

- Add FIXME comment in old `climo()` that breaks with "ANN" season

- Rename `climo_new.py` to `climo_xcdat.py` and add module level docstring - Add unit tests - Update references to `climo_xr.py`

- Update Dataset class methods to remove *args and **kwargs - Update Dataset class methods to return source xr.Dataset objects with the desired variables for downstream processing such as calculating climatologies - Add FIXME comments to various files

Refer to the PR for more information because the changelog is massive.

…SM-Project#677) Refer to the PR for more information because the changelog is massive.

Refer to the PR for more information because the changelog is massive.

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

…SM-Project#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 (E3SM-Project#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` (E3SM-Project#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 (E3SM-Project#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

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

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)

tomvothecoder added the refactor label Mar 17, 2023

tomvothecoder force-pushed the refactor/658-lat-lon-set branch from 8882c38 to 8ca24c3 Compare March 22, 2023 18:53

tomvothecoder commented Mar 22, 2023

View reviewed changes

e3sm_diags/driver/utils/climo.py Outdated Show resolved Hide resolved

tomvothecoder mentioned this pull request Mar 22, 2023

Climatology function breaks with season == "ANN" and drivers silently ignores errors with fallback behavior #680

Closed

tomvothecoder mentioned this pull request May 15, 2023

e3sm_diags.drivers.utils.climo is not designed for "SEASONALCYCLE" or "ANNUALCYCLE" #689

Closed

tomvothecoder force-pushed the refactor/658-lat-lon-set branch from 18e3c26 to a39c865 Compare June 23, 2023 19:48

tomvothecoder self-assigned this Jul 6, 2023

tomvothecoder added the enhancement label Jul 6, 2023

tomvothecoder force-pushed the refactor/658-lat-lon-set branch from 4f854ca to 5ca3c73 Compare July 19, 2023 00:11

tomvothecoder mentioned this pull request Jul 19, 2023

[Bug]: IndexError when reversing decreasing lev coordinates in hybrid_to_plevs() and pressure_to_plevs() #705

Open

tomvothecoder force-pushed the refactor/658-lat-lon-set branch from 1207793 to ca5795d Compare August 22, 2023 18:45

tomvothecoder mentioned this pull request Aug 24, 2023

[Bug]: Regridding time-series variable's climatology from hybrid to pressure levels breaks due to misaligned axes #721

Open

tomvothecoder force-pushed the refactor/658-lat-lon-set branch 2 times, most recently from ab41c9e to 6a2a1c9 Compare September 6, 2023 22:17

tomvothecoder changed the title ~~CDAT Migration Phase 2: Refactor lat_lon set~~ CDAT Migration Phase 2: Refactor core utilities and lat_lon set Sep 11, 2023

tomvothecoder commented Sep 11, 2023

View reviewed changes

tomvothecoder commented Sep 12, 2023

View reviewed changes

pyproject.toml Outdated Show resolved Hide resolved

tomvothecoder added 11 commits September 13, 2023 15:38

Add dataset_new.py and failing unit tests

f5af934

Add dataset_new.py with updated Dataset class

e718fc4

- Refactor `get_climo_variable()`, `get_static_variable()`, `_get_climo_var()`, and `_get_var_from_timeseries_file()` from using cdms2 to xarray - Add acme_new.py` with refactoreed derivation functions to operate on `xarray.DataArray`

Update new climo() with DataArray parsing

585a0bd

- Add FIXME comment in old `climo()` that breaks with "ANN" season

Add names to derived variable formula functions

e00ab57

Update climo() to support derived variables

51d24f7

Update refactored climo function with "ANN" averaging

fbfd472

Refactor new climatology func for cleaner code

b0bd99a

Add climo_xr.py as a temp solution

9fa2a17

- Rename `climo_new.py` to `climo_xcdat.py` and add module level docstring - Add unit tests - Update references to `climo_xr.py`

Fix conditional in climo_xcdat.py

f568174

tomvothecoder added a commit that referenced this pull request Nov 29, 2023

CDAT Migration Phase 2: Refactor core utilities and lat_lon set (#677)

272d745

Refer to the PR for more information because the changelog is massive.

tomvothecoder added a commit that referenced this pull request Nov 29, 2023

CDAT Migration Phase 2: Refactor core utilities and lat_lon set (#677)

92e795d

Refer to the PR for more information because the changelog is massive.

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

CDAT Migration Phase 2: Refactor core utilities and lat_lon set (#677)

4c9dac1

Refer to the PR for more information because the changelog is massive.

tomvothecoder added a commit to forsyth2/e3sm_diags that referenced this pull request Dec 4, 2023

CDAT Migration Phase 2: Refactor core utilities and lat_lon set (E3…

f717301

…SM-Project#677) Refer to the PR for more information because the changelog is massive.

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

CDAT Migration Phase 2: Refactor core utilities and lat_lon set (#677)

14308c4

Refer to the PR for more information because the changelog is massive.

tomvothecoder mentioned this pull request Dec 5, 2023

Regression testing for lat_lon variables NET_FLUX_SRF and RESTOM #754

Merged

9 tasks

This was referenced Dec 18, 2023

Diff of zonal mean xy refactoring forsyth2/e3sm_diags#4

Closed

CDAT Migration: Refactor polar set #749

Merged

forsyth2 mentioned this pull request Mar 29, 2024

Refactor area_mean_time_series #750

Merged

21 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CDAT Migration Phase 2: Refactor core utilities and `lat_lon` set#677

CDAT Migration Phase 2: Refactor core utilities and `lat_lon` set#677
tomvothecoder merged 110 commits intocdat-migration-fy24from
refactor/658-lat-lon-set

tomvothecoder commented Mar 17, 2023 •

edited

Loading

Uh oh!

Uh oh!

tomvothecoder commented May 15, 2023 •

edited

Loading

Uh oh!

chengzhuzhang commented May 16, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	def _subset_on_region(ds: xr.Dataset, var_key: str, region: str) -> xr.Dataset:
	"""Subset a variable in the dataset based on the region.

	Parameters
	----------
	ds : xr.Dataset
	The dataset.
	var_key : str
	The variable to subset.
	region : str
	The region

	Returns
	-------
	xr.Dataset
	The dataest with the subsetted variable.

	Notes
	-----
	Replaces `e3sm_diags.utils.general.select_region`.
	"""

Conversation

tomvothecoder commented Mar 17, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

How to Review

Main Focus Areas

(1) Dataset class

(2) Computation, Regridding, and I/O Utilities

(3) Lat Lon Driver

(4) Lat Lon Plotter

(5) Derived Variables

(6) Misc.

Checklist

Viewer Outputs

Future Items

Uh oh!

Uh oh!

tomvothecoder commented May 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chengzhuzhang commented May 16, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

tomvothecoder commented Mar 17, 2023 •

edited

Loading

tomvothecoder commented May 15, 2023 •

edited

Loading