Bugfix #2897 develop python_valid_time

[JohnHalleyGotway]

Note that this bug in Point-Stat and Ensemble-Stat occurs when reading point observations through Python embedding. If some of the obs share a common valid time, then there is a mismatch in the unique list of numeric valid times and the non-unique list of valid time strings.

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:
  
  Please see the testing described in the body of Bugfix: Fix inconsistent handling of point observation valid times processed through Python embedding #2897. I demonstrated the problem using the nightly build versions of MET and the solution using seneca:/d1/projects/MET/MET_pull_requests/met-12.0.0/beta5/MET-bugfix_2897_develop_python_valid_time.
  I did also test with all 151 obs for the TCI use case and confirmed that the consistent and correct valid time is now being used.

• Recommend testing for the reviewer(s) to perform, including the location of input datasets, and any additional instructions:
  
  Review the changes and test using seneca:/d1/projects/MET/MET_pull_requests/met-12.0.0/beta5/MET-bugfix_2897_develop_python_valid_time as you see fit.

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

• Do these changes include sufficient testing updates? [No]
  None needed as these will impact the output from METplus use cases and it will be tested there.

• Will this PR result in changes to the MET test suite? [No]
  
  If yes, describe the new output and/or changes to the existing output:
  
  I do not expect diffs from MET.

• Will this PR result in changes to existing METplus Use Cases? [Yes]
  
  If yes, create a new Update Truth METplus issue to describe them.
  I expect modified output from the TCI use case and it has the potential to impact output from other use cases.

• Do these changes introduce new SonarQube findings? [No]
  
  If yes, please describe:
  I hope not.

• Please complete this pull request review by [Thurs 5/23/24].
  

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 ⚙️ 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.

[DanielAdriaansen]

@JohnHalleyGotway thanks for making this fix, I will test it shortly.

Regarding asking for additional debugging that would have helped, I think it's worthwhile to consider adding a high DEBUG option (10? 10+?) to print the "header" (an entire row of the 11-column data) for each of these rejection types:

MET/src/libcode/vx_statistics/pair_data_point.cc

Lines 424 to 430 in 16aac92

	rej_typ = (int ***) 0;
	rej_mask = (int ***) 0;
	rej_fcst = (int ***) 0;
	rej_cmn = (int ***) 0;
	rej_csd = (int ***) 0;
	rej_mpr = (int ***) 0;
	rej_dup = (int ***) 0;

In debugging I actually added some print statements here:

MET/src/libcode/vx_statistics/pair_data_point.cc

Lines 1020 to 1022 in 16aac92

	if(hdr_ut < beg_ut || hdr_ut > end_ut) {
	rej_vld++;
	return;

So that would be one place where if rej_vld is being incremented, check if the DEBUG exceeds some level and if so dump the row of data that's being rejected. I do think this would not be used very frequently, but the Python embedding case is a good example of where we were confident about this section of code (in pair_data_point.cc), but then came along with another way to serve data to this class (Python embedding). Future vehicles for serving data to MET that are yet to be defined would benefit from this added debugging.

[DanielAdriaansen]

I approve. I was able to test on Seneca and confirm that I received the correct number of rejections for valid time. I was also able to see the new debug messages, which provide more information about whether an observation is being used or skipped, and if it is being skipped, why it is being skipped. Thanks @JohnHalleyGotway!