feature 779 add support for output_flag, nc_pairs_flag, and climo_cdf dictionaries in GridStat

[georgemccabe]

To assist with the process for adding support for new MET config variables via METplus wrappers, I wrote a script to give suggestions of what is needed to change to add support. I will paste the output from this script in a comment below.

I also re-organized the CI scripts, adding documentation for the scripts, and removed old scripts that are no longer used.

While doing this work, I learned that some of the default values for output_flag and nc_pairs_flag in the GridStatConfig_wrapped file differ from the defaults in share/met/config/GridStatConfig_default. I had to add additional variables to the use case conf files to replicate the old behavior.

Pull Request Testing

• Describe testing already performed for these changes:
  

Ran diff tests (triggered by ci-run-diff in a commit message) to ensure that the output produced matches the previous runs.

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

One of the latest use case additions (from PR #856) produces PDF output files that are not yet supported in the diff logic. For now, I modified the diff logic to check for the pdf extension and add the files to the diff artifacts so they can be compared by hand.

Review the files in the diff-use_cases_s2s_1-3 artifact to confirm that the PDF output looks correct.
Review the GitHub Actions logs for the pull request run to ensure that the diff logic was executed correctly and there are no differences in the output.

• 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]
  The diff tests should prove that nothing has changed in the output from these changes. I also added pytests for reading the new configs and setting env vars in GridStat and PointStat.

• Will this PR result in changes to the test suite? [No]
  
  Once we are confident that the PDF output produced is correct, we can consider the diff output to be OK.

Pull Request Checklist

See the METplus Workflow for details.

• 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), Project(s), and Milestone
• After submitting the PR, select Linked Issues with the original issue number.
• 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.

[georgemccabe]

The following is output from the doc util script to assist with adding support for new MET config variables. The script can also be run on a single item or list variable to generate similar content (i.e. ~/METplus/metplus/util/doc_util.py grid_stat output_prefix)

~/METplus/metplus/util/doc_util.py grid_stat nc_pairs_flag latlon raw diff climo climo_cdp weight nbrhd fourier gradient distance_map apply_mask

WARNING: Guidance output from this script may differ slightly from the actual steps to take. It is intended to assist the process. The text that is generated should be reviewed for accuracy before adding to codebase.

Wrapper: GridStat
MET Variable: nc_pairs_flag
Dictionary Items:
  latlon
  raw
  diff
  climo
  climo_cdp
  weight
  nbrhd
  fourier
  gradient
  distance_map
  apply_mask

==================================================



In the grid_stat_wrapper.py file, in the GridStatWrapper class, add the following to the WRAPPER_ENV_VAR_KEYS class variable list:


        'METPLUS_NC_PAIRS_FLAG_DICT',



==================================================

In the create_c_dict function for GridStatWrapper, add a function call to read the new METplus config variables and save the value to be added to the wrapped MET config file.


Typically a function is written to handle MET config dictionary items. Search for functions that start with handle_ in CommandBuilder or other parent class wrappers to see if a function already exists for the item you are adding or to use as an example to write a new one.



==================================================

Add the new variables to the basic use case example for the tool,
i.e. parm/use_cases/met_tool_wrapper/GridStat/GridStat.conf:


#GRID_STAT_NC_PAIRS_FLAG_LATLON =
#GRID_STAT_NC_PAIRS_FLAG_RAW =
#GRID_STAT_NC_PAIRS_FLAG_DIFF =
#GRID_STAT_NC_PAIRS_FLAG_CLIMO =
#GRID_STAT_NC_PAIRS_FLAG_CLIMO_CDP =
#GRID_STAT_NC_PAIRS_FLAG_WEIGHT =
#GRID_STAT_NC_PAIRS_FLAG_NBRHD =
#GRID_STAT_NC_PAIRS_FLAG_FOURIER =
#GRID_STAT_NC_PAIRS_FLAG_GRADIENT =
#GRID_STAT_NC_PAIRS_FLAG_DISTANCE_MAP =
#GRID_STAT_NC_PAIRS_FLAG_APPLY_MASK =


==================================================

