Bugfix #2437 develop convert

[JohnHalleyGotway]

Expected Differences

• Do these changes introduce new tools, command line arguments, or configuration file options? [No]
  
  If yes, please describe:
  

• 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

• Describe testing already performed for these changes:
  

Manually tested using a single monthly input file.

Note that I originally submitted this PR for main_v11.0 rather than develop on accident. The GHA tests did NOT trigger the differencing logic. So I manually triggered a full regression test run here:
https://github.com/dtcenter/MET/actions/runs/4109020631

Note that all unit tests should run and the only differences should be...

• The addition of 2 new Grid-Stat output files.

• Modifying the FCST_UNITS and OBS_UNITS columns of some existing output files.

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

Recommend that you actually download the diff files and confirm that the modifications to existing files are limited to the FCST_UNITS and OBS_UNITS columns.

You can test these code on seneca as the met_test user in:
/d1/projects/MET/MET_pull_requests/met-11.0.1/issue_2437/MET-bugfix_2437_develop_convert

You could test additional dates, including leap years, and pay close attention to the "Skipping", "Storing", "Shifting", and "Interpolating" log messages to confirm that the logic is working as expected.

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

• Do these changes include sufficient testing updates? [Yes]

• Will this PR result in changes to the test suite? [Yes]
  
  If yes, describe the new output and/or changes to the existing output:
  

Adds a new call to Grid-Stat to run an existing unit test but with 1 monthly climo input time instead of 2.

• Please complete this pull request review by [Thurs 2/9/23].
  

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)
  Select: Organization level software support Project or Repository level development cycle Project
  Select: Milestone as the version that will include these changes
• After submitting the PR, select Development issue with the original issue number.
• 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.

[j-opatz]

There's an odd behavior I found when testing an outlier case: Using the 19590115 climo mean data, and requesting a valid time of 20221226_03 with the hour_interval set to NA in Grid-Stat, MET finds all hourly entries (0,6,12,18) as valid (which makes sense). However, take a look at what happens just before it errors out:

DEBUG 3: For forecast valid at 20221226_030000, found 4 climatology field(s) with valid time(s): 20230113_180000, 20230115_000000, 20230115_060000, 20230115_120000
ERROR  : 
ERROR  : climo_hms_interp() -> Can't find climatology data before/after date 20221226_000000.
ERROR  : 

What's going on with that first date it found, 20230113_180000? All of the other dates are properly set to 0115, but the 18z hour is set to a day and 6 hours earlier. I'm assuming this has to do with the logic you had to put in place to have it look a day before rather than 18z the day of. I ran a similar test (19590115 climo mean data, NA on hour_interval) but with a valid time of 20221226_23 and got similar odd times found:

DEBUG 3: For forecast valid at 20221226_230000, found 4 climatology field(s) with valid time(s): 20230114_180000, 20230116_000000, 20230116_060000, 20230115_120000

The valid times are all over the place, with 0116, 0115, and 0114 for that one. I'm not sure this is really an issue: we discussed that the NA values really can't be used without consequences especially with this interpolation process. But I did want to make a note that it looks like the interpolated times get pretty wonky if the hour_interval isn't provided.

[j-opatz]

Additional testing shows there's still something going wrong with the interpolation method for times relying on the 18z and 0z hours. Look at the following output:

DEBUG 3: For forecast valid at 20221226_230000, found 2 climatology field(s) with valid time(s): 20230114_180000, 20230116_000000
DEBUG 3: Shifting climatology times -19 day(s) from 20230114_180000 to 20221226_180000 and 20230116_000000 to 20221228_000000.
DEBUG 3: Interpolating climatology fields at 20221226_180000 and 20221228_000000 to 20221226_230000 using the NEAREST interpolation method.

That's with the appropriate hour_interval set, so there's no reason it should be grabbing something from 1228_00 for a valid time of 1226_23.

[JohnHalleyGotway]

@j-opatz, looking more closely at the timestamps in the 1215 climo file I see:

wgrib /Volumes/d1/projects/MET/MET_unit_test/MET_test_input/climatology_data/NCEP_2.5deg/pgba_mean.19591215 | grep TMP | grep "2 m"
57:682728:d=59121418:TMP:kpds5=11:kpds6=105:kpds7=2:TR=51:P1=6:P2=0:TimeU=1:2 m above gnd:clim? p1=6? 0hr?:NAve=40
58:695952:d=59121500:TMP:kpds5=11:kpds6=105:kpds7=2:TR=51:P1=6:P2=0:TimeU=1:2 m above gnd:clim? p1=6? 0hr?:NAve=40
59:709176:d=59121506:TMP:kpds5=11:kpds6=105:kpds7=2:TR=51:P1=6:P2=0:TimeU=1:2 m above gnd:clim? p1=6? 0hr?:NAve=40
60:722400:d=59121512:TMP:kpds5=11:kpds6=105:kpds7=2:TR=51:P1=6:P2=0:TimeU=1:2 m above gnd:clim? p1=6? 0hr?:NAve=40

