CDAT migration: Fix African easterly wave density plots in TC analysis and convert H20LNZ units to ppm/volume by tomvothecoder · Pull Request #882 · E3SM-Project/e3sm_diags

tomvothecoder · 2024-10-29T18:57:39Z

Description

Related to Fixing African easterly wave density plots in TC analysis #851 -- looks like I already addressed this on cdat-migration-fy24 but I refactored it a bit more
Related to Convert H2OLNZ units to ppm by volume #874
Relatedt to [Refactor]: CDAT Migration Phase 3 - Port changes from main to cdat-migration-fy24 #871

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

Hey @chengzhuzhang this PR is ready for review. I've performed a self-review, but good to make sure the changes align with your two PRs mentioned in this PR's description.

Summary of changes

tc_analysis_driver.py
- Remove lat/lon slice
- Add function _filter_lines_within_year_bounds() to remove excessive time points crossing year bounds for 6 hourly data
zonal_mean_2d_driver.py
- Update _convert_g_kg_to_ppm_units() to convert to ppm by volume.

tomvothecoder · 2024-10-29T18:59:02Z

e3sm_diags/driver/zonal_mean_2d_driver.py

Changes should align with https://github.com/E3SM-Project/e3sm_diags/pull/874/files#diff-794c0344b0a718e616d2051d6961d02c79f24e8d5e97c7c355fa7cb0ec467e19

tomvothecoder · 2024-10-29T18:59:50Z

e3sm_diags/driver/tc_analysis_driver.py

Changes should align with https://github.com/E3SM-Project/e3sm_diags/pull/851/files

Note, #872 was already addressed on this branch.

tomvothecoder · 2024-10-29T19:05:11Z

e3sm_diags/driver/tc_analysis_driver.py

+def _filter_lines_within_year_bounds(
+    lines_orig: List[str], data_end_year: int
+) -> List[str]:
+    """Filters lines within the specified year bounds.
+
+    This function processes a list of strings, each representing a line of data.
+    It filters out lines based on a year extracted from each line, ensuring that
+    only lines with years less than or equal to `data_end_year` are retained.
+    Additionally, it removes excessive time points crossing year bounds from
+    6-hourly data.
+
+    Parameters
+    ----------
+    lines_orig : List[str]
+        A list of strings where each string represents a line of data.
+    data_end_year : int
+        The end year for filtering lines. Only lines with years less than or
+        equal to this value will be retained.
+    Returns
+    -------
+    List[str]
+        A list of strings filtered based on the specified year bounds.
+    """
+    line_ind = []
+    for i in range(0, np.size(lines_orig)):
+        if lines_orig[i][0] == "s":
+            year = int(lines_orig[i].split("\t")[2])
+
+            if year <= data_end_year:
+                line_ind.append(i)
+
+    end_ind = line_ind[-1]
+
+    new_lines = lines_orig[0:end_ind]
+    return new_lines


FYI I used GitHub Co-Pilot to extract this function and generated the docstring. I've found it to be useful for these kinds of tasks.

Nice finding!

e3sm_diags/driver/zonal_mean_2d_driver.py

chengzhuzhang

@tomvothecoder I think the changes in .cfg files for H2OLNZ units change are not brought in this PR: https://github.com/E3SM-Project/e3sm_diags/pull/874/files#diff-794c0344b0a718e616d2051d6961d02c79f24e8d5e97c7c355fa7cb0ec467e19

tomvothecoder · 2024-10-29T20:46:39Z

@tomvothecoder I think the changes in .cfg files for H2OLNZ units change are not brought in this PR: #874 (files)

For that specific PR, the changes are already here. Example:

e3sm_diags/e3sm_diags/driver/default_diags/zonal_mean_2d_stratosphere_model_vs_obs.cfg