In the parm/met_config/GridStatConfig_wrapped file, compare the default values set for nc_pairs_flag to the version in share/met/config/GridStatConfig_default. If they do differ, make sure to add variables to the use case config files so that they produce the same output.


In the parm/met_config/GridStatConfig_wrapped file, replace:


nc_pairs_flag = ...

 with:

//nc_pairs_flag = {
${METPLUS_NC_PAIRS_FLAG_DICT}



==================================================



In docs/Users_Guide/wrappers.rst under GridStat => METplus Configuration section, add:


| :term:`GRID_STAT_NC_PAIRS_FLAG_LATLON`
| :term:`GRID_STAT_NC_PAIRS_FLAG_RAW`
| :term:`GRID_STAT_NC_PAIRS_FLAG_DIFF`
| :term:`GRID_STAT_NC_PAIRS_FLAG_CLIMO`
| :term:`GRID_STAT_NC_PAIRS_FLAG_CLIMO_CDP`
| :term:`GRID_STAT_NC_PAIRS_FLAG_WEIGHT`
| :term:`GRID_STAT_NC_PAIRS_FLAG_NBRHD`
| :term:`GRID_STAT_NC_PAIRS_FLAG_FOURIER`
| :term:`GRID_STAT_NC_PAIRS_FLAG_GRADIENT`
| :term:`GRID_STAT_NC_PAIRS_FLAG_DISTANCE_MAP`
| :term:`GRID_STAT_NC_PAIRS_FLAG_APPLY_MASK`

==================================================



In docs/Users_Guide/wrappers.rst under GridStat => MET Configuration section, add:


**${METPLUS_NC_PAIRS_FLAG_DICT}**

.. list-table::
   :widths: 5 5
   :header-rows: 0

   * - METplus Config(s)
     - MET Config File
   * - :term:`GRID_STAT_NC_PAIRS_FLAG_LATLON`
     - nc_pairs_flag.latlon
   * - :term:`GRID_STAT_NC_PAIRS_FLAG_RAW`
     - nc_pairs_flag.raw
   * - :term:`GRID_STAT_NC_PAIRS_FLAG_DIFF`
     - nc_pairs_flag.diff
   * - :term:`GRID_STAT_NC_PAIRS_FLAG_CLIMO`
     - nc_pairs_flag.climo
   * - :term:`GRID_STAT_NC_PAIRS_FLAG_CLIMO_CDP`
     - nc_pairs_flag.climo_cdp
   * - :term:`GRID_STAT_NC_PAIRS_FLAG_WEIGHT`
     - nc_pairs_flag.weight
   * - :term:`GRID_STAT_NC_PAIRS_FLAG_NBRHD`
     - nc_pairs_flag.nbrhd
   * - :term:`GRID_STAT_NC_PAIRS_FLAG_FOURIER`
     - nc_pairs_flag.fourier
   * - :term:`GRID_STAT_NC_PAIRS_FLAG_GRADIENT`
     - nc_pairs_flag.gradient
   * - :term:`GRID_STAT_NC_PAIRS_FLAG_DISTANCE_MAP`
     - nc_pairs_flag.distance_map
   * - :term:`GRID_STAT_NC_PAIRS_FLAG_APPLY_MASK`
     - nc_pairs_flag.apply_mask


==================================================

In docs/Users_Guide/glossary.rst, add:


   GRID_STAT_NC_PAIRS_FLAG_LATLON
     Specify the value for 'nc_pairs_flag.latlon' in the MET configuration file for GridStat.

     | *Used by:* GridStat

   GRID_STAT_NC_PAIRS_FLAG_RAW
     Specify the value for 'nc_pairs_flag.raw' in the MET configuration file for GridStat.

     | *Used by:* GridStat

   GRID_STAT_NC_PAIRS_FLAG_DIFF
     Specify the value for 'nc_pairs_flag.diff' in the MET configuration file for GridStat.

     | *Used by:* GridStat

   GRID_STAT_NC_PAIRS_FLAG_CLIMO
     Specify the value for 'nc_pairs_flag.climo' in the MET configuration file for GridStat.

     | *Used by:* GridStat

   GRID_STAT_NC_PAIRS_FLAG_CLIMO_CDP
     Specify the value for 'nc_pairs_flag.climo_cdp' in the MET configuration file for GridStat.

     | *Used by:* GridStat

   GRID_STAT_NC_PAIRS_FLAG_WEIGHT
     Specify the value for 'nc_pairs_flag.weight' in the MET configuration file for GridStat.

     | *Used by:* GridStat

   GRID_STAT_NC_PAIRS_FLAG_NBRHD
     Specify the value for 'nc_pairs_flag.nbrhd' in the MET configuration file for GridStat.

     | *Used by:* GridStat

   GRID_STAT_NC_PAIRS_FLAG_FOURIER
     Specify the value for 'nc_pairs_flag.fourier' in the MET configuration file for GridStat.

     | *Used by:* GridStat

   GRID_STAT_NC_PAIRS_FLAG_GRADIENT
     Specify the value for 'nc_pairs_flag.gradient' in the MET configuration file for GridStat.

     | *Used by:* GridStat

   GRID_STAT_NC_PAIRS_FLAG_DISTANCE_MAP
     Specify the value for 'nc_pairs_flag.distance_map' in the MET configuration file for GridStat.

     | *Used by:* GridStat

   GRID_STAT_NC_PAIRS_FLAG_APPLY_MASK
     Specify the value for 'nc_pairs_flag.apply_mask' in the MET configuration file for GridStat.

     | *Used by:* GridStat


==================================================

In internal_tests/pytests/grid_stat/test_grid_stat_wrapper.py, add the following items to the tests to ensure the new items are set properly. Note: if the tool does not have unit tests to check the handling of MET config variables, you will need to add those tests. See grid_stat/test_grid_stat_wrapper.py for an example. Change VALUE to an appropriate value for the variable.


        ({'GRID_STAT_NC_PAIRS_FLAG_LATLON': 'VALUE', },
         {'METPLUS_NC_PAIRS_FLAG_DICT': 'nc_pairs_flag = {latlon = VALUE;}'}),

        ({'GRID_STAT_NC_PAIRS_FLAG_RAW': 'VALUE', },
         {'METPLUS_NC_PAIRS_FLAG_DICT': 'nc_pairs_flag = {raw = VALUE;}'}),

        ({'GRID_STAT_NC_PAIRS_FLAG_DIFF': 'VALUE', },
         {'METPLUS_NC_PAIRS_FLAG_DICT': 'nc_pairs_flag = {diff = VALUE;}'}),

        ({'GRID_STAT_NC_PAIRS_FLAG_CLIMO': 'VALUE', },
         {'METPLUS_NC_PAIRS_FLAG_DICT': 'nc_pairs_flag = {climo = VALUE;}'}),

        ({'GRID_STAT_NC_PAIRS_FLAG_CLIMO_CDP': 'VALUE', },
         {'METPLUS_NC_PAIRS_FLAG_DICT': 'nc_pairs_flag = {climo_cdp = VALUE;}'}),

        ({'GRID_STAT_NC_PAIRS_FLAG_WEIGHT': 'VALUE', },
         {'METPLUS_NC_PAIRS_FLAG_DICT': 'nc_pairs_flag = {weight = VALUE;}'}),

        ({'GRID_STAT_NC_PAIRS_FLAG_NBRHD': 'VALUE', },
         {'METPLUS_NC_PAIRS_FLAG_DICT': 'nc_pairs_flag = {nbrhd = VALUE;}'}),

        ({'GRID_STAT_NC_PAIRS_FLAG_FOURIER': 'VALUE', },
         {'METPLUS_NC_PAIRS_FLAG_DICT': 'nc_pairs_flag = {fourier = VALUE;}'}),

        ({'GRID_STAT_NC_PAIRS_FLAG_GRADIENT': 'VALUE', },
         {'METPLUS_NC_PAIRS_FLAG_DICT': 'nc_pairs_flag = {gradient = VALUE;}'}),

        ({'GRID_STAT_NC_PAIRS_FLAG_DISTANCE_MAP': 'VALUE', },
         {'METPLUS_NC_PAIRS_FLAG_DICT': 'nc_pairs_flag = {distance_map = VALUE;}'}),

        ({'GRID_STAT_NC_PAIRS_FLAG_APPLY_MASK': 'VALUE', },
         {'METPLUS_NC_PAIRS_FLAG_DICT': 'nc_pairs_flag = {apply_mask = VALUE;}'}),

        ({
           'GRID_STAT_NC_PAIRS_FLAG_LATLON': 'VALUE',
           'GRID_STAT_NC_PAIRS_FLAG_RAW': 'VALUE',
           'GRID_STAT_NC_PAIRS_FLAG_DIFF': 'VALUE',
           'GRID_STAT_NC_PAIRS_FLAG_CLIMO': 'VALUE',
           'GRID_STAT_NC_PAIRS_FLAG_CLIMO_CDP': 'VALUE',
           'GRID_STAT_NC_PAIRS_FLAG_WEIGHT': 'VALUE',
           'GRID_STAT_NC_PAIRS_FLAG_NBRHD': 'VALUE',
           'GRID_STAT_NC_PAIRS_FLAG_FOURIER': 'VALUE',
           'GRID_STAT_NC_PAIRS_FLAG_GRADIENT': 'VALUE',
           'GRID_STAT_NC_PAIRS_FLAG_DISTANCE_MAP': 'VALUE',
           'GRID_STAT_NC_PAIRS_FLAG_APPLY_MASK': 'VALUE',
          },
         {'METPLUS_NC_PAIRS_FLAG_DICT': 'nc_pairs_flag = {latlon = VALUE;raw = VALUE;diff = VALUE;climo = VALUE;climo_cdp = VALUE;weight = VALUE;nbrhd = VALUE;fourier = VALUE;gradient = VALUE;distance_map = VALUE;apply_mask = VALUE;}'}),

[JohnHalleyGotway]

@georgemccabe does the 'STAT' here mean that by default all of these output types are turned on? In the default Grid-Stat config file, they are all turned off by setting them to 'NONE':
https://github.com/dtcenter/MET/blob/5b3d8f7a15879bc49e3a0ebf8b75672da4568327/met/data/config/GridStatConfig_default#L210

So the user must actually decide what sort of output they want.

[georgemccabe]

@JohnHalleyGotway these are the unit tests. This list of tuples is applied to each run of the test function as arguments. The first tuple item is a dictionary of METplus config variable names and their values. The 2nd tuple item is the environment variable that will be set and what it expected to be set to. All other environment variables that start with METPLUS_ in the list that gets set for each wrapper is assumed to be an empty string unless specified in the 2nd dictionary. This example is showing that if you set the value of GRID_STAT_OUTPUT_FLAG_FHO to 'STAT' then it will set the METPLUS_OUTPUT_FLAG_DICT value appropriately to override the default setting, in which fho = NONE;

[JohnHalleyGotway]

Ah, thanks for explaining that. So is there a "default" METplus value for these config file settings? Or no, the default MET config settings apply... unless the user specifies something in their METplus conf file, then it'll be defined by METplus and override whatever the default MET setting is. Is that correct?

[georgemccabe]

The default behavior if none of the GRID_STAT_OUTPUT_FLAG_* variables are set is to set METPLUS_OUTPUT_FLAG_DICT to an empty string. This results in all of the default values from the share/met/config file. If a single flag is set in the METplus config, only that value overrides the default value and the rest of the defaults are still used.

[JohnHalleyGotway]

In reviewing the file differences, I noted that:

• Perhaps diff-pdf (https://vslavik.github.io/diff-pdf/) or something like it could provide a way to diff PDF files?
• Many of the 57 diff files were either deleted/renamed, had only comments added, or had superficial changes (such as changing paths relative to $basedir).
• Many updates to glossary.rst... all look fine.
• Looks like doc_util.py was factored out of met_util.py.
• Many changes to handle METplus setting the climo_cdf, output_flag, and nc_pairs dictionaries.
• Many changes to run_use_cases.py.
• Many changes to use case conf files... will need to check the GHA output to make sure no differences were introduced. I visually compared 4 PDF's and 1 PNG from the S2S output and saw no discernible differences.

These changes look fine to me and continue to produce the same output as before. I approve.