The 2-m Temp is dated on the 14-th at 18z when I would have expected it to be the 15-th!
So I'm not really concerned about that timestamp on the 13-th, since it's due to oddities in the climo data that was not created by us.

[JohnHalleyGotway]

FYI, for some reason this 18Z timestamp date difference affects the surface data (2m and 10m) but not the upper air data:

wgrib /Volumes/d1/projects/MET/MET_unit_test/MET_test_input/climatology_data/NCEP_2.5deg/pgba_mean.19591215 | grep "18:"
4:35730:d=59121518:HGT:kpds5=7:kpds6=100:kpds7=1000:TR=51:P1=0:P2=0:TimeU=1:1000 mb:0-0hr product:ave@1yr:NAve=40
8:87312:d=59121518:HGT:kpds5=7:kpds6=100:kpds7=700:TR=51:P1=0:P2=0:TimeU=1:700 mb:0-0hr product:ave@1yr:NAve=40
12:140208:d=59121518:HGT:kpds5=7:kpds6=100:kpds7=500:TR=51:P1=0:P2=0:TimeU=1:500 mb:0-0hr product:ave@1yr:NAve=40
16:197046:d=59121518:HGT:kpds5=7:kpds6=100:kpds7=250:TR=51:P1=0:P2=0:TimeU=1:250 mb:0-0hr product:ave@1yr:NAve=40
18:224808:d=59121506:TMP:kpds5=11:kpds6=100:kpds7=850:TR=51:P1=0:P2=0:TimeU=1:850 mb:0-0hr product:ave@1yr:NAve=40
20:251256:d=59121518:TMP:kpds5=11:kpds6=100:kpds7=850:TR=51:P1=0:P2=0:TimeU=1:850 mb:0-0hr product:ave@1yr:NAve=40
24:300210:d=59121518:TMP:kpds5=11:kpds6=100:kpds7=500:TR=51:P1=0:P2=0:TimeU=1:500 mb:0-0hr product:ave@1yr:NAve=40
28:343908:d=59121518:TMP:kpds5=11:kpds6=100:kpds7=250:TR=51:P1=0:P2=0:TimeU=1:250 mb:0-0hr product:ave@1yr:NAve=40
32:390234:d=59121518:UGRD:kpds5=33:kpds6=100:kpds7=850:TR=51:P1=0:P2=0:TimeU=1:850 mb:0-0hr product:ave@1yr:NAve=40
36:433932:d=59121518:VGRD:kpds5=34:kpds6=100:kpds7=850:TR=51:P1=0:P2=0:TimeU=1:850 mb:0-0hr product:ave@1yr:NAve=40
40:480258:d=59121518:UGRD:kpds5=33:kpds6=100:kpds7=500:TR=51:P1=0:P2=0:TimeU=1:500 mb:0-0hr product:ave@1yr:NAve=40
44:523956:d=59121518:VGRD:kpds5=34:kpds6=100:kpds7=500:TR=51:P1=0:P2=0:TimeU=1:500 mb:0-0hr product:ave@1yr:NAve=40
48:574224:d=59121518:UGRD:kpds5=33:kpds6=100:kpds7=250:TR=51:P1=0:P2=0:TimeU=1:250 mb:0-0hr product:ave@1yr:NAve=40
52:619236:d=59121518:VGRD:kpds5=34:kpds6=100:kpds7=250:TR=51:P1=0:P2=0:TimeU=1:250 mb:0-0hr product:ave@1yr:NAve=40
56:669504:d=59121518:PRMSL:kpds5=2:kpds6=102:kpds7=0:TR=51:P1=0:P2=0:TimeU=1:MSL:0-0hr product:ave@1yr:NAve=40
57:682728:d=59121418:TMP:kpds5=11:kpds6=105:kpds7=2:TR=51:P1=6:P2=0:TimeU=1:2 m above gnd:clim? p1=6? 0hr?:NAve=40
61:735624:d=59121418:TMAX:kpds5=15:kpds6=105:kpds7=2:TR=51:P1=6:P2=0:TimeU=1:2 m above gnd:clim? p1=6? 0hr?:NAve=40
65:788520:d=59121418:TMIN:kpds5=16:kpds6=105:kpds7=2:TR=51:P1=6:P2=0:TimeU=1:2 m above gnd:clim? p1=6? 0hr?:NAve=40
69:841416:d=59121418:UGRD:kpds5=33:kpds6=105:kpds7=10:TR=51:P1=6:P2=0:TimeU=1:10 m above gnd:clim? p1=6? 0hr?:NAve=40
73:883800:d=59121418:VGRD:kpds5=34:kpds6=105:kpds7=10:TR=51:P1=6:P2=0:TimeU=1:10 m above gnd:clim? p1=6? 0hr?:NAve=40

[j-opatz]

As updated in the PR, the date mismatch identified in my previous review was the result of the data itself, rather than the code. 2m variables have a 14th date attached to them, causing unexpected, but properly coded, results.
Given this, the updates work as requested and this PR is approved.