Feature #3059 MPR python embedding #3115
Conversation
…odaHeaders to IODAReader/IODAMetadata/IODAHeaders.
…eader::validate_metadata() to make it accessible from pair_stat.
…mes rather than handling it in the application code.
…still need to populate the header columns in a sensical manner.
…ce time from input files and interpret it the same way as a model initialization time.
… instead of just STATLine inputs. Do still need some more work in the vx_stat_out library for Pair-Stat to avoid false alarm warning messages.
…time for each pair.
…the forecast lead time for each pair which will require changes to the PairBase class.
…ed pair. Note that this feature is currently only used by Pair-Stat when processing IODA inputs where the forecast lead times can vary.
… station ID name from an input stationIdentifier variable name.
…puts and update the StatHdrInfo::get_shc() logic to not print a warning message if those header column values are manually defined.
…iltering options over from Stat-Analysis to Pair-Stat.
…nfiguration options.
…actually apply those filters to the data though.
…PR input columns rather than the previous 37 number of them.
…test to demonstrate that functionality.
…ate unit_pair_stat.xml to test with IODA inputs with climo input.
…e climo data. This is needed for pair_stat but the verification grid is already defined for all the other application code.
…separately, which is needed for pair_stat."
…tput_sondes.nc4 correctly.
…smells flagged for PR #3114.
Co-authored-by: Dan Adriaansen <dadriaan@ucar.edu>
Co-authored-by: Dan Adriaansen <dadriaan@ucar.edu>
Co-authored-by: Dan Adriaansen <dadriaan@ucar.edu>
…r to version 12 and 39 columns for version 12 and above.
There was a problem hiding this comment.
I set up a GHA workflow run to test the use cases that were flagged using the MET Docker image that was created for this PR.
https://github.com/dtcenter/METplus/actions/runs/14182040777
It looks like the failing use case now succeeds with no differences.
The other use case has differences in the MODEL column (values of WRF were changed to NA). @JohnHalleyGotway is confirming that this is expected.
I approve given that there aren't any changes needed to prevent the MODEL column diff.
…are set to an empty string in the config file by default, and those values should only be written to the output when they are set to a non-empty string.
|
I just pushed this commit that fixes the functionality when I test locally on seneca. Basically, when model and desc are set to their default value of an empty string in the config file, we want to ignore that setting and use the values read from the input .stat data instead. We were ignoring empty string in beta1, but I consolidated the logic and unintentionally introduced that difference. Unfortunately, since this is a code change, all the GHA tests will need to rerun. Please rerun your METplus test at your convenience and confirm that no differences remain. |
There was a problem hiding this comment.
I reran the METplus GHA testing workflow after the latest commit and confirmed that it resolves the differences:
https://github.com/dtcenter/METplus/actions/runs/14182040777
I approve.
Expected Differences
This is the second pull request for issue #3059. The first one #3114 updated
read_ascii_mpr.pyto read 39 MPR columns (in MET version 12 and following) instead of 37. That works fine for the MET unit tests but broke existing METplus use cases in this GHA run.These changes update the MPR Python embedding logic to specify the number of MPR columns to be read based on the major version number: 37 columns for less than version 12 and 39 columns for version 12 and later.
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:
Confirmed that
unit_pair_stat.xmlcommands run well and also tested with MET version 11.0.3 and that works.Recommend testing for the reviewer(s) to perform, including the location of input datasets, and any additional instructions:
We ultimately need to confirm these changes fix the METplus failure.
Do these changes include sufficient documentation updates, ensuring that no errors or warnings exist in the build of the documentation? [Yes]
None needed.
Do these changes include sufficient testing updates? [Yes]
None needed.
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:
Will this PR result in changes to existing METplus Use Cases? [Yes]
If yes, create a new Update Truth METplus issue to describe them.
It will hopefully fix the MPR Python embedding failure.
Do these changes introduce new SonarQube findings? [Yes or No]
If yes, please describe:
Unknown.
Please complete this pull request review by [Mon 3/31/25].
Pull Request Checklist
See the METplus Workflow for details.
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