[develop] Verification upgrades and bug fixes #973
Conversation
…iable fieldname_in_MET_filedir_names (as it is with APCP).
…UPA, this time in METplus conf files.
… ">=", ">", "<=", and "<" to "ge", "gt", "le", and "lt".
… variables are grouped together, (2) commented-out settings of FCST_VAR<n>_... and OBS_VAR<n>_... are removed, (3) FCST_VAR<n>_OPTIONS and OBS_VAR<n>_OPTIONS come last, and (4) each element in options is on a separate line.
…tool names to template METplus conf files as variables; add new workflow variable that specifies the METplus verbosity level.
…ks for APCP (i.e. not just for accumulation > 1 hr but also for accumulation = 1 hour) use obs and forecast tasks that have been processed by the PcpCombine tool of METplus. These files are all in NetCDF format and are generated from grib2 input files (both for obs and forecast). This involves: * Renaming certain workflow variables for verification so their role/purpose is clearer. * Modifying the names of the arrays for APCP variables in the NetCDF files. * Changing the names of the levels associated with APCP variables in the NetCDF files (e.g. "A1" instead of "A01"). * For consistency, making changes for accumulated snow (ASNOW) analogous to the above changes for APCP. * In ex-scripts for vx tasks, removing if-statement that treats the case of APCP accumulation period > 1 hour separately and performing the same steps for both accumulation = 1 hour and > 1 hour.
…should be replaced with [OBS|FCST]_PCP_COMBINE_INPUT_DATATYPE. Thus, they are only relevant to METplus conf files that run PcpCombine. Thus, remove all use of these variables in conf files that do not run PcpCombine, and in the ones that do, replace them with the appropriate new variables (some already have the new variables).
…lates for APCP01h and APCPgt01h.
…s for combining METplus conf files for APCP01h and APCPgt01h (inadvertantly left out of a previous commit).
…f files no longer differentiate between 1 hour accumulation and > 1 hour accumulation.
…_NATIVE_DATA_TYPE.
MichaelLueken
added
bug
…h (that is used for PR ufs-community#973 into ufs-srweather-app) into the feature/nep_nmep branch.
… to compare ADP[SFC|UPA] conf files with other branches.
… created using other branches.
There was a problem hiding this comment.
Did couple of tests on Hercules:
----------------------------------------------------------------------------------------------------
Experiment name | Status | Core hours used
----------------------------------------------------------------------------------------------------
grid_RRFS_CONUS_25km_ics_NAM_lbcs_NAM_suite_GFS_v16 COMPLETE 29.85
grid_RRFS_CONUS_25km_ics_FV3GFS_lbcs_RAP_suite_RAP COMPLETE 13.24
MET_verification_only_vx COMPLETE 0.27
grid_RRFS_CONUS_13km_ics_FV3GFS_lbcs_FV3GFS_suite_GFS_v16_plot COMPLETE 32.35
grid_SUBCONUS_Ind_3km_ics_FV3GFS_lbcs_FV3GFS_suite_WoFS_v0 COMPLETE 21.12
----------------------------------------------------------------------------------------------------
Total COMPLETE 96.83
Approved.
| OBS_VAR1_THRESH = >=20.0 | ||
| OBS_VAR1_OPTIONS = censor_thresh = lt-20.0; censor_val = -20.0; cnt_thresh = [ >15 ]; cnt_logic = UNION; convert(x) = x * 3280.84 * 0.001; | ||
| OBS_VAR1_THRESH = ge20 | ||
| OBS_VAR1_OPTIONS = censor_thresh = lt-20.0; |
There was a problem hiding this comment.
Are these copy-pasted from the REFC values? I suppose it does make sense to censor negative values since they don't make physical sense (outside of really edge cases that I'm not sure ever realistically take place), but maybe that should be lt0.0?
There was a problem hiding this comment.
@mkavulich This is the same as in the original file except each individual option is on a separate line to make the options easier to read and not have one super-long line. The splitting of the options onto separate lines is also in anticipation of the next PR, which jinja-fies things a lot. I didn't change any values here, just kept whatever we've inherited from the time Michelle/Jamie/Evan put these files together.
|
|
||
| #FCST_VAR8_NAME = RETOP_L0_ENS_FREQ_ge50 | ||
| FCST_VAR8_NAME = {{fieldname_in_met_output}}_L0_ENS_FREQ_ge50 |
There was a problem hiding this comment.
We should maybe consider GE0 verification here too...though I can't remember if this variable is above sea level or above ground level
There was a problem hiding this comment.
@mkavulich What is GE0?
|
|
||
| #FCST_VAR3_NAME = RETOP_L0_ENS_FREQ_ge40 | ||
| FCST_VAR3_NAME = {{fieldname_in_met_output}}_L0_ENS_FREQ_ge40 | ||
| OBS_VAR2_THRESH = ge30 |
There was a problem hiding this comment.
Is there a reason that all of these different thresholds are considered separate variables here, vs with wind for example where different thresholds are specified for the same variable?
There was a problem hiding this comment.
@mkavulich I noticed this as well but decided not to explore it in too much depth for the purposes of this PR. So I don't understand things completely yet, but here are a couple of hints:
-
The wind case(s) you noticed are in the deterministic PointStat files, i.e.
PointStat_(ADP)[SFC|UPA].conf, whereas this file is for processing of ensemble probabilistic fields. You can also see the thresholds grouped together for deterministic RETOP processing inGridStat_REFC.conf, where there are lines like this:FCST_VAR1_THRESH = {{field_thresholds}}and
{{field_thresholds}}is a list of values set in the ex-script, e.g. set to'ge20, ge30, ge40, ge50'for RETOP. -
Here, a specific forecast array named
{{fieldname_in_met_output}}_L0_ENS_FREQ_ge30is being read in from the NetCDF file generated by GenEnsProd (where{{fieldname_in_met_output}}is equal to RETOP). Note that the stringge30is hard-coded in this name. IfOBS_VAR2_THRESHwere set to a list of thresholds instead, then we would somehow need to specify a list of array names forFCST_VAR2_NAME(and some of the other variables likeOBS_VAR2_NAMEwould have to become lists as well). I don't know if METplus can handle that. Maybe it can, but I didn't explore further.
Maybe @michelleharrold has a better answer. If there is a more compact way to express what we want done, I'd use it.
ush/config_defaults.yaml
Outdated
| # "ASNOW" may be added to this list in order to include | ||
| # the related verification tasks in the workflow. | ||
| # accumulated snow (ASNOW) is often not of interest in non-winter cases | ||
| # and because observation files for ASNOW are not availabe on NOAA HPSS |
There was a problem hiding this comment.
| # and because observation files for ASNOW are not availabe on NOAA HPSS | |
| # and because observation files for ASNOW are not available on NOAA HPSS |
I didn't realize that info was available (easily?). Where can one see the line number change for a PR? There will be a much larger reduction of lines in my next PR :) |
|
At the top of the PR, on the right hand most side, there are green numbers with a plus and red numbers with a minus. The green plus signifies the number of added lines in a PR, while the red minus represents the number of lines removed. For this PR, I see the following in the top right side:
so there were 1,394 added lines, and 4,133 removed lines in this PR. |
MichaelLueken
added
the
run_we2e_coverage_tests
|
The WE2E coverage tests were manually run on Derecho and all successfully passed: |
Oh right, thanks @MichaelLueken! |
|
@JeffBeck-NOAA @RatkoVasic-NOAA @mkavulich Thanks for the reviews! |
|
@gsketefian - All of the tests passed, with the exception of two tests on Jet:
The Jenkins workspace on Jet can be found: |
|
@MichaelLueken Thanks for the update Mike. The PR doesn't touch the |
|
The two tests that had failed on Jet - |
|
@gsketefian - Given that @christinaholtNOAA's PR #994 was approved and tested first, I merged that PR first. Changes were made to to the ex-scripts to transition to UW's CLI command line tool, which kicked off conflicts in these scripts in your branch. Please merge the current authoritative develop into your |
|
While attempting to run one last batch of verification tests, specifically running @mkavulich's new |
There was a problem hiding this comment.
Several issues have been identified with the new winter weather verification WE2E test, MET_ensemble_verification_winter_wx, that @mkavulich has introduced as part of PR #997.
- The
VX_FIELDSintests/WE2E/test_configs/verification/config.MET_ensemble_verification_winter_wx.yamlneeds to be updated to useVX_FIELDS: [ "APCP", "REFC", "RETOP", "ADPSFC", "ADPUPA", "ASNOW" ], rather thanVX_FIELDS: [ "APCP", "REFC", "RETOP", "SFC", "UPA", "ASNOW" ] - The
run_MET_GenEnsProd_vx_ASNOW06htask is failing. I see the following in thelog/metplus.log.GenEnsProd_ASNOW06hlogfile:
WARNING:
WARNING: process_ensemble() -> ensemble field "ASNOW/A06" not found in file "/work/noaa/epic/mlueken/ufs-srweather-app/expt_dirs/MET_ensemble_verification_winter_wx/2022020300/mem001/metprd/PcpCombine_fcst/srw.t00z.prslev.f006.rrfs_conuscompact_25km_ASNOW_a06h.nc"
WARNING:
WARNING:
WARNING: process_ensemble() -> ensemble field "ASNOW/A06" not found in file "/work/noaa/epic/mlueken/ufs-srweather-app/expt_dirs/MET_ensemble_verification_winter_wx/2022020300/mem002/metprd/PcpCombine_fcst/srw.t00z.prslev.f006.rrfs_conuscompact_25km_ASNOW_a06h.nc"
WARNING:
WARNING:
WARNING: process_ensemble() -> ensemble field "ASNOW/A06" not found in file "/work/noaa/epic/mlueken/ufs-srweather-app/expt_dirs/MET_ensemble_verification_winter_wx/2022020300/mem003/metprd/PcpCombine_fcst/srw.t00z.prslev.f006.rrfs_conuscompact_25km_ASNOW_a06h.nc"
WARNING:
WARNING:
WARNING: process_ensemble() -> ensemble field "ASNOW/A06" not found in file "/work/noaa/epic/mlueken/ufs-srweather-app/expt_dirs/MET_ensemble_verification_winter_wx/2022020300/mem004/metprd/PcpCombine_fcst/srw.t00z.prslev.f006.rrfs_conuscompact_25km_ASNOW_a06h.nc"
WARNING:
WARNING:
WARNING: process_ensemble() -> ensemble field "ASNOW/A06" not found in file "/work/noaa/epic/mlueken/ufs-srweather-app/expt_dirs/MET_ensemble_verification_winter_wx/2022020300/mem005/metprd/PcpCombine_fcst/srw.t00z.prslev.f006.rrfs_conuscompact_25km_ASNOW_a06h.nc"
WARNING:
WARNING:
WARNING: process_ensemble() -> ensemble field "ASNOW/A06" not found in file "/work/noaa/epic/mlueken/ufs-srweather-app/expt_dirs/MET_ensemble_verification_winter_wx/2022020300/mem006/metprd/PcpCombine_fcst/srw.t00z.prslev.f006.rrfs_conuscompact_25km_ASNOW_a06h.nc"
WARNING:
WARNING:
WARNING: process_ensemble() -> ensemble field "ASNOW/A06" not found in file "/work/noaa/epic/mlueken/ufs-srweather-app/expt_dirs/MET_ensemble_verification_winter_wx/2022020300/mem007/metprd/PcpCombine_fcst/srw.t00z.prslev.f006.rrfs_conuscompact_25km_ASNOW_a06h.nc"
WARNING:
WARNING:
WARNING: process_ensemble() -> ensemble field "ASNOW/A06" not found in file "/work/noaa/epic/mlueken/ufs-srweather-app/expt_dirs/MET_ensemble_verification_winter_wx/2022020300/mem008/metprd/PcpCombine_fcst/srw.t00z.prslev.f006.rrfs_conuscompact_25km_ASNOW_a06h.nc"
WARNING:
WARNING:
WARNING: process_ensemble() -> ensemble field "ASNOW/A06" not found in file "/work/noaa/epic/mlueken/ufs-srweather-app/expt_dirs/MET_ensemble_verification_winter_wx/2022020300/mem009/metprd/PcpCombine_fcst/srw.t00z.prslev.f006.rrfs_conuscompact_25km_ASNOW_a06h.nc"
WARNING:
WARNING:
WARNING: process_ensemble() -> ensemble field "ASNOW/A06" not found in file "/work/noaa/epic/mlueken/ufs-srweather-app/expt_dirs/MET_ensemble_verification_winter_wx/2022020300/mem010/metprd/PcpCombine_fcst/srw.t00z.prslev.f006.rrfs_conuscompact_25km_ASNOW_a06h.nc"
WARNING:
ERROR :
ERROR : process_ensemble() -> 0 of 10 (0) fields found for "ASNOW/A06" does not meet the threshold specified by "ens.ens_thresh" (0.05) in the configuration file.
ERROR :
Both of these issues will need to be corrected with the newly added MET_ensemble_verification_winter_wx test before I can move forward with merging this PR in.
|
@MichaelLueken I encountered those problems as well with test to I made this change in However, I also found a stealthy bug in What it should be is: So I think the thresholds for the obs and forecasts were not matching. So although the I'm rerunning the test now to make sure it works from scratch and will then push my fixes. |
|
@MichaelLueken @willmayfield I reran the Please feel free to retest and merge. Thanks. |
There was a problem hiding this comment.
@gsketefian - Thank you very much for addressing the failures in the new test and finding a new bug in the configuration files! I was able to successfully run the MET_ensemble_verification_winter_wx WE2E test:
----------------------------------------------------------------------------------------------------
Experiment name | Status | Core hours used
----------------------------------------------------------------------------------------------------
MET_ensemble_verification_winter_wx_20240112093529 COMPLETE 158.23
----------------------------------------------------------------------------------------------------
Total COMPLETE 158.23
Reapproving PR and retesting following the latest changes.
@willmayfield - Please let me know if you are okay with these changes at your earliest convenience. Thanks!
|
@gsketefian - Here is the current update on the retesting for this PR: The WE2E coverage tests on Gaea have completed successfully: The WE2E coverage tests on Gaea C5 have completed successfully: The WE2E coverage tests on Hera GNU have completed successfully: The WE2E coverage tests on Hera Intel have completed successfully: The WE2E coverage tests on Hercules have completed successfully: The tests are still running on both Jet and Orion. |
|
The WE2E coverage tests have successfully passed on Jet: Still awaiting completion on Orion. |
|
@MichaelLueken @gsketefian I tried it again and everything worked fine! I'm good with the changes. I was worried that something was wrong with these results, but I now know that the problem was the model/physics giving unrealistic results on this test case, and not something due to this PR. |
|
The WE2E coverage tests have successfully passed on Orion: Given @willmayfield's continued approval after retesting these changes, I will now move forward with merging this PR. |
|
@willmayfield @MichaelLueken Thanks for working on this! |
DESCRIPTION OF CHANGES:
This PR cleans up and simplifies the verification tasks in the SRW App. Main changes:
GridStat_ensprob_ASNOW.conf. There is an inadvertent shift in the threshold values used in the forecast field array names with respect to the threshold values specified for the observations. Fix to make thresholds for forecast and obs match.Type of change
TESTS CONDUCTED:
The set of fundamental WE2E tests as well as all the verification tests were run on Hera with Intel. All completed successfully. The fundamental tests are:
The verification tests are:
Manual regression tests were also run on the following WE2E tests:
All had minor expected differences in results relative to the
developbranch. There was a major difference in output (stat files) from therun_MET_GridStat_vx_ensprob_ASNOW06htask of theMET_ensemble_verification_winter_wx, but that is due to the bug fix inGridStat_ensprob_ASNOW.confregarding the mismatch between forecast and obs thresholds (and is thus expected).DEPENDENCIES:
None
CHECKLIST
LABELS (optional):
A Code Manager needs to add the following labels to this PR:
CONTRIBUTORS (optional):
@michelleharrold @JeffBeck-NOAA @willmayfield