Feature 2857 tripolar coordinates

hsoh-u commented 1 week ago

Expected Differences

[x] Do these changes introduce new tools, command line arguments, or configuration file options? [No]
If yes, please describe:
[x] Do these changes modify the structure of existing or add new output data types (e.g. statistic line types or NetCDF variables)? [No]
If yes, please describe:

Pull Request Testing

[x] Describe testing already performed for these changes:

The first issue is not finding lat/lon variables as the point data. The second issue is overriding the lat/lon variables from the variable's coordinates attribute. The third issue is supporting different lat/lon variables on selecting multiple variables. The fourth issue is overriding lat/lon variables when the variable's coordinate variable does not exist. The lat/lon variables can be specified by using the configuration file.

/d1/personal/jopatz/workbench/tripolar/CICE_fcst/iceh.2018-01-03.c00.nc has multiple lat/lon variables. hs_d uses TLON and TLAT variables and uvel_d uses ULON ULAT. The content of /d1/personal/jopatz/workbench/tripolar/CICE_fcst/iceh.2018-01-03.c00.nc:

        float hi_d(time, nj, ni) ;
                hi_d:units = "m" ;
                hi_d:long_name = "grid cell mean ice thickness" ;
                hi_d:coordinates = "TLON TLAT time" ;
        float uvel_d(time, nj, ni) ;
                uvel_d:units = "m/s" ;
                uvel_d:long_name = "ice velocity (x)" ;
                uvel_d:coordinates = "ULON ULAT time" ;

The first problem was this:

/nrit/ral/met/bin/point2grid /d1/personal/jopatz/workbench/tripolar/CICE_fcst/iceh.2018-01-03.c00.nc G231 cice3_to_G231.nc -field 'name="hi_d"; level="(0,*,*)";'
ERROR  :
ERROR  : get_data_from_lat_lon_vars() -> MET can only process Latitude/Longitude files where the latitudes are evenly spaced (dlat=0.0631027, delta[25]=0.0598373)
ERROR  :
ERROR  : get_data_from_lat_lon_vars() -> Please check the input data is the lat/lon projection
ERROR  :

The first and the second issue:

/d1/personal/hsoh/git/pull_request/MET_feature_2857_tripolar_coordinates/bin/point2grid  /d1/personal/jopatz/workbench/tripolar/CICE_fcst/iceh.2018-01-03.c00.nc G231 cice_to_G231.nc -field 'name="hi_d"; level="(0,*,*)";' -v 7 >& log_point2grid.log

grep -E "get_nc_var_l|coordinate variables|process_point_nccf_file" log_point2grid.log
DEBUG 6: get_nc_var_lat() found the latitude variable "TLAT"
DEBUG 6: get_nc_var_lon() found the longitude variable "TLON"
DEBUG 5: NcCfFile::open() -> coordinate variables: x=ULON, y=ULAT, z=N/A, t=time
DEBUG 5: process_point_nccf_file() -> Cell mapping from TLAT and TLON variables
DEBUG 6: process_point_nccf_file() -> took 0.553494 seconds

point2grid finds the lat/lon variables from the target variable's lat/lon variables form hi_d:coordinates = "TLON TLAT time" ;. MET library picks ULON and ULAT variables as the lat/lon projection (NcCfFile::open() above). point2grid replaces lat/lon variables during handling hi_d variable.

Third issue: Processing two variables with different lat/lon variables:

/d1/personal/hsoh/git/pull_request/MET_feature_2857_tripolar_coordinates/bin/point2grid /d1/personal/jopatz/workbench/tripolar/CICE_fcst/iceh.2018-01-03.c00.nc G231 cice3_to_G231.nc -field 'name="uvel_d"; level="(0,*,*)";' -field 'name="hi_d"; level="(0,*,*)";' -v 7 >& log_point2grid2.log

grep -i "cell mapping" log_point2grid2.log
DEBUG 5: process_point_nccf_file() -> Cell mapping from ULAT and ULON variables
DEBUG 4: process_point_nccf_file() -> Override cell mapping from TLAT and TLON

Fourth issue: override lat/lon variables by using configuration file. Note: This is only for testing in order to make sure overriding the lat/lon variables with configuration file.

cat  Point2GridConfig_User
var_name_map = [
  { key = "lat_vname";     val = "NLAT"; },
  { key = "lon_vname";     val = "NLON"; }
];

/d1/personal/hsoh/git/pull_request/MET_feature_2857_tripolar_coordinates/bin/point2grid /d1/personal/jopatz/workbench/tripolar/CICE_fcst/iceh.2018-01-03.c00.nc G231 cice4_to_G231.nc -field 'name="uvel_d"; level="(0,*,*)";' -config Point2GridConfig_User -v 7 >& log_point2grid4.log

DEBUG 6: get_nc_var_lat() found the latitude variable "TLAT"
DEBUG 6: get_nc_var_lon() found the longitude variable "TLON"
DEBUG 5: NcCfFile::open() -> coordinate variables: x=ULON, y=ULAT, z=N/A, t=time
DEBUG 6: get_nc_data(NcVar *, float *) took 0.036533 seconds for NLAT
DEBUG 6: get_nc_data(NcVar *, float *) took 0.02716 seconds for NLON

TLAT/TLON from the variable's coordinates attribute MET library paicks ULON and ULAT as the file's grid information. NLAT and NLON from the configuration file are applied for cell mapping generation.

[x] Recommend testing for the reviewer(s) to perform, including the location of input datasets, and any additional instructions:

The build is available at /d1/personal/hsoh/git/pull_request/MET_feature_2857_tripolar_coordinates. Tests with other tri-polar data.

[x] Do these changes include sufficient documentation updates, ensuring that no errors or warnings exist in the build of the documentation? [Yes]
[x] Do these changes include sufficient testing updates? [Yes]

Added two unit tests:

