Feature #1943 gridstat_seeps: Load SEEPS climo only if SEEPS is enabled #2368

hsoh-u · 2022-12-05T19:33:18Z

Expected Differences

The SEEPS climo file should not be loaded if thre SEESP is not enabled.

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:

Manually modified unittest commands and check if SEESP climo file loaded with debug level 7

grid_stat: Checking the log message:

DEBUG 7: SeepsClimoGrid::get_seeps_climo_filename() -> SEEPS climo name="/d1/personal/hsoh/git/features/feature_1943_gridstat_seeps/MET/share/met/climo/seeps/PPT24_seepsweights_grid.nc

from the commands (with/without SEEPS)

export MET_SEEPS_GRID_CLIMO_NAME='${MET_TEST_INPUT}/climatology_data/seeps/PPT24_seepsweights_grid.nc'
export OMP_NUM_THREADS='2'
export OUTPUT_PREFIX='SEEPS'
export SEEPS_FLAG='NONE'
export SEEPS_P1_THRESH='NA'
/d1/personal/hsoh/git/features/feature_1943_gridstat_seeps/MET/share/met/../../bin/grid_stat \
      /d1/projects/MET/MET_test_data/unit_test/model_data/seeps/gpm_2021120100_2021120200_trmmgrid.nc \
      /d1/projects/MET/MET_test_data/unit_test/model_data/seeps/prods_op_gl-mn_2021120100_f024_A24_trmmgrid.nc \
      /d1/personal/hsoh/git/features/feature_1943_gridstat_seeps/MET/internal/test_unit/config/GridStatConfig_SEEPS \
      -outdir /d1/personal/hsoh/MET/test_output/feature_1943_gridstat_seeps/grid_stat -v 7
unset MET_SEEPS_GRID_CLIMO_NAME
unset OMP_NUM_THREADS
unset OUTPUT_PREFIX
unset SEEPS_FLAG
unset SEEPS_P1_THRESH

export MET_SEEPS_GRID_CLIMO_NAME='${MET_TEST_INPUT}/climatology_data/seeps/PPT24_seepsweights_grid.nc'
export OMP_NUM_THREADS='2'
export OUTPUT_PREFIX='SEEPS'
export SEEPS_FLAG='BOTH'
export SEEPS_P1_THRESH='NA'
/d1/personal/hsoh/git/features/feature_1943_gridstat_seeps/MET/share/met/../../bin/grid_stat \
      /d1/projects/MET/MET_test_data/unit_test/model_data/seeps/gpm_2021120100_2021120200_trmmgrid.nc \
      /d1/projects/MET/MET_test_data/unit_test/model_data/seeps/prods_op_gl-mn_2021120100_f024_A24_trmmgrid.nc \
      /d1/personal/hsoh/git/features/feature_1943_gridstat_seeps/MET/internal/test_unit/config/GridStatConfig_SEEPS \
      -outdir /d1/personal/hsoh/MET/test_output/feature_1943_gridstat_seeps/grid_stat -v 7
unset MET_SEEPS_GRID_CLIMO_NAME
unset OMP_NUM_THREADS
unset OUTPUT_PREFIX
unset SEEPS_FLAG
unset SEEPS_P1_THRESH

point_stat:

DEBUG 7: SeepsClimo::get_seeps_climo_filename() -> SEEPS climo name="/d1/personal/hsoh/git/features/feature_1943_gridstat_seeps/MET/share/met/climo/seeps/PPT24_seepsweights.nc"

from the commands (with/without SEEPS)

export BEG_DS='-1800'
export END_DS='1800'
export FCST_FIELD_LEVEL='(*,*)'
export FCST_FIELD_NAME='APCP_24'
export OBS_DICT='{ field = [ { name  = "TP24"; level = "L0"; is_precipitation = TRUE; } ]; }'
export OUTPUT_PREFIX='NCMET_NAM_NDAS_SEEPS'
export SEEPS_FLAG='NONE'
export SEEPS_P1_THRESH='ge0.1&&le0.85'
/d1/personal/hsoh/git/features/feature_1943_gridstat_seeps/MET/share/met/../../bin/point_stat \
      /d1/projects/MET/MET_test_data/unit_test/model_data/met_nc/nam/nam_2012040900_F036_APCP24.nc \
      /d1/personal/hsoh/MET/test_output/feature_1943_gridstat_seeps/pb2nc/ndas.20120410.t12z.prepbufr.tm00.nc \
      /d1/personal/hsoh/git/features/feature_1943_gridstat_seeps/MET/internal/test_unit/config/PointStatConfig_APCP \
      -outdir /d1/personal/hsoh/MET/test_output/feature_1943_gridstat_seeps/point_stat -v 7
unset BEG_DS
unset END_DS
unset FCST_FIELD_LEVEL
unset FCST_FIELD_NAME
unset OBS_DICT
unset OUTPUT_PREFIX
unset SEEPS_FLAG
unset SEEPS_P1_THRESH

export BEG_DS='-1800'
export END_DS='1800'
export FCST_FIELD_LEVEL='(*,*)'
export FCST_FIELD_NAME='APCP_24'
export OBS_DICT='{ field = [ { name  = "TP24"; level = "L0"; is_precipitation = TRUE; } ]; }'
export OUTPUT_PREFIX='NCMET_NAM_NDAS_SEEPS'
export SEEPS_FLAG='BOTH'
export SEEPS_P1_THRESH='ge0.1&&le0.85'
/d1/personal/hsoh/git/features/feature_1943_gridstat_seeps/MET/share/met/../../bin/point_stat \
      /d1/projects/MET/MET_test_data/unit_test/model_data/met_nc/nam/nam_2012040900_F036_APCP24.nc \
      /d1/personal/hsoh/MET/test_output/feature_1943_gridstat_seeps/pb2nc/ndas.20120410.t12z.prepbufr.tm00.nc \
      /d1/personal/hsoh/git/features/feature_1943_gridstat_seeps/MET/internal/test_unit/config/PointStatConfig_APCP \
      -outdir /d1/personal/hsoh/MET/test_output/feature_1943_gridstat_seeps/point_stat -v 7
unset BEG_DS
unset END_DS
unset FCST_FIELD_LEVEL
unset FCST_FIELD_NAME
unset OBS_DICT
unset OUTPUT_PREFIX
unset SEEPS_FLAG
unset SEEPS_P1_THRESH

Recommend testing for the reviewer(s) to perform, including the location of input datasets, and any additional instructions:
Do these changes include sufficient documentation updates, ensuring that no errors or warnings exist in the build of the documentation? [No]
Do these changes include sufficient testing updates? [No]
Will this PR result in changes to the test suite? [No]

If yes, describe the new output and/or changes to the existing output:
Please complete this pull request review by [Fill in date].

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)
Select: Organization level software support Project or Repository level development cycle Project
Select: Milestone as the version that will include these changes
After submitting the PR, select Development 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.

JohnHalleyGotway · 2022-12-05T21:14:34Z

src/libcode/vx_statistics/pair_data_point.cc

+      for(int j=0; j < n_mask; j++){
+         for(int k=0; k < n_interp; k++){
+            pd[i][j][k].load_seeps_climo();
+            loaded = true;


@hsoh-u, I think PairDataPoint::load_seeps_climo() is just setting a pointer to a single instance of the SEEPS climo info. But that pointer should be set for each PairDataPoint object. So I don't think you should be breaking out of the loop based on the "loaded". You could just remove all the "loaded" stuff, as I've recommended here.

Suggested change

loaded = true;

JohnHalleyGotway · 2022-12-05T21:14:44Z

src/libcode/vx_statistics/pair_data_point.cc

@@ -1470,6 +1478,23 @@ void VxPairDataPoint::set_obs_perc_value(int percentile) {

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

+void VxPairDataPoint::load_seeps_climo() {
+   bool loaded = false;


Suggested change

bool loaded = false;

JohnHalleyGotway · 2022-12-05T21:14:59Z

src/libcode/vx_statistics/pair_data_point.cc

+            loaded = true;
+            break;
+         }
+         if (loaded) break;


Suggested change

if (loaded) break;

JohnHalleyGotway · 2022-12-05T21:15:08Z

src/libcode/vx_statistics/pair_data_point.cc

+         }
+         if (loaded) break;
+      }
+      if (loaded) break;


Suggested change

if (loaded) break;

You are right. The singleton is implemented at vx_seeps/seeps.cc

… MET_SEEPS_GRID_CLIMO_NAME or MET_SEEPS_POINT_CLIMO_NAME environment variables.

JohnHalleyGotway

I approve of these changes.

I see that all the GHA tests ran without error and produced no diffs.
I inspected the code changes and see that load_seeps_climo() is only called when the SEEPS or SEEPS_MPR output line types are requested.
I ran Point-Stat at verbosity 10 WITHOUT seeps requested and see no seeps log messages.
Rerunning WITH seeps = BOTH; logs:

DEBUG 7: SeepsClimo::get_seeps_climo_filename() -> SEEPS climo name="/Volumes/d1/projects/MET/MET_development/MET-develop/share/met/climo/seeps/PPT24_seepsweights.nc"
DEBUG 6: SeepsClimo::read_records() -> dimensions nstn = 5293
DEBUG 6: SeepsClimo::read_records() -> took 0.074679 seconds

Running Grid-Stat with seeps = NONE; produces no seeps log messages.
Running Grid-Stat with seeps = STAT; only produces seeps log messages when actually processing precip data.

Note that with this commit, I updated seeps-related error messages to tell users exactly what environment variables they can set.

#1943 Load SEEPS climo only if SEEPS is enabled

6700632

hsoh-u requested a review from JohnHalleyGotway December 5, 2022 19:33

#1943 Corrected path for PPT24_seepsweights.nc

ad89e66

JohnHalleyGotway changed the title ~~#1943 Load SEEPS climo only if SEEPS is enabled~~ Feature #1943 gridstat_seeps: Load SEEPS climo only if SEEPS is enabled Dec 5, 2022

JohnHalleyGotway added this to the MET 11.0.0 milestone Dec 5, 2022

JohnHalleyGotway linked an issue Dec 5, 2022 that may be closed by this pull request

Enhance Grid-Stat to compute SEEPS for gridded observations and write the SEEPS STAT line type. #1943

Closed

22 tasks

Howard Soh added 2 commits December 5, 2022 12:59

#1943 Corrected path for print_pointnc2ascii.py

31fba07

#1943 SEEPS is not computed if SEEPS climo is not loaded

84efc94

JohnHalleyGotway reviewed Dec 5, 2022

View reviewed changes

#1943 load SEEPS climo if activated

c95fcb4

hsoh-u requested a review from JohnHalleyGotway December 5, 2022 21:33

Howard Soh and others added 2 commits December 5, 2022 14:34

#1943 load SEEPS climo if activated

d9e4b13

Per #1943, tweak log messages for SEEPS pointing users to setting the…

6ce65cf

… MET_SEEPS_GRID_CLIMO_NAME or MET_SEEPS_POINT_CLIMO_NAME environment variables.

JohnHalleyGotway approved these changes Dec 6, 2022

View reviewed changes

hsoh-u merged commit c713d08 into develop Dec 6, 2022

JohnHalleyGotway deleted the feature_1943_gridstat_seeps branch February 28, 2023 19:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature #1943 gridstat_seeps: Load SEEPS climo only if SEEPS is enabled #2368

Feature #1943 gridstat_seeps: Load SEEPS climo only if SEEPS is enabled #2368

hsoh-u commented Dec 5, 2022 •

edited

Loading

JohnHalleyGotway Dec 5, 2022

JohnHalleyGotway Dec 5, 2022

JohnHalleyGotway Dec 5, 2022

JohnHalleyGotway Dec 5, 2022

hsoh-u Dec 5, 2022

JohnHalleyGotway left a comment

Feature #1943 gridstat_seeps: Load SEEPS climo only if SEEPS is enabled #2368

Feature #1943 gridstat_seeps: Load SEEPS climo only if SEEPS is enabled #2368

Conversation

hsoh-u commented Dec 5, 2022 • edited Loading

Expected Differences

Pull Request Testing

Pull Request Checklist

JohnHalleyGotway Dec 5, 2022

Choose a reason for hiding this comment

JohnHalleyGotway Dec 5, 2022

Choose a reason for hiding this comment

JohnHalleyGotway Dec 5, 2022

Choose a reason for hiding this comment

JohnHalleyGotway Dec 5, 2022

Choose a reason for hiding this comment

hsoh-u Dec 5, 2022

Choose a reason for hiding this comment

JohnHalleyGotway left a comment

Choose a reason for hiding this comment

hsoh-u commented Dec 5, 2022 •

edited

Loading