Bugfix: Refine the regrid dictionary's data conversion and censoring operations and fix climo time matching logic for a single monthly climo file.

[j-opatz]

Describe the Problem

This was brought to our attention in Discussions #2033. When the user places the convert function of K_to_C(x) as follows, MET produces climatology field output that is 2x the requested amount:

////////////////////////////////////////////////////////////////////////////////

//
// Climatology data
//
climo_mean = {
   convert(x) = K_to_C(x);
   file_name = ["pgba_mean.19590115"];
   field     = [{name ="TMP"; level="Z2";}];
   regrid = {
      method     = NEAREST;
      width      = 1;
      vld_thresh = 0.5;
      shape      = SQUARE;
   }

This behavior was confirmed in both MET-11.0.0 and the nightly build. It was also consistent regardless of the requested conversion (e.g. 2*x,log10(x)).

The short-term fix was identified by @JohnHalleyGotway, which places the convert function inside the field array. This produces the expected output:

////////////////////////////////////////////////////////////////////////////////

//
// Climatology data
//
climo_mean = {
   file_name = ["pgba_mean.19590115"];
   field     = [{name ="TMP"; level="Z2"; convert(x) = K_to_C(x);}];
   regrid = {
      method     = NEAREST;
      width      = 1;
      vld_thresh = 0.5;
      shape      = SQUARE;
   }

Additional issues were discovered during this issue investigation.

The first is that in the GridStatConfig_Default, https://github.com/dtcenter/MET/blob/main_v11.0/data/config/GridStatConfig_default, the match_month setting in the climatology mean dictionary is still present. This setting has been deprecated and should be removed.

The second is that in a Point-Stat unit test,

MET/internal/test_unit/config/PointStatConfig_climo

Line 54 in 340358f

field = [

, the set_attr_units variable is not utilized to set FCST_UNITS and OBS_UNITS columns with the right strings; instead, the DESC column is used. To stay consistent with best coding practices, this should be updated.

The third is that when providing a single monthly climo file with 4 times, 00, 06, 12, and 18, and requesting an off-hour (i.e. 03), no matching climo data is found. Instead, MET should use the 00 and 06 hours and apply the specified interpolation logic.

Here's a summary of things to fix:

• Limit the scope of data censoring and conversion defined in the regrid dictionary.
• Remove deprecated match_month setting from GridStatConfig files.
• Fix climo timing issue for off-hours when only 1 monthly climo file is provided.
• Add Point-Stat unit test that demonstrates unit conversion operations to main_v11.0 and enhance it.
• Add Grid-Stat unit test to demonstrate climo timing logic when processing a single monthly input with off hours.

Expected Behavior

For the main bug issue (climo), the convert function should produce the same results regardless of if it's placed in the field array or outside it.

For deprecated variable, it simply shouldn't be there.

And for the unit test, the unit columns for the forecast and observation data should be populated via set_attr_units

Environment

Describe your runtime environment:

1. Machine: Seneca
2. OS: Linux
3. MET 11.0.0 and NB20230202

To Reproduce

Describe the steps to reproduce the behavior:

1. Use climo data of your choice in Grid-Stat (or any tool), populating the dictionary as described above. Make sure to request netCDF output for climo for quickest visual confirmation
2. run Grid-Stat as usual
3. Open output netCDF file and view climo field.

Relevant Deadlines

Bugfix for MET 11.0.1

Funding Source

Define the source of funding and account keys here or state NONE.

Define the Metadata

Assignee

• Select engineer(s) or no engineer required
• Select scientist(s) or no scientist required

Labels

• Select component(s)
• Select priority
• Select requestor(s)

Projects and Milestone

• Select Organization level Project for support of the current coordinated release
• Select Repository level Project for development toward the next official release or add alert: NEED PROJECT ASSIGNMENT label
• Select Milestone as the next bugfix version

Define Related Issue(s)

Consider the impact to the other METplus components.

• METplus, MET, METdataio, METviewer, METexpress, METcalcpy, METplotpy
  None.

Bugfix Checklist

See the METplus Workflow for details.

• Complete the issue definition above, including the Time Estimate and Funding Source.
• Fork this repository or create a branch of main_<Version>.
  Branch name: bugfix_<Issue Number>_main_<Version>_<Description>
• Fix the bug and test your changes.
• Add/update log messages for easier debugging.
• Add/update unit tests.
• Add/update documentation.
• Add any new Python packages to the METplus Components Python Requirements table.
• Push local changes to GitHub.
• Submit a pull request to merge into main_<Version>.
  Pull request: bugfix <Issue Number> main_<Version> <Description>
• Define the pull request metadata, as permissions allow.
  Select: Reviewer(s) and Development issues
  Select: Organization level software support Project for the current coordinated release
  Select: Milestone as the next bugfix version
• Iterate until the reviewer(s) accept and merge your changes.
• Delete your fork or branch.
• Complete the steps above to fix the bug on the develop branch.
  Branch name: bugfix_<Issue Number>_develop_<Description>
  Pull request: bugfix <Issue Number> develop <Description>
  Select: Reviewer(s) and Development issues
  Select: Repository level development cycle Project for the next official release
  Select: Milestone as the next official version
• Close this issue.

[JohnHalleyGotway]

The culprit here is the met_regrid() function in the vx_regrid.cc file. After regridding data, it applies any convert(x) and censor operations that are defined at any higher level of config file context than the regrid dictionary itself.

Since the climo_mean dictionary contains a regrid dictionary, if convert(x) is defined at the same level as regrid, the conversion operator will be applied to the output of the conversion logic. I was also able to replicate this behavior in Grid-Stat by defining convert(x) = 2*x; at the highest level of config file context. When no regridding is requested, the fcst/obs values are multiplied by 2. When regridding IS requested (to_grid = "G130";) then the values are multiplied by 4:

The max value is 38.664 instead of 9.188.

I'd argue that we do NOT want to apply the data censoring and conversion logic AFTER we perform regridding. Instead, they are applied when we READ the data initially, not when we process it.

[JohnHalleyGotway]

@j-opatz I see that support for the convert(x) and censor logic was added to the regrid dictionary by MET issue #1293 for MET version 9.1. I read through that issue but don't recall or understand the motivation for it. Perhaps @CPKalb does?

It's certainly easy enough to back out that change. But I worry about removing functionality without knowing why we added it in the first place. Instead, I'd rather limit the scope of the convert and censor operations within the regrid dictionary.

When parsing those entries from the regrid dictionary, DISABLE the config file contextual lookup. So if they're not defined within the regrid dictionary itself, they won't be found. That'll avoid the unexpected behavior the user found, but still preserve the functionality added by #1293.