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

ij index fix for issue #531 #549

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Conversation

grantfirl
Copy link
Collaborator

@grantfirl grantfirl commented Jan 7, 2025

  • reinstate functionality to give UFS_case_gen.py an i,j index pair along with tile number to generate a case
  • add ability to provide UFS_forcing_ensemble_generator.py with a list of i,j indices to generate multiple cases using the i,j,tile paradigm
  • update documentation to reflect reinstated capability

fixes #531

@grantfirl
Copy link
Collaborator Author

Note: Still need to test UFS_forcing_ensemble_generator.py functionality on Hera.

@egrell
Copy link
Collaborator

egrell commented Jan 30, 2025

@grantfirl
I tried the -ij option to extract the forcing for my selected points. While the code ran successfully, it didn't give me the locations that I wanted.
I apologize if I perhaps wasn't clear in describing my use of the option.
I would like to choose the points based on what I can see in my model (SRW) output, ie via ncview, which makes it easy to identify the points. The problem, I think, is that the case_gen script is first finding the points in the $grid_dir/grid.tile{0}.halo{1}.nc file, which is on the supergrid, not on the model grid.
I see that the code includes an accounting for the halo (I have n_lam_halo_points = 3, the default), and extraction of every other point, but it still isn't coming out quite right.

I wonder if either:
a) the code could be re-organized to call find_loc_indices_UFS_history first, and set the location from the history files rather than the input files, (I think this was done previously?) or
b) Do I have some settings wrong regarding the INPUT grid? My model output has dimensions: grid_xt = 1000 ; grid_yt = 780 ; but the INPUT/grid.tile7.halo3.nc file has dimensions 2060 x 1612. These settings are all defaults. I don't quite see how subtracting the halo2 and taking every other point gets me to the matching grid.

@grantfirl
Copy link
Collaborator Author

@grantfirl I tried the -ij option to extract the forcing for my selected points. While the code ran successfully, it didn't give me the locations that I wanted. I apologize if I perhaps wasn't clear in describing my use of the option. I would like to choose the points based on what I can see in my model (SRW) output, ie via ncview, which makes it easy to identify the points. The problem, I think, is that the case_gen script is first finding the points in the $grid_dir/grid.tile{0}.halo{1}.nc file, which is on the supergrid, not on the model grid. I see that the code includes an accounting for the halo (I have n_lam_halo_points = 3, the default), and extraction of every other point, but it still isn't coming out quite right.

I wonder if either: a) the code could be re-organized to call find_loc_indices_UFS_history first, and set the location from the history files rather than the input files, (I think this was done previously?) or b) Do I have some settings wrong regarding the INPUT grid? My model output has dimensions: grid_xt = 1000 ; grid_yt = 780 ; but the INPUT/_grid.tile7.halo3.nc file has dimensions 2060 x 1612. These settings are all defaults. I don't quite see how subtracting the halo_2 and taking every other point gets me to the matching grid.

@egrell Do you have any of the data saved somewhere on Hera that I could copy and test with to try to understand better what is going wrong?

From your description, it sounds like what was intended for the -ij option just isn't what you're really looking for. This option is supposed to correspond to the native grid, not the output grid. We could probably add yet another option like "-ij_history" or something to do what you need.

Nevertheless, I'd like to use some of your data to see if the algorithm is doing what was intended. It's certainly possible that something has changed in the SRW grid configuration to make the code wrong (or maybe it was never right - always possible).

@grantfirl
Copy link
Collaborator Author

@egrell @hertneky As far as I can tell, the code is still doing what is intended. Providing the i and j indices correspond to the FV3 native grid, and this appears to still be correct. The script is looking in CXXXX_grid.tile7.halo3.nc to read in the supergrid, including halo points. Halo points are subtracted and every other point is kept, giving one the native grid. This all appears to be correct, as long as one sets n_halo_points in the script to correspond to the available CXXXX_grid.tile7.haloY.nc file correctly.

Note that the native grid need not have much of a relationship to the output file grid (see https://ufs-weather-model.readthedocs.io/en/develop/InputsOutputs.html#output-files). The write component takes the output on the native grid and writes the output files how they are specified in model_configure. For the given data from @egrell, the write component produced files of size (1000, 780) with the following parameters (these are global attributes in the dynf/phyfXXX.nc files):

            :cen_lat = 39.5f ;
            :cen_lon = -83.f ;
            :dx = 3000.f ;
            :dy = 3000.f ;
            :grid = "lambert_conformal" ;
            :grid_id = 1LL ;
            :hydrostatic = "non-hydrostatic" ;
            :lat1 = 27.5f ;
            :lon1 = -98.f ;
            :ncnsto = 10 ;
            :nx = 1000 ;
            :ny = 780 ;
            :source = "FV3GFS" ;
            :stdlat1 = 39.5f ;
            :stdlat2 = 39.5f ;

This grid is centered on the same point as the native grid, but uses a different projection. It also removes 20 points in the y direction and 24 points in the x direction, probably to not output data with boundary effects(?). Note that it is possible to configure the write component to write out the FV3 native grid, then the i,j indices should correspond to one-to-one to the native grid calculated from CXXXX_grid.tile7.haloY.nc, but this is not the default.

If what is desired is to take indices from the files written from the write component (currently given names of dynf/phyfXXX.nc), translate those indices into lat/lon, then grab the closest point on the native grid, that is also possible, but it is basically the reverse of what is currently being done, and this doesn't really constitute a bug fix, but an added feature.

@hertneky I've now tested the UFS_forcing_ensemble_generator.py addition and it works on Hera after a couple of bug fixes. This should be ready to review/approve/merge for the bug fix release.

@grantfirl grantfirl marked this pull request as ready for review February 26, 2025 20:01
@grantfirl
Copy link
Collaborator Author

Note: Still need to test UFS_forcing_ensemble_generator.py functionality on Hera.

This has been done. It works fine as long as #563 is used.

Copy link
Collaborator

@hertneky hertneky left a comment

Choose a reason for hiding this comment

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

Code changes look good. Tests provided expected results, checking that the correct point was extracted from the native grid. SCM from both produced data files ran fine.

Tested both UFS_case_gen.py:
./UFS_case_gen.py -ij 24 24 -t 4 -i /scratch1/BMC/gmtb/Dustin.Swales/SCM/replay/rt_3252959/control_c48_intel/INPUT/ -g /scratch1/BMC/gmtb/Dustin.Swales/SCM/replay/rt_3252959/control_c48_intel/INPUT/ -f /scratch1/BMC/gmtb/Dustin.Swales/SCM/replay/rt_3252959/control_c48_intel -n control_c48_intel_C_test_n000 -sc -fm 2 -vm 2

And ./UFS_forcing_ensemble_generator.py:
./UFS_forcing_ensemble_generator.py -d /scratch1/BMC/gmtb/Dustin.Swales/SCM/replay/rt_3252959/control_c48_intel -sc --C_RES 48 -dt 1200 -n control_c48_intel_C_test -sdf SCM_GFS_v17_p8 -tile 4 -is 24 -js 24

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

casegen error when supplying ij indices
3 participants