This PR affects the Point-Stat and Ensemble-Stat tools and refines the logic for when warning messages are printed based on NOAA/EMC feedback.
Previously, when processing the config files for these tools, the code checked to see if the requested forecast levels do not fully contain the observation levels... only for forecast pressure level types. The intention is to warn the user that vertical interpolation may not work as they expect.
The challenge is that a range of pressure levels (e.g. TMP/P500-850) may correspond to multiple individual levels... or it may correspond to a single record containing a LAYER of data. NOAA/EMC was seeing warnings when verifying a layer of data and would like those warnings to go away.
The change in this PR is to move the location of these checks. Rather than doing it when the config file is parsed, do it after the gridded data is read from the input forecast files. Change the logic to check this for ALL LEVEL TYPES (not just pressure levels) since vertical interpolation also works for height levels. But only check this if more than 1 forecast record was ACTUALLY read from the data. In NOAA/EMC's CAPE data case, only a single record will be found. So no warning will be printed.
Note as well, that Ensemble-Stat includes logic to only print this warning once for each verification task... rather than once for each ensemble member.
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
[ ] Describe testing already performed for these changes:
See test in seneca:/d1/projects/MET/MET_pull_requests/met-12.0.0/beta5/MET-feature_2795_level_mismatch_warning/test_2795/run_ps.sh
This makes the following 4 comparisons for temperature data in PointStatConfig:
fcst = {
field = [
{ name = "TMP"; level = "L180-150"; },
{ name = "TMP"; level = "L180-150"; },
{ name = "TMP"; level = "P500-850"; },
{ name = "TMP"; level = "P500-850"; }
];
}
obs = {
field = [
{ name = "TMP"; level = "P180-150"; },
{ name = "TMP"; level = "P0-500"; },
{ name = "TMP"; level = "P500-850"; },
{ name = "TMP"; level = "P250-1000"; }
];
}
It runs 2 versions of Point-Stat... one from the nightly build develop branch and once from my feature branch.
Here's the difference in the log files:
< WARNING:
< WARNING: PointStatVxOpt::process_config() -> The range of requested observation pressure levels is not contained within the range of requested forecast pressure levels. No vertical interpolation will be performed for observations falling outside the range of forecast levels. Instead, they will be matched to the single nearest forecast level.
< WARNING:
> WARNING:
> WARNING: process_fcst_climo_files() -> The forecast level range (TMP/P850-500) does not fully contain the observation level range (TMP/P1000-250). No vertical interpolation will be performed for observations falling outside the range of forecast levels. Instead, they will be matched to the single nearest forecast level.
> WARNING:
Both are only for the 4th comparison. This admittedly isn't the most comprehensive test. But it does demonstrate the change to the warning message. And it points out that the new code does NOT trigger a warning for the 2nd task, where there's a mismatch between the forecast and observations levels. But since the forecast data is a single record with a layer of data, no warning message is triggered.
[x] Recommend testing for the reviewer(s) to perform, including the location of input datasets, and any additional instructions:
Code compiled and available for testing as met_test in seneca:/d1/projects/MET/MET_pull_requests/met-12.0.0/beta5/MET-feature_2795_level_mismatch_warning.
[x] Do these changes include sufficient documentation updates, ensuring that no errors or warnings exist in the build of the documentation? [No]
None needed.
[x] Do these changes include sufficient testing updates? [No]
None needed.
[x] 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:
Although existing UGRID diffs will likely be flagged. I confirmed that all the diffs in this GHA testing workflow run really are from UGRID diffs unrelated to this PR.
[x] Will this PR result in changes to existing METplus Use Cases? [No]
If yes, create a new Update TruthMETplus issue to describe them.
[x] Do these changes introduce new SonarQube findings? [No]
If yes, please describe:
[x] Please complete this pull request review by [Fri 5/3/24].
[x] Review the source issue metadata (required labels, projects, and milestone).
[x] Complete the PR definition above.
[x] Ensure the PR title matches the feature or bugfix branch name.
[x] 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
[x] 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.
[x] After the PR is approved, merge your changes. If permissions do not allow this, request that the reviewer do the merge.
[x] Close the linked issue and delete your feature or bugfix branch from GitHub.
This PR affects the Point-Stat and Ensemble-Stat tools and refines the logic for when warning messages are printed based on NOAA/EMC feedback.
Previously, when processing the config files for these tools, the code checked to see if the requested forecast levels do not fully contain the observation levels... only for forecast pressure level types. The intention is to warn the user that vertical interpolation may not work as they expect.
The challenge is that a range of pressure levels (e.g.
TMP/P500-850
) may correspond to multiple individual levels... or it may correspond to a single record containing a LAYER of data. NOAA/EMC was seeing warnings when verifying a layer of data and would like those warnings to go away.The change in this PR is to move the location of these checks. Rather than doing it when the config file is parsed, do it after the gridded data is read from the input forecast files. Change the logic to check this for ALL LEVEL TYPES (not just pressure levels) since vertical interpolation also works for height levels. But only check this if more than 1 forecast record was ACTUALLY read from the data. In NOAA/EMC's CAPE data case, only a single record will be found. So no warning will be printed.
Note as well, that Ensemble-Stat includes logic to only print this warning once for each verification task... rather than once for each ensemble member.
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
seneca:/d1/projects/MET/MET_pull_requests/met-12.0.0/beta5/MET-feature_2795_level_mismatch_warning/test_2795/run_ps.sh
This makes the following 4 comparisons for temperature data in PointStatConfig:
It runs 2 versions of Point-Stat... one from the nightly build develop branch and once from my feature branch. Here's the difference in the log files:
Both are only for the 4th comparison. This admittedly isn't the most comprehensive test. But it does demonstrate the change to the warning message. And it points out that the new code does NOT trigger a warning for the 2nd task, where there's a mismatch between the forecast and observations levels. But since the forecast data is a single record with a layer of data, no warning message is triggered.
[x] Recommend testing for the reviewer(s) to perform, including the location of input datasets, and any additional instructions: Code compiled and available for testing as
met_test
inseneca:/d1/projects/MET/MET_pull_requests/met-12.0.0/beta5/MET-feature_2795_level_mismatch_warning
.[x] Do these changes include sufficient documentation updates, ensuring that no errors or warnings exist in the build of the documentation? [No] None needed.
[x] Do these changes include sufficient testing updates? [No] None needed.
[x] 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: Although existing UGRID diffs will likely be flagged. I confirmed that all the diffs in this GHA testing workflow run really are from UGRID diffs unrelated to this PR.
[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:
[x] Please complete this pull request review by [Fri 5/3/24].
Pull Request Checklist
See the METplus Workflow for details.