Lines 65 to 75 in 3df16d2

    
           [#] 
        
           sets = ["zonal_mean_2d_stratosphere"] 
        
           case_id = "MERRA2 (relative difference)" 
        
           variables = ["H2OLNZ"] 
        
           ref_name = "MERRA2" 
        
           reference_name = "MERRA2 Reanalysis" 
        
           seasons = ["ANN", "01", "02", "03", "04", "05", "06", "07", "08", "09", "10", "11", "12", "DJF", "MAM", "JJA", "SON"] 
        
           diff_type = "relative" 
        
           diff_title = "(Model - Obs.)/Obs. * 100" 
        
           contour_levels = [0.5,1,1.5,2,2.5,3,3.5,4,4.5,5,5.5,6,7,8] 
        
           diff_levels = [-100, -75, -50, -30, -15, -5, 5, 15, 30, 50, 75, 100]

I rebased the dev branch on the latest main, which pulls in changes to .cfg files. I don't need to update any .cfg files in this PR.

e3sm_diags/driver/zonal_mean_2d_driver.py

chengzhuzhang

Looking good! The PR is good to go!

chengzhuzhang · 2024-10-29T20:56:34Z

@tomvothecoder Just a heads-up that this PR is also just merged: #879 and we need to bring to migration branch..

…s and convert H20LNZ units to ppm/volume (#882)

tomvothecoder · 2024-10-29T21:18:41Z

@tomvothecoder Just a heads-up that this PR is also just merged: #879 and we need to bring to migration branch..

Done

…s and convert H20LNZ units to ppm/volume (#882)

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 2 commits October 29, 2024 11:54

Fix African easterly wave density plots in TC analysis

e11641e

Convert H20LNZ units to ppm by volume

3df16d2

tomvothecoder changed the title ~~Fix African easterly wave density plots in TC analysis~~ CDAT migration: Fix African easterly wave density plots in TC analysis and Convert H20LNZ units to ppm/volume Oct 29, 2024

tomvothecoder changed the title ~~CDAT migration: Fix African easterly wave density plots in TC analysis and Convert H20LNZ units to ppm/volume~~ CDAT migration: Fix African easterly wave density plots in TC analysis and convert H20LNZ units to ppm/volume Oct 29, 2024

tomvothecoder commented Oct 29, 2024

View reviewed changes

tomvothecoder marked this pull request as ready for review October 29, 2024 19:02

tomvothecoder added the cdat-migration-fy24 CDAT Migration FY24 Task label Oct 29, 2024

tomvothecoder added this to the FY25 Q1 (10/1/24 - 12/31/24) milestone Oct 29, 2024

tomvothecoder self-assigned this Oct 29, 2024

tomvothecoder requested a review from chengzhuzhang October 29, 2024 19:03

tomvothecoder commented Oct 29, 2024

View reviewed changes

tomvothecoder mentioned this pull request Oct 29, 2024

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

Closed

chengzhuzhang reviewed Oct 29, 2024

View reviewed changes

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

chengzhuzhang requested changes Oct 29, 2024

View reviewed changes

tomvothecoder commented Oct 29, 2024

View reviewed changes

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

Update e3sm_diags/driver/zonal_mean_2d_driver.py

e9217f1

chengzhuzhang approved these changes Oct 29, 2024

View reviewed changes

tomvothecoder merged commit d5a1aad into cdat-migration-fy24 Oct 29, 2024

tomvothecoder deleted the feature/851-874 branch October 29, 2024 21:17

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

CDAT migration: Fix African easterly wave density plots in TC analysi…

7550b3d

…s and convert H20LNZ units to ppm/volume (#882)

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

CDAT migration: Fix African easterly wave density plots in TC analysi…

18639fd

…s and convert H20LNZ units to ppm/volume (#882)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CDAT migration: Fix African easterly wave density plots in TC analysis and convert H20LNZ units to ppm/volume#882

CDAT migration: Fix African easterly wave density plots in TC analysis and convert H20LNZ units to ppm/volume#882
tomvothecoder merged 3 commits intocdat-migration-fy24from
feature/851-874

tomvothecoder commented Oct 29, 2024 •

edited

Loading

Uh oh!

tomvothecoder left a comment •

edited

Loading

Uh oh!

tomvothecoder Oct 29, 2024

Uh oh!

tomvothecoder Oct 29, 2024

Uh oh!

tomvothecoder Oct 29, 2024

Uh oh!

chengzhuzhang Oct 29, 2024

Uh oh!

Uh oh!

chengzhuzhang left a comment

Uh oh!

tomvothecoder commented Oct 29, 2024

Uh oh!

Uh oh!

chengzhuzhang left a comment

Uh oh!

chengzhuzhang commented Oct 29, 2024

Uh oh!

tomvothecoder commented Oct 29, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

tomvothecoder commented Oct 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Uh oh!

tomvothecoder left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tomvothecoder Oct 29, 2024

Choose a reason for hiding this comment

Uh oh!

tomvothecoder Oct 29, 2024

Choose a reason for hiding this comment

Uh oh!

tomvothecoder Oct 29, 2024

Choose a reason for hiding this comment

Uh oh!

chengzhuzhang Oct 29, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

chengzhuzhang left a comment

Choose a reason for hiding this comment

Uh oh!

tomvothecoder commented Oct 29, 2024

Uh oh!

Uh oh!

chengzhuzhang left a comment

Choose a reason for hiding this comment

Uh oh!

chengzhuzhang commented Oct 29, 2024

Uh oh!

tomvothecoder commented Oct 29, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

tomvothecoder commented Oct 29, 2024 •

edited

Loading

tomvothecoder left a comment •

edited

Loading