Feature #2609 tc_diag_missing_data

[JohnHalleyGotway]

Expected Differences

Please note that these changes constitute only one part of the changes needed in TC-Diag for MET version 12.0.0. This PR includes the following changes:

• Update default TC-Diag config file to explicitly list expected lead times of 0, 6, 12, ..., 126 and add new one_time_per_file_flag config option.

• Update vx_log library to easily turn on/off warning messages, and use that in TC-Diag.

• Add Met2dDataFile::data_planes(...) member function to retrieve a vector of requested fields from a single input file, rather than reading one field at a time.

• Minor edits in derived 2D data file type libraries to clean up whitespace.

• Add get_series_entries(...) utility function to retrieve a list of fields rather than one at a time.

• Update control flow logic in TC-Diag tool.

• Do these changes introduce new tools, command line arguments, or configuration file options? [Yes]
  
  If yes, please describe:
  
  Adds one_time_per_file_flag configuration option for TC-Diag.

• 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:
  
  Added one_time_per_file_flag = TRUE; to the existing unit test. The existing unit test should run fine but process additional time steps and run faster.

For the develop branch (without new one_time_per_file_flag option and with only 5 valid lead times of data):

musial6:test_unit johnhg$ perl/unit.pl xml/unit_tc_diag.xml 
TEST: tc_diag_IAN  - pass -  80.224 sec

For the feature_2609_tc_diag_missing_data branch (with one_time_per_file_flag = TRUE; and with all expected 0 - 126 hours of lead time):

musial6:test_unit johnhg$ perl/unit.pl xml/unit_tc_diag.xml 
TEST: tc_diag_IAN  - pass -  56.030 sec

The updated output file contains 22 times instead of only 5:

time = 22 ;

There's only valid data for the first 5 time steps, but the rest are present and contain missing data. Previously TC-Diag would error out due to missing data. But it no longer does.

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

@jvigh: Please review the config file changes and documentation updates.
@georgemccabe: Please review the code changes and be aware of the new one_time_per_file_flag config option. Please see dtcenter/METplus#2340.

• 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]
  Existing unit test still works fine.

• Will this PR result in changes to the test suite? [Yes]
  
  If yes, describe the new output and/or changes to the existing output:
  
  Modifies the output of the unit_tc_diag.xml as described above.

• Please complete this pull request review by [Tues 9/12/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) 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.

[JohnHalleyGotway]

FYI, as expected, differences were flagged in the output of this GHA testing workflow run. After downloading the logs artifact and diff artifact, I see only one difference flagged in comp_dir.log:

COMPARING tc_diag/tc_diag_AL092022_GFSO_2022092400_cyl_grid_parent.nc
file1: /data/output/met_test_truth/tc_diag/tc_diag_AL092022_GFSO_2022092400_cyl_grid_parent.nc
file2: /data/output/met_test_output/tc_diag/tc_diag_AL092022_GFSO_2022092400_cyl_grid_parent.nc
ERROR: NetCDF headers differ:
3,4c3,4
<       track_line = 5 ;
<       time = 5 ;
---
>       track_line = 49 ;
>       time = 22 ;

This TC-Diag NetCDF output file difference is expected.

Feel free to inspect the old/new files are you'd like.

[georgemccabe]

These changes look good. I see there is a new variable called one_time_per_file_flag which will need to be supported via METplus wrappers and that this is noted in the METplus GitHub issue. I approve.

[jvigh]

@JohnHalleyGotway There's something weird here. There's an empty code block, but then the lead times that follow are split across multiple lines in a weird way. Should these actually be inside the code block?

[jvigh]

Oh, I think I understand -- to be inside the code block, the lead = [ ]; stuff needs to be indented.

[JohnHalleyGotway]

@jvigh, great catch! Thanks for your review. I pushed a fix for that indent problem.

[jvigh]

The -data snippet is spread across two lines in the rendered doc. Is there any way to protect this term from spreading across the line break? I couldn't find out how after a few minutes of searching . . .

[JohnHalleyGotway]

It would seem that RTD treats the leading - and the following data as part of separate words when wrapping text. I'm guessing it treats the same as hyphenated words like "check-in" where the - character is a good place to wrap. You'll notice that as you adjust the browser window width, the wrapping changes dynamically.

I reworded the sentence to address this by moving the the -data away from the end of the line when the browser window is at max width. I could switch the formatting from bold -data to this in-line code block -data instead. That'd probably fix it but wouldn't be consistent with the formatting in the rest of the User's Guide.

But we could choose to modify the formatting for all -something command line options to always use that in-line code block formatting... just not in this PR.

[JohnHalleyGotway]

Actually, playing around with the formatting of my comment above by modifying the browser window width, formatting as -data does NOT fix it. It still wraps at the - as it would for hyphenated words. Probably not something worth worrying about any more than this.

[jvigh]

@JohnHalleyGotway Overall, things look good. I left a couple single-line comments regarding minor formatting issues. Once these are approved, I approve this PR.