point2grid_sea_ice_tripolar: processing two variables with different lat/lon variables
point2grid_sea_ice_tripolar_config: the lat/lon variable names are configured at the configuration file (-config &CONFIG_DIR;/Point2GridConfig_lat_lon
[x] Will this PR result in changes to the MET test suite? [Yes]
If yes, describe the new output and/or changes to the existing output:

Two new output files: point2grid/point2grid_sea_ice.nc and point2grid/point2grid_sea_ice_snow.nc

[x] Will this PR result in changes to existing METplus Use Cases? [No]
If yes, create a new Update Truth METplus issue to describe them.
[x] Do these changes introduce new SonarQube findings? [No]
If yes, please describe:
[ ] Please complete this pull request review by [6/27/2024].

Pull Request Checklist

See the METplus Workflow for details.

[ ] Review the source issue metadata (required labels, projects, and milestone).
[ ] Complete the PR definition above.
[ ] Ensure the PR title matches the feature or bugfix branch name.
[ ] Define the PR metadata, as permissions allow. Select: Reviewer(s) and Development issue Select: Milestone as the version that will include these changes Select: Coordinated METplus-X.Y Support project for bugfix releases or MET-X.Y.Z Development project for official releases
[ ] After submitting the PR, select the :gear: icon in the Development section of the right hand sidebar. Search for the issue that this PR will close and select it, if it is not already selected.
[ ] After the PR is approved, merge your changes. If permissions do not allow this, request that the reviewer do the merge.
[ ] Close the linked issue and delete your feature or bugfix branch from GitHub.

jprestop commented 1 week ago

@hsoh-u I'm still looking at this and testing with files on seneca. I was thinking it may be a good idea to add some additional unit tests for these cases. I know that would add some additional work and time and I'm not sure what your availability is for that. What are your thoughts on adding unit tests for the various tripolar data? Apologies if this has already been discussed and I missed it.

jprestop commented 1 week ago

@j-opatz I should have tagged you in my last message for your opinion as well:

@hsoh-u I'm still looking at this and testing with files on seneca. I was thinking it may be a good idea to add some additional unit tests for these cases. I know that would add some additional work and time and I'm not sure what your availability is for that. What are your thoughts on adding unit tests for the various tripolar data? Apologies if this has already been discussed and I missed it.

jprestop commented 1 week ago

Also @j-opatz I'd like to test the code with the GFSv17_ocean data located at seneca:/d1/projects/MET/MET_issues/feature_2857/GFSv17_ocean, but I'm unsure of the command line to use to test with. Can you please provide the command line test you used to test the GFS-v17_ocean data in this comment? I reviewed the header file for the NetCDF data, but I'm just not familiar enough with this to know what values I should be using.

hsoh-u commented 1 week ago

Added two unit tests:

point2grid_sea_ice_tripolar: processing two variables with different lat/lon variables
point2grid_sea_ice_tripolar_config: the lat/lon variable names are configured at the configuration file (-config &CONFIG_DIR;/Point2GridConfig_lat_lon. Two more output files: point2grid/point2grid_sea_ice.nc and point2grid/point2grid_sea_ice_snow.nc

jprestop commented 1 week ago

@hsoh-u Thank you for adding the unit tests. I just need some help from @j-opatz so I can test with the other data. Then, I can approve this PR.

j-opatz commented 1 week ago

@jprestop I've gone ahead and tested against the branch @hsoh-u compiled on Seneca (/d1/personal/hsoh/git/pull_request/MET_feature_2857_tripolar_coordinates) using two commands, testing 2 separate datasets:

/d1/personal/hsoh/git/pull_request/MET_feature_2857_tripolar_coordinates/bin/point2grid /d1/projects/MET/MET_issues/feature_2857/GFSv17_ocean/gfs.ocean.t00z.6hr_avg.f006.nc "stereo 304 448 33.92 279.26 135.0 25 6367.47 70.0 N" testing_GFSocean.nc -field 'name="SST"; level="(0,*,*)";'

/d1/personal/hsoh/git/pull_request/MET_feature_2857_tripolar_coordinates/bin/point2grid /d1/projects/MET/MET_issues/feature_2857/RTOFS/rtofs_glo_2ds_f006_ice.nc "stereo 304 448 33.92 279.26 135.0 25 6367.47 70.0 N" testing_RTOFS.nc -field 'name="ice_coverage"; level="(0,*,*)";'

And was puzzled to find BOTH commands ended with an error.

The first command tests against GFSv17 ocean data, which previously also errored out when using point2grid. The error for that command is

ERROR  : 
ERROR  : process_data_file() -> "/d1/projects/MET/MET_issues/feature_2857/GFSv17_ocean/gfs.ocean.t00z.6hr_avg.f006.nc" not a valid data file
ERROR  :

I believe this is the same error it showed before this PR.

The second command tests against RTOFS data, which previous worked with Point2Grid. It now errors out with this message

ERROR  : 
ERROR  : get_data_from_lat_lon_vars() -> MET can only process Latitude/Longitude files where the latitudes are evenly spaced (dlat=0.0319977, delta[421]=0.0336151)
ERROR  : 
ERROR  : 
ERROR  : get_data_from_lat_lon_vars() -> Please check the input data is the lat/lon projection
ERROR  :

This seems to be a step back from where point2grid previously was, since it worked with MET before. I confirmed my memory by testing the RTOFS dataset against the Nightly Build of MET with the command

 /d1/projects/MET/MET_regression/develop/NB20240627/MET-develop/bin/point2grid /d1/projects/MET/MET_issues/feature_2857/RTOFS/rtofs_glo_2ds_f006_ice.nc "stereo 304 448 33.92 279.26 135.0 25 6367.47 70.0 N" testing_RTOFS.nc -field 'name="ice_coverage"; level="(0,*,*)";'

And completed a successful run. I'm not sure what's happening to cause this discrepancy, but we need to discuss what commands you're currently using to test and find why my commands aren't meeting with the same success. I will also note that I did not yet test against CICE data (which also previously errored out).

jprestop commented 1 week ago

Thanks so much for your testing, @j-opatz. While you said, you did not yet test against CICE data (which also previously errored out), I tested the new code and the code at /nrit/ral/met/bin/point2grid with the /d1/projects/MET/MET_issues/feature_2857/CICE/iceh.2018-01-04.c00.nc file and see everything is working as expected in /d1/personal/hsoh/git/pull_request/MET_feature_2857_tripolar_coordinates, but the old code at /nrit/ral/met/bin/point2grid errors out.

hsoh-u commented 1 week ago

The input (/d1/projects/MET/MET_issues/feature_2857/GFSv17_ocean/gfs.ocean.t00z.6hr_avg.f006.nc) is not NCCF file. point2grid does not support "file_type=NETCDF_NCCF.

// global attributes:
                :NumFilesInSet = 1 ;
                :title = "UFS_Weather_Model_Forecast" ;
                :grid_type = "regular" ;
                :grid_tile = "N/A" ;

This command will make it as NETCDF_NCCF.

ncatted -a conventions,global,c,c,"CF-1.0" gfs.ocean.t00z.6hr_avg.f006.nc

And there are log messages

WARNING:
WARNING: NcCfFile::get_grid_from_dimensions() -> Found multiple variables for longitude, using "xh".
WARNING:
WARNING:
WARNING: NcCfFile::get_grid_from_dimensions() -> Found multiple variables for latitude, using "yh".
WARNING:

        float SST(time, yh, xh) ;
                SST:_FillValue = -1.e+34f ;
                SST:missing_value = -1.e+34f ;
                SST:units = "degC" ;
                SST:long_name = "Sea Surface Temperature" ;
                SST:cell_methods = "area:mean yh:mean xh:mean time: mean" ;
                SST:time_avg_info = "average_T1,average_T2,average_DT" ;

The lat/lon variables match in this case. If the lat/lon variables do not match, the configuration file should be created with following:

var_name_map = [
  { key = "lat_vname";     val = "actual_lat_variable_name"; },
  { key = "lon_vname";     val = "actual_lon_variable_name"; }
];

I've gone ahead and tested against the branch Howard compiled on Seneca (/d1/personal/hsoh/git/pull_request/MET_feature_2857_tripolar_coordinates) using two commands, testing 2 separate datasets:
/d1/personal/hsoh/git/pull_request/MET_feature_2857_tripolar_coordinates/bin/point2grid /d1/projects/MET/MET_issues/feature_2857/GFSv17_ocean/gfs.ocean.t00z.6hr_avg.f006.nc "stereo 304 448 33.92 279.26 135.0 25 6367.47 70.0 N" testing_GFSocean.nc -field 'name="SST"; level="(0,*,*)";'

/d1/personal/hsoh/git/pull_request/MET_feature_2857_tripolar_coordinates/bin/point2grid /d1/projects/MET/MET_issues/feature_2857/RTOFS/rtofs_glo_2ds_f006_ice.nc "stereo 304 448 33.92 279.26 135.0 25 6367.47 70.0 N" testing_RTOFS.nc -field 'name="ice_coverage"; level="(0,*,*)";'
And was puzzled to find BOTH commands ended with an error.

The first command tests against GFSv17 ocean data, which previously also errored out when using point2grid. The error for that command is
ERROR  : 
ERROR  : process_data_file() -> "/d1/projects/MET/MET_issues/feature_2857/GFSv17_ocean/gfs.ocean.t00z.6hr_avg.f006.nc" not a valid data file
ERROR  : 
I believe this is the same error it showed before this PR.

The second command tests against RTOFS data, which previous worked with Point2Grid. It now errors out with this message
ERROR  : 
ERROR  : get_data_from_lat_lon_vars() -> MET can only process Latitude/Longitude files where the latitudes are evenly spaced (dlat=0.0319977, delta[421]=0.0336151)
ERROR  : 
ERROR  : 
ERROR  : get_data_from_lat_lon_vars() -> Please check the input data is the lat/lon projection
ERROR  : 
This seems to be a step back from where point2grid previously was, since it worked with MET before. I confirmed my memory by testing the RTOFS dataset against the Nightly Build of MET with the command
 /d1/projects/MET/MET_regression/develop/NB20240627/MET-develop/bin/point2grid /d1/projects/MET/MET_issues/feature_2857/RTOFS/rtofs_glo_2ds_f006_ice.nc "stereo 304 448 33.92 279.26 135.0 25 6367.47 70.0 N" testing_RTOFS.nc -field 'name="ice_coverage"; level="(0,*,*)";'
And completed a successful run. I'm not sure what's happening to cause this discrepancy, but we need to discuss what commands you're currently using to test and find why my commands aren't meeting with the same success. I will also note that I did not yet test against CICE data (which also previously errored out).

j-opatz commented 1 week ago

The input (/d1/projects/MET/MET_issues/feature_2857/GFSv17_ocean/gfs.ocean.t00z.6hr_avg.f006.nc) is not NCCF file. point2grid does not support "file_type=NETCDF_NCCF.

Thanks for confirming this, @hsoh-u. I think the issue I have with your purposed solution (using ncatted) is that this file was provided to us directly from EMC, which means all of their files coming from GFSv17 ocean will need this fix. I don't disagree that allowing more non-CF compliant files to get in MET without user-provided tweaks is not ideal, but I want to be cognizant of EMC's needs and if this presents a large hurdle to their verification process.

Let me reach out to EMC for further guidance on this.

Did you happen to test the RTOFS file as well? Is there a similar issue with it not being CF-compliant? Or does that have to do with the lat/lon variables not matching? It was odd that my test using the nightly build worked, but the feature branch build did not; to me it doesn't seem related to the CF-compliance issue of the GFSv17 ocean data.

hsoh-u commented 1 week ago

Two changes were added:

Overriding file_type is allowed by the configuration file.
Do not check the existence of the time variable for input /d1/projects/MET/MET_issues/feature_2857/RTOFS/rtofs_glo_2ds_f006_ice.nc

Following commands work:

/d1/personal/hsoh/git/pull_request/MET_feature_2857_tripolar_coordinates/bin/point2grid /d1/projects/MET/MET_issues/feature_2857/GFSv17_ocean/gfs.ocean.t00z.6hr_avg.f006.nc G179  testing_GFSocean.nc -field 'name="SST"; level="(0,*,*)";' -config /d1/personal/hsoh/data/MET-2857/Point2GridConfig_SST

/d1/personal/hsoh/git/pull_request/MET_feature_2857_tripolar_coordinates/bin/point2grid /d1/projects/MET/MET_issues/feature_2857/RTOFS/rtofs_glo_2ds_f006_ice.nc "stereo 304 448 33.92 279.26 135.0 25 6367.47 70.0 N" testing_RTOFS.nc -field 'name="ice_coverage"; level="(0,*,*)";'

Note: added -config /d1/personal/hsoh/data/MET-2857/Point2GridConfig_SST to override file_type for gfs.ocean.t00z.6hr_avg.f006.nc

jprestop commented 6 days ago

@j-opatz What are your thoughts on Howard's latest comment above? Is that sufficient for this work? Can you please test and ensure you get expected results? This is the last PR to approve before we can cut the beta release.

@hsoh-u if the results of the new work for RTOFS and GFSv17 ocean are successful and make sense, can you please add unit tests for those as well so that we ensure they keep working in the future?

j-opatz commented 5 days ago

@j-opatz What are your thoughts on Howard's latest comment above? Is that sufficient for this work? Can you please test and ensure you get expected results? This is the last PR to approve before we can cut the beta release.

@jprestop I'm still waiting to hear back from a few stakeholders at EMC on this; only Mallory has commented so far, and I want to make sure this doesn't present an issue for verification on this side.

jprestop commented 5 days ago

Thanks so much for the update @j-opatz. Thank you also for reaching out to EMC stakeholders. I appreciate your efforts.

jprestop commented 4 days ago

@hsoh-u I tested the same two cases in my environment that you listed above and they worked well for me also. While we're waiting for John O. to hear back from EMC, can you please go ahead and add two new unit tests for the RTOFS and GFSv17 ocean data, so that this is ready to go sometime next week?

hsoh-u commented 4 days ago

I added two testcases locally, but not ready to check in them.

DEBUG 2: Range of data (name="ice_coverage"; level="(0,*,*)";)
DEBUG 2:        input: 0 to 1   regridded: 0 to 0.

This log message means between 0 (min value) to 1 (max value) from the input, but value range after regridding is 0 to 0 (all 0). I'm still looking at this issue.

jprestop commented 4 days ago

This log message means between 0 (min value) to 1 (max value) from the input, but value range after regridding is 0 to 0 (all 0). I'm still looking at this issue.

Thanks, @hsoh-u!

dtcenter / MET