Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix several issues in file regridding; Also prevent double-flipping of vertical indices #270

Merged
merged 7 commits into from
Oct 24, 2023

Conversation

yantosca
Copy link
Contributor

@yantosca yantosca commented Oct 23, 2023

Overview

This PR fixes several remaining issues with GCPy file regridding using online weights.

  1. Routine rename_and_flip_gchp_rst_vars.py now uses inquiry function is_cubed_sphere_rst_grid internally. If the data is on the GCHP checkpoint/restart grid, then data variables will be renamed according to GCHP conventions and vertical levels will be flipped so that it is positive down. Otherwise the original data will be returned unchanged. This was necessary to prevent a bug in which vertical levels were flipped twice in compare_diags.py and the example plotting scripts plot_comparisons.py and plot_single_level.py.

  2. Added the boolean argument gchp_indices to routines get_ilev_coords and get_lev_coords. If true, then the coordinates will be returned as indices (e.g. 1..72), which is used for the GCHP level dimension.

  3. Fixed several logic issues in file_regrid.py:
    a. Move flip_lev_coord_if_necessary so that it occurs before variable renaming/regridding. This prevents the flipping algorithm from being spoofed by renamed variables.
    b. Renamed drop_and_rename_classic_variables to drop_classic_vars, which only drops the non-regriddable variables.
    c. Move variable renaming into rename_restart_variables. Also improve logic so that variables are renamed properly
    d. Updated logic in the flip_lev_coord_if_necessary routine. Also make sure to save coordinates in the proper order. GCHP level coordinates are now saved as indices.

  4. Fixed issues with command-line arguments:
    a. Renamed Python argument fin to filein, to match the command-line argument
    b. Renamed Python argument fout to fileout, to match the command-line argument
    c. Added verbose printout of arguments
    d. Remove nargs=1 from weightsdir and verbose, which turns them into a list of 1 element (instead of a scalar, which is what it should be)

Output from file_regrid.py is shown below.

TODO: Double-check the Regridding.rst instructions to make sure they are consistent with this PR. Further documentation fixes will be pushed to docs/dev.

Common use cases

GCClassic to GCHP checkpoint

gcc2chk

GCClassic to GCClassic

gcc2gcc

GCHP checkpoint to GCHP checkpoint

chk2chk

GCHP checkpoint to GCHP checkpoint, stretched grid:

NOTE: Not sure if this is a plotting issue or a regridding issue.
chk2str

GCHP checkpoint to GCClassic

chk2gcc

Less common use cases (shown for completeness)

I wanted to show these cases, just to show that the regridding seems to work OK. Some editing of variable names may still be needed. In any case, these are not the usual use case so I think it's OK.

GCHP checkpoint to GCHP diagnostic

chk2dgn

GCHP diagnostic to GCHP checkpoint

dgn2chk

GCHP diagnostic to GCHP diagnostic

dgn2dgn

GCClassic to GCHP diagnostic

gcc2dgn

GCHP diagnostic to GCClassic

dgn2gcc

gcpy/file_regrid.py
- Renamed "fin" to "filein", to match the command-line argument
- Renamed "fout" to "fileout", to match the command-line argument
- Added verbose printout of arguments
- Remove nargs=1 from weightsdir and verbose, which turns them into
  a list of 1 element

Signed-off-by: Bob Yantosca <yantosca@seas.harvard.edu>
gcpy/file_regrid.py
- Removed a double-flipping of vertical levels in LL -> CS/SG regridding

gcpy/regrid_restart_file.py
- Removed function check_lev_attribute, it is not defined

Signed-off-by: Bob Yantosca <yantosca@seas.harvard.edu>
gcpy/file_regrid.py
- Removed several debug printout statements at about line 1333

Signed-off-by: Bob Yantosca <yantosca@seas.harvard.edu>
gcpy/util.py
- Routine "rename_and_flip_gchp_rst_vars" now uses the inquiry function
  "is_cubed_sphere_rst_grid" from cstools.py to determine if the dataset
  is from a GCHP restart file.  If not, the original dataset will be
  returned unchanged.
- Updated Pydoc and comments
- Updated algorithm in "rename_and_flip_gchp_rst_vars" so that we create
  a dictionary of variable name replacements so that the replacement
  can be done as a single operation.  This is more efficient.
- If we have flipped the dataset, also set lev.positive="up"

gcpy/benchmark_funcs.py
gcpy/examples/diagnostics/compare_diags.py
gcpy/examples/plotting/plot_comparisons.py
gcpy/examples/plotting/plot_single_panel.py
- Remove external tests for the GCHP restart grid before calling the
  "rename_and_flip_gchp_rst_vars" function from util.py.  The function
  can now detemrine internally whether or not the passed dataset is
  from a GCHP restart file.  If not the original data will be returned
  unchnaged.
- Updated comments for clarity.

Signed-off-by: Bob Yantosca <yantosca@seas.harvard.edu>
gcpy/grid.py
- Remove unused import statement
- Added "gchp_indices" argument to get_lev_coords and get_ilev_coords
  functions.  If gchp_indices=True, the lev/ilev coordinates will be
  returned as an array of indices (as is the custom for GCHP netCDF
  files).
- Updated comments and Pydoc

Signed-off-by: Bob Yantosca <yantosca@seas.harvard.edu>
gcpy_file_regrid.py
- Move flip_lev_coord_if_necessary so that it occurs before
  variable renaming/regridding.  This prevents the flipping algorithm
  from being spoofed by renamed variables.
- Renamed "drop_and_rename_classic_variables" to "drop_classic_vars",
  which only drops the non-regriddable variables.
- Move variable renaming into "rename_restart_variables".  Also improve
  logic so that variables are renamed properly
- Updated logic in the flip_lev_coord_if_necessary routine.  Also
  make sure to save coordinates in the proper order.  GCHP level
  coordinates are now saved as indices.
- Updated Pydoc and comments.
- Trimmed trailing whitespace.

gcpy/util.py
- Initialize "old_to_new" to a blank dictionary object

Signed-off-by: Bob Yantosca <yantosca@seas.harvard.edu>
@yantosca yantosca added topic: Benchmark Plots and Tables Issues pertaining to generating plots/tables from benchmark output topic: Regridding Issues pertaining to horizontal & vertical regridding category: Bug Fix Fixes a bug that was previously reported labels Oct 23, 2023
@yantosca yantosca added this to the 1.4.0 milestone Oct 23, 2023
@yantosca yantosca self-assigned this Oct 23, 2023
Copy link
Contributor

@lizziel lizziel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This all looks good to me except for the stretched grid. I suggest we do not allow stretched grid regridding using online weights until it is resolved. Using offline weights works so we can stick with that.

@lizziel
Copy link
Contributor

lizziel commented Oct 24, 2023

I am using this to create a GCHP restart from GC-Classic and noticed this output 6 times:
using dimensions ('lat', 'lon') from data variable TropLev as the horizontal dimensions for this dataset.

Is this needed? Could we suppress it?

gcpy/file_regrid.py
- Because there is an issue in regridding to stretched-grids,
  we now throw a RuntimeError if sg_params_in or sg_params_out
  does not match the default (i.e. non-stretched) values.

Signed-off-by: Bob Yantosca <yantosca@seas.harvard.edu>
@yantosca
Copy link
Contributor Author

As suggested by @lizziel, we now prevent regridding to/from stretched-grids for the time being, as the stretched-grid plots above indicate some sort of error (which may be either regridding or plotting). Revisit this sometime in the future.

The error looks like:

Traceback (most recent call last):
  File "/n/holyscratch01/jacob_lab/ryantosca/data/Restarts_Out/./rst.py", line 216, in <module>
    main()
  File "/n/holyscratch01/jacob_lab/ryantosca/data/Restarts_Out/./rst.py", line 155, in main
    file_regrid(
  File "/n/home09/ryantosca/python/gcpy/gcpy/file_regrid.py", line 111, in file_regrid
    raise RuntimeError(msg)
RuntimeError: Regridding to or from cubed-sphere stretched grids is
currently not supported.  Please use the offline regridding
method described in the Regridding section of gcpy.readthedocs.io.

@yantosca
Copy link
Contributor Author

I am using this to create a GCHP restart from GC-Classic and noticed this output 6 times: using dimensions ('lat', 'lon') from data variable TropLev as the horizontal dimensions for this dataset.

Is this needed? Could we suppress it?

Thanks @lizziel. I think that is coming in from ESMF. I am not sure how to suppress it. It doesn't appear to be generated by xESMF (at least as far as I can tell from looking through the source code).

@yantosca yantosca requested a review from lizziel October 24, 2023 15:46
Copy link
Contributor

@lizziel lizziel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Look good!

@yantosca
Copy link
Contributor Author

Update: the "Using dimensions..." warnings is in xESMF (see frontend.py). I have tried to disable the warnings but somehow they still come up.

@yantosca yantosca merged commit 90b3b68 into dev Oct 24, 2023
2 of 5 checks passed
@yantosca yantosca deleted the bugfix/file-regrid branch October 24, 2023 17:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: Bug Fix Fixes a bug that was previously reported topic: Benchmark Plots and Tables Issues pertaining to generating plots/tables from benchmark output topic: Regridding Issues pertaining to horizontal & vertical regridding
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants