Add METplus-based ensemble verification by michelleharrold · Pull Request #575 · ufs-community/regional_workflow

michelleharrold · 2021-08-20T19:21:35Z

DESCRIPTION OF CHANGES:

This PR adds the capability to perform METplus-based ensemble verification within the regional_workflow. In addition to ensemble verification, modifications were made to the existing verification to allow for deterministic verification of each individual ensemble member.

Created jjobs and ex-scripts for running EnsembleStat, GridStat, and PointStat jobs for ensemble verification. Updated the generation, setup and valid param scripts, and FV3LAM_wflow.xml template to include the ensemble verification tasks (and modify deterministic verification for individual ensemble members). Added MET (EnsembleStat, GridStat, and PointStat) and METplus (APCP, REFC, RETOP, and sfc/upper-air) configuration file templates for ensemble verification. Added envars to the config_defaults.sh to set ensemble-related fields and to turn grid- and point-based ensemble verification on/off (w/ RUN_TASK_VX_ENSGRID and RUN_TASK_VX_ENSPOINT, respectively).

Other small modifications were made to already established deterministic verification-related jjobs, ex-scripts, and MET/METplus configuration files to address adding verification of individual ensemble members, removing unneeded passed arguments, and adding more appropriate logging info.

A WE2E test was added for ensemble verification: tests/WE2E/test_configs/wflow_features/config.MET_ensemble_verification.sh. It is based on config.MET_verification.sh but adds capability to test a 2 member ensemble, including verification. When testing, a small bug was found with the paths to the IC/LBC files on Hera. A fix suggested by Gerard Ketefian was added to tests/WE2E/run_WE2E_tests.sh to correct the path to IC/LBC files on Hera. Paths were also changed for the staged observation files from: /scratch2/BMC/det/UFS_SRW_app/v1p0/obs_data to /scratch2/BMC/det/UFS_SRW_app/develop/obs_data.

This PR also addresses a recent CCPA bug fix (CCPAv4.1.3) that was implemented into production on 5/4/2021. The bug remains in the 00Z directory, 1-hour CCPA data from 7/18/2018-5/4/2021. Logic has been added in the exregional_get_ccpa_files.sh script to address the bug fix.

TESTS CONDUCTED:

Two single case studies were run (2019061500 and 2020061500, 36-h forecasts). This has only been tested on Hera; there are plans to add Cheyenne as an available platform in the near future. Tested with METv10.0.0 and METplusv4.0.0 for RRFS_CONUS_25km domain with FV3_GSD_SAR physics suite.

DEPENDENCIES:

None.

DOCUMENTATION:

Documentation for verification is pending.

CONTRIBUTORS (optional):

Jamie Wolff and Lindsay Blank

…ity_develop

…tat with METplus. Updated MET and METplus config files.

…grid-stat vx. Updated run_gridstatvx script.

…ity_develop

…og/, rather than default logs/.

…ity_develop

…gional_workflow into community_develop

…elop to this feature branch

jwolff-ncar

Provided some inline comments. I am happy to chat about any of these if we need to iterate on them.

jwolff-ncar · 2021-08-23T16:34:22Z


        os.system('cp '+MRMS_PROD_DIR+'/'+valid.strftime('%Y%m%d')+'/dcom/us007003/ldmdata/obs/upperair/mrms/conus/'+MRMS_PRODUCT+'/'+filename1+' '+VALID_DIR+'/'+filename2)
-	os.system('gunzip '+VALID_DIR+'/'+filename2)
+        os.system('gunzip '+VALID_DIR+'/'+filename2)


This change is also associated with PR #571 so they just need to be coordinated depending on timing of which one goes in first.

jwolff-ncar · 2021-08-23T16:38:07Z

@@ -42,6 +42,8 @@ RUN_TASK_GET_OBS_MRMS="FALSE"
 RUN_TASK_GET_OBS_NDAS="FALSE"


Do we want to add anything in this file as a place holder for ensemble runs? Not required, but I mention it for thoughts from others? (Turn off ensemble but list it? For example:)
DO_ENSEMBLE="FALSE"
NUM_ENS_MEMBERS="2"

I think that'd be appropriate.

Thanks - I added this into ush/config.community.sh

jwolff-ncar · 2021-08-23T16:39:35Z

    #
    # Load the .local module file if available for the given task
    #
+set +u


This will also need to be coordinated with PR#571 - still in progress to get to the final settings here.

Since PR #571 is now merged, does the develop branch just need to be merged in?

jwolff-ncar · 2021-08-23T16:43:54Z

-      <taskdep task="&GET_OBS_CCPA_TN;"/>
+      <and>
+        <taskdep task="&GET_OBS_CCPA_TN;"/>
+        {%- if write_dopost %}


Why are some if statements {%- and some not?)

We need {%- ... %} blocks for every "if" statement to avoid unnecessary blank lines in the final XML file.

Thanks for the explanation, Jeff! All if blocks now have {%- ... %}.

jwolff-ncar · 2021-08-23T16:49:49Z

+//
+mask = {
+   grid = [];
+   poly = [ "${MASKS}/EAST.nc",


I think in develop we have this passed in through the METplus conf file and just have this set to CONUS.nc for resource purposes only. Maybe be consistent here (for all these new config files)?

For ensemble-stat, the functionality to pass in the verification mask via METplus conf file did not exist in released versions prior to v4.0.0. Since we updated to run with METplus v4.0.0, I updated our METplus conf files to pass in the vx poly files and removed the hard-coded NC files in the MET config files (but this will not work with versions prior to METplus v4.0.0!).

jwolff-ncar · 2021-08-23T16:56:54Z

@@ -0,0 +1,259 @@
+////////////////////////////////////////////////////////////////////////////////


Does this need a separate _upper_air file?

No, I removed unneeded files, so now we have PointStatConfig_prob and PointStatConfig_mean.

jwolff-ncar · 2021-08-23T16:57:10Z

+mask = {
+   grid = ${POINT_STAT_GRID};
+//   poly = [
+//           "${FIXverif_global}/vx_mask_files/grid2obs/CONUS.nc",


Remove commented out vx regions if this file remains.

This has been removed!

jwolff-ncar · 2021-08-23T17:00:47Z

@@ -0,0 +1,94 @@
+# Ensemble Mean Composite Reflectivity 


Did we decide to include the mean for REFC just as an example or remove it? I don't think we are using it, correct?

Yes! This won't be included in the generation of the workflow, but we decided to keep the functionality (in case we are able to find a good way to configure MET with NEP and NMEP to use mean REFC/RETOP in a meaningful way).

jwolff-ncar · 2021-08-23T17:01:15Z

@@ -0,0 +1,95 @@
+# Ensemble Mean Composite Reflectivity 


Similar to REFC , are we using this one?

Same as REFC (that is, we are not using it).

jwolff-ncar · 2021-08-23T17:04:39Z


 # location of grid_stat MET config file
-GRID_STAT_CONFIG_FILE = {CONFIG_DIR}/GridStatConfig_REFL
+GRID_STAT_CONFIG_FILE = {CONFIG_DIR}/GridStatConfig_REFC


I think this should this be RETOP.

In develop, there is currently only a reflectivity MET config file -- I think since the RETOP and REFC files would be the same, it was decided to only have one? (You might know the history better than me, though!).

JeffBeck-NOAA

Please let me know if any of my comments aren't correct or need clarification!

JeffBeck-NOAA · 2021-08-27T21:53:08Z

+#mkdir_vrfy -p "${ensemblestat_dir}"
+
+#cd_vrfy "${ensemblestat_dir}"
+#


Is this commented block of code above still needed? If not, I would recommend removing it.

Ah, good catch. The commented out section is not needed (commented out for testing purposes but never removed)!

JeffBeck-NOAA · 2021-08-27T21:53:33Z

+#mkdir_vrfy -p "${gridstat_mean_dir}"
+
+#cd_vrfy "${gridstat_mean_dir}"
+#


Same argument as previous comment regarding commented code.

Same response as above -- not needed, will remove.

JeffBeck-NOAA · 2021-08-27T21:54:00Z

+#mkdir_vrfy -p "${gridstat_prob_dir}"
+
+#cd_vrfy "${gridstat_prob_dir}"
+#


Same comment about commented code.

Same response as above -- not needed, will remove.

JeffBeck-NOAA · 2021-08-27T21:54:36Z

+#mkdir_vrfy -p "${ensemblestat_dir}"
+
+#cd_vrfy "${ensemblestat_dir}"
+#


Same comment about commented code block above.

Same response as above -- not needed, will remove.

JeffBeck-NOAA · 2021-08-27T21:54:53Z

+#mkdir_vrfy -p "${pointstat_mean_dir}"
+
+#cd_vrfy "${pointstat_mean_dir}"
+#


Question about commented code block above.

Same response as above -- not needed, will remove.

JeffBeck-NOAA · 2021-08-27T22:34:21Z

 {%- endif %}

+
 {%- if run_task_get_obs_ccpa %}


Extra emply line not needed here?

JeffBeck-NOAA · 2021-08-27T22:34:50Z

-  {%- endif %}
-    </dependency>
-
  </task>


Same question as above?

JeffBeck-NOAA · 2021-08-27T22:35:06Z

-      <metataskdep metatask="&RUN_POST_TN;{{ uscore_ensmem_name }}"/>
-  {%- endif %}
-    </dependency>
-


Same question as above.

JeffBeck-NOAA · 2021-08-27T22:38:07Z

@@ -0,0 +1,129 @@
+# Ensemble Stat
+


Should the following files also be listed as "Examples" in the title like the previous files? Probably should be listed as "Ensemble Stat Precipitation Example" as well.

gsketefian · 2021-09-02T18:40:57Z

+#
+#-----------------------------------------------------------------------
+#
+# Export some environment variables passed in by the XML 


I don't think you need to export these since the XML already does it for you (using <envar>). Any variable declared using <envar> in the xml should be available in any scripts, sub-scripts, functions, nested functions, etc. called by the j-job and ex-script. Maybe try commenting out these lines and see if it still works.

Same comment in other ex-scripts that re-export variables declared in the XML.

@gsketefian - I ran a test with the commenting out the exports that are declared in the XML. Some ended up being necessary, but I did remove the ones that were unnecessary.

gsketefian · 2021-09-02T18:55:19Z

+#-----------------------------------------------------------------------
+#
+if [ ${VAR} == "APCP" ]; then
+  export acc="${ACCUM}h" # for stats output prefix in EnsembleStatConfig


I'm a stickler for not exporting variables if they are only needed locally. That way your environment contains only what you absolutely need and you don't end up using a variable that you inadvertently set in some other script. It looks like acc is used only on line 155 below. If so, can we remove the export in its definition? Thx.

@gsketefian - I removed this.

gsketefian · 2021-09-02T19:07:39Z

@@ -0,0 +1,186 @@
+#!/bin/sh -l


Let's change this to #!/bin/bash and without the -l. because (a) we're using bash; sh usually points to bash but no guarantee; and (2) the -l will source the user's .bashrc or .login (or both; I forget which), and we don't want that (environment should be the same for all users).

Same comment anywhere else this appears.

@gsketefian - I addressed this for all instance it existed for the vx.

gsketefian · 2021-09-03T00:18:00Z

+fhr_last=`echo ${FHR}  | awk '{ print $NF }'`
+export fhr_last
+
+fhr_list=`echo ${FHR} | sed "s/ /,/g"`


Where is fhr_list used? I don't see it anywhere in the rest of this script (except the export command that's next), and I don't see it in the master_metplus.py script (at /contrib/METplus/METplus-4.0.0/ush on Hera) either.

Sorry, I'm still learning how the verification code works. I see now that it is in one of the METPlus configuration files.

gsketefian · 2021-09-07T18:04:22Z

@michelleharrold After looking at the PR a bit more, I think we can make it more compact (and so reduce repeated code) by using some of the capabilities in the workflow. What I mean is:

We can combine JREGIONAL_RUN_VX_ENS[GRID|POINT][_MEAN|_PROB] (a total of 6 files) into one since they are so similar. We can set arguments or environment variables (in the XML) to specify which one of the 6 cases we want to do. We can do the same for the corresponding 6 exregional_... files. I don't think it will be too difficult (I've done this for the get_extrn_[ics|lbcs] tasks -- they use only one j-job and one ex-script even though they're two tasks).
We can use jinja to reduce the number of tasks in ush/templates/FVLAM_wflow.xml (like VX_ENSGRID_MEAN_24h_TN) so that instead of having one for each GRID vs. POINT, cycle hour, and MEAN vs. PROB vs. nothing combination, those parameters are looped over in the XML using jinja syntax. I did not yet carefully compare each of these tasks to see how similar they are, but if the only parameters they differ by are GRID vs. POINT, cycle hour, and MEAN vs. PROB vs. nothing, then they are definitely jinja-fiable! This is pretty much what jinja was designed to help with.

I realize this is a lot of extra work, so I'm happy to help where I can. I can do an example of how to combine the j-jobs and ex-scripts from which you can generalize, and happy to do telecons if that would help. If you need to get this PR in ASAP, we can create new issues to get it later (but it's better to do it now since if we make issues, more than likely we won't get to them for a long time!).

bluefinweiwei · 2021-09-09T17:15:02Z

+export CDATE
+export hh
+
+fhr_last=`echo ${FHR}  | awk '{ print $NF }'`


Is this "fhr_last" the last forecast lead to process?

Based on the syntax I see here, yes.

bluefinweiwei · 2021-09-09T17:47:48Z

+#
+#-----------------------------------------------------------------------
+#
+print_info_msg "$VERBOSE" "Starting ensemble-stat verification"


"Starting ensemble-stat verification" --> "Starting point-stat verification"?

Note: I changed it to say point-based ensemble-stat verification (to be more descriptive)

Thanks - very helpful!

bluefinweiwei · 2021-09-09T17:51:16Z

+print_input_args valid_args
+#-----------------------------------------------------------------------
+#
+# Begin grid-to-grid vx on ensemble output.


Typo? "grid-to-grid" --> "grid-to-point"

bluefinweiwei

As Gerard suggested, it'd be nice if we can combine some of the nearly repeated files into one, for example, the files with APCP01, 03, 06, 24?

michelleharrold · 2021-09-09T20:15:02Z

@bluefinweiwei - a number of the ensemble grid/point tasks can likely be combined, but unfortunately, I don't think the APCP files can (forecast length can change and the settings/accumulations between them are different).

michelleharrold · 2021-09-10T14:59:31Z

@gsketefian - After looking through this a bit more, I 100% agree that there are multiple areas where the ensemble verification could be made more compact (and after looking at some of your suggestions, there are also ways to make the pre-existing deterministic verification more compact as well). After talking with @jwolff-ncar, she advised it would be best to get this PR in ASAP (due to project needs), and as soon as it is merged, we will immediately make a new PR to make the verification more compact. (Tangentially related, there are some METplus config/code changes coming that will require a little bit of an overhaul, some of which will require some significant changes -- that might be a good time to re-evaluate how to make our current set-up more efficient.)

bluefinweiwei

Looks good! @jwolff-ncar @michelleharrold @JeffBeck-NOAA

gsketefian · 2021-09-10T17:42:34Z

@michelleharrold @jwolff-ncar @JeffBeck-NOAA @bluefinweiwei I'll repeat here what I talked about with Jeff so everyone is in the loop.

I attended an EPIC/Raytheon introduction/tutorial yesterday on something called Agile/SAFe. There was a lot of business-speak for sure, but one of the points they were emphatic about is that developers need to build good code the first time around. Going back later to fix things is a definite no-no.

@JeffBeck-NOAA seemed to think that it's possible to use Michelle's fork for the stochastic work. I hope that's the case. The problem with making putting unready code into the develop branch a habit (and hoping you'll fix it later) is that the bad code will build up, and after a few weeks it will be hard for others to understand and modify. It is ok if it's in someone's personal fork and they use it for their research, but when it's something that will be shared with the whole community, it should be high quality. (And I'm not saying that I follow what I'm preaching all the time either, but I try.) It looks like EPIC is going to force this (and other changes in our work habits) on us anyway, and it's going to be an issue most people in the DTC will have to deal with (since most are scientists, not expert programmers).

jwolff-ncar · 2021-09-10T18:22:30Z

@gsketefian Thank you for your thorough review and comments, most of which were addressed. We agree that there are ways to improve the implementation but that is nearly always the case with software development (it is never perfect the first time). This is a high priority item that works as intended and needs to be merged. A subsequent issue will be created to address simplifying the scripts but due to the nature of that work it will be done in a separate PR.

jwolff-ncar

Approve for merge given a new issue will be created to address remaining concerns to enhance/reduce the number of scripts required. Thank you for all your work on this!

* enhancements and bug fix for v0.7.1: 1) max_output_fields = 500 2) run soil/lake surgery using operation rap/hrrr 3) use netcdf parallel for model forecast product IO * Add function to cycle surface and satbias from a common location during machine switch and system version update. Turn on smoke/dust and CLM for EnKF DA cycles. * Add ${APRUN} to all executables in exregional_run_prepstart.sh * Make output file type as an option in configure: WRTCMP_output_file = netcdf_parallel will turn on the parallel netcdf IO and compression. * Only skip cold start gsi on WCOSS2 for now. Cold start GSI crashes on WCOSS2. --------- Co-authored-by: ming hu <ming.hu@clogin07.cactus.wcoss2.ncep.noaa.gov> Co-authored-by: ming hu <ming.hu@dlogin03.dogwood.wcoss2.ncep.noaa.gov>

jwolff-ncar added 30 commits March 2, 2020 18:33

Fix to post flat file.

4d2d117

Merge remote-tracking branch 'upstream/community_develop' into commun…

442c3a3

…ity_develop

Create MET and METplus config files under ush/templates/parm

28693d1

Added script to pull and reorg ccpa data. Added a script to run grids…

1f453dc

…tat with METplus. Updated MET and METplus config files.

Added new jjob for running grid-stat vx. Updated setup.sh to include …

bab2c0d

…grid-stat vx. Updated run_gridstatvx script.

Fixed typo on script name from ksh to sh

9148743

Moved some hard coded items out from the script to the XML

26cb99d

Updates to get METplus to run with fewer hard-coded paths.

f1710bd

Updates to add grid-stat task to XML generation.

c3def15

Bug fixes for adding grid-stat to XML generation

332b127

Merge remote-tracking branch 'upstream/community_develop' into commun…

cf26307

…ity_develop

Merge branch 'community_develop' into feature/add_metplus

473f63a

Updates to remove hard-coded paths in config files

d4c26f0

Change log dir to put master_metplus log file with other logs under l…

0c3133d

…og/, rather than default logs/.

Updates to generate xml without hard-coded paths for MET

ba6a846

Add hera gridstat module file

b6d56e7

Add METplus point-stat task for both sfc and upper air

68f52eb

Small tweaks to remove hard coded paths and add some flexibility

8e7b604

Updates for adding point-stat into auto-generated xml

b297b17

Add in function to set point-stat task to FALSE

c7bffda

Final tweaks to get it to generate the xml correctly

b8ad8fd

Merge remote-tracking branch 'upstream/community_develop' into commun…

36f43d7

…ity_develop

Merge remote-tracking branch 'upstream/community_develop' into commun…

2f92b9b

…ity_develop

Merge branch 'community_develop' of https://github.com/jwolff-ncar/re…

e95481c

…gional_workflow into community_develop

Merge branch 'community_develop' into feature/add_metplus

cc0cf24

Minor updates to run ensure 0,6,12,18

3814d0b

Tweaks to var list for Point-Stat

fa9eb95

Updated files to manually resolve conflicts so I can merge in new dev…

b777999

…elop to this feature branch

Add METplus settings to config_defaults

a432746

Move quote for end of settings and fix extra comment.

bb7ae7a

michelleharrold requested a review from BenjaminBlake-NOAA as a code owner August 20, 2021 19:21

jwolff-ncar reviewed Aug 23, 2021

View reviewed changes

michelleharrold added 3 commits August 27, 2021 14:57

Mods to address comments on PR575.

f50c21e

Sync with develop.

fc336a5

Updated ensemble METPlus conf files for changes to post output name.

bb664e6

JeffBeck-NOAA reviewed Aug 27, 2021

View reviewed changes

gsketefian reviewed Sep 2, 2021

View reviewed changes

gsketefian reviewed Sep 3, 2021

View reviewed changes

bluefinweiwei reviewed Sep 9, 2021

View reviewed changes

Addessed comments in PR and mods for 10-m WIND.

9e668b1

bluefinweiwei approved these changes Sep 10, 2021

View reviewed changes

Sync with develop.

b57d68b

jwolff-ncar approved these changes Sep 15, 2021

View reviewed changes

Addressing final comments in PR.

b03571f

michelleharrold merged commit e0a3743 into ufs-community:develop Sep 16, 2021

This was referenced Sep 16, 2021

Consolidate verification footprint #601

Closed

Update CCPA pull script to handle the bug fix data appropriately #487

Closed

michelleharrold deleted the feature/add_metplus_gridpoint_ensvx branch March 22, 2022 17:37

mkavulich mentioned this pull request Sep 28, 2022

Consolidate verification footprint ufs-community/ufs-srweather-app#381

Closed

		@@ -42,6 +42,8 @@ RUN_TASK_GET_OBS_MRMS="FALSE"
		RUN_TASK_GET_OBS_NDAS="FALSE"

		@@ -0,0 +1,259 @@
		////////////////////////////////////////////////////////////////////////////////

Conversation

michelleharrold commented Aug 20, 2021

DESCRIPTION OF CHANGES:

TESTS CONDUCTED:

DEPENDENCIES:

DOCUMENTATION:

CONTRIBUTORS (optional):

Uh oh!

jwolff-ncar left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JeffBeck-NOAA left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gsketefian Sep 2, 2021 •

edited

Loading

gsketefian Sep 3, 2021 •

edited

Loading