-
Notifications
You must be signed in to change notification settings - Fork 119
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[develop] Add verification of snowfall accumulation #853
[develop] Add verification of snowfall accumulation #853
Conversation
…/ufs-srweather-app into feature/snowfall_vx
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@willmayfield I have gone over your changes in this PR and overall everything looks pretty good. I have noted a few spelling/grammatical issues and had a question regarding the NOHRSC_OBS_DIR
path in RunSRW.rst
, but otherwise, everything else looks okay.
I was able to successfully run all of the verification WE2E tests on Hera, as well as the fundamental WE2E tests. I was even able to test the newly added ASNOW option. I do have a question regarding this.
The metatask_PcpCombine_fcst_APCP_all_accums_all_mems
entry correctly sets ACCUM_HH
to 01
, 03
, and 06
for FCST_LEN_HRS
= 6. I noted that the only way that I could get metatask_PcpCombine_fcst_ASNOW_all_accums_all_mems
to set ACCUM_HH
to 06
was by setting FCST_LEN_HRS
to a value of 24 or greater. Is this correct, or should I be able to set FCST_LEN_HRS
= 6 and still have ACCUM_HH
set to 06
? If this is what is supposed to happen, would you mind running a test with FCST_LEN_HRS
= 6 and seeing whether ACCUM_HH
is properly set?
Once the above question has been answered, I think I can move forward with approving these changes.
@MichaelLueken Thank you for your review. I fixed the typos and another duplicate line I noticed in default_workflow.yaml. Good catch for the issue when forecast length is less than 24h. It seems when only one element of the verification.VX_ASNOW_ACCUMS_HRS list is used, it propagates ACCUM_HH without the 2-digit formatting, but I have no idea why, as it should happen in the loop in verify_pre.yaml:
For a 2 hour forecast length:
(the workflow also fails because the ACCUM_HH var is empty for PcpCombine_fcst_ASNOW_all_accums_all_mems, so I will try to find a way to handle that as well, probably by removing those tasks with some logic in setup.py) Maybe @gsketefian or @mkavulich can offer any ideas of why it isn't doing the 2-digit formatting? |
@@ -151,23 +151,31 @@ METplus Parameters | |||
* ``SS`` refers to the two-digit valid seconds of the hour | |||
|
|||
``CCPA_OBS_DIR``: (Default: "") | |||
User-specified location of top-level directory where CCPA hourly precipitation files used by METplus are located. This parameter needs to be set for both user-provided observations and for observations that are retrieved from the NOAA :term:`HPSS` (if the user has access) via the ``GET_OBS_CCPA`` task. (This task is activated in the workflow by using the taskgroup file ``parm/wflow/verify.yaml``). | |||
User-specified location of top-level directory where CCPA hourly precipitation files used by METplus are located. This parameter needs to be set for both user-provided observations and for observations that are retrieved from the NOAA :term:`HPSS` (if the user has access) via the ``GET_OBS_CCPA`` task. (This task is activated in the workflow by using the taskgroup file ``parm/wflow/verify_pre.yaml``). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change the task name to lowercase, i.e. GET_OBS_CCPA
--> get_obs_ccpa
.
``MRMS_OBS_DIR``: (Default: "") | ||
User-specified location of top-level directory where MRMS composite reflectivity files used by METplus are located. This parameter needs to be set for both user-provided observations and for observations that are retrieved from the NOAA :term:`HPSS` (if the user has access) via the ``GET_OBS_MRMS`` task (activated in the workflow automatically when using the taskgroup file ``parm/wflow/verify.yaml``). When pulling observations directly from NOAA HPSS, the data retrieved will be placed in this directory. Please note, this path must be defind as ``/<full-path-to-obs>/mrms/proc``. | ||
User-specified location of top-level directory where MRMS composite reflectivity files used by METplus are located. This parameter needs to be set for both user-provided observations and for observations that are retrieved from the NOAA :term:`HPSS` (if the user has access) via the ``GET_OBS_MRMS`` task (activated in the workflow automatically when using the taskgroup file ``parm/wflow/verify_pre.yaml``). When pulling observations directly from NOAA HPSS, the data retrieved will be placed in this directory. Please note, this path must be defind as ``/<full-path-to-obs>/mrms/proc``. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change the task name to lowercase, i.e. GET_OBS_MRMS
--> get_obs_mrms
.
|
||
METplus configuration files require the use of a predetermined directory structure and file names. Therefore, if the MRMS files are user-provided, they need to follow the anticipated naming structure: ``{YYYYMMDD}/MergedReflectivityQCComposite_00.50_{YYYYMMDD}-{HH}{mm}{SS}.grib2``, where YYYYMMDD and {HH}{mm}{SS} are as described in the note :ref:`above <METParamNote>`. | ||
|
||
.. note:: | ||
METplus is configured to look for a MRMS composite reflectivity file for the valid time of the forecast being verified; since MRMS composite reflectivity files do not always exactly match the valid time, a script (within the main script that retrieves MRMS data from the NOAA HPSS) is used to identify and rename the MRMS composite reflectivity file to match the valid time of the forecast. The script to pull the MRMS data from the NOAA HPSS has an example of the expected file-naming structure: ``scripts/exregional_get_obs_mrms.sh``. This script calls the script used to identify the MRMS file closest to the valid time: ``ush/mrms_pull_topofhour.py``. | ||
|
||
``NDAS_OBS_DIR``: (Default: "") | ||
User-specified location of the top-level directory where NDAS prepbufr files used by METplus are located. This parameter needs to be set for both user-provided observations and for observations that are retrieved from the NOAA :term:`HPSS` (if the user has access) via the ``GET_OBS_NDAS`` task (activated in the workflow automatically when using the taskgroup file ``parm/wflow/verify.yaml``). When pulling observations directly from NOAA HPSS, the data retrieved will be placed in this directory. Please note, this path must be defined as ``/<full-path-to-obs>/ndas/proc``. METplus is configured to verify near-surface variables hourly and upper-air variables at 00 and 12 UTC with NDAS prepbufr files. | ||
User-specified location of the top-level directory where NDAS prepbufr files used by METplus are located. This parameter needs to be set for both user-provided observations and for observations that are retrieved from the NOAA :term:`HPSS` (if the user has access) via the ``GET_OBS_NDAS`` task (activated in the workflow automatically when using the taskgroup file ``parm/wflow/verify_pre.yaml``). When pulling observations directly from NOAA HPSS, the data retrieved will be placed in this directory. Please note, this path must be defined as ``/<full-path-to-obs>/ndas/proc``. METplus is configured to verify near-surface variables hourly and upper-air variables at 00 and 12 UTC with NDAS prepbufr files. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change the task name to lowercase, i.e. GET_OBS_NDAS
--> get_obs_ndas
.
|
||
METplus configuration files require the use of a predetermined directory structure and file names. If the CCPA files are user-provided, they need to follow the anticipated naming structure: ``{YYYYMMDD}/ccpa.t{HH}z.01h.hrap.conus.gb2``, where YYYYMMDD and HH are as described in the note :ref:`above <METParamNote>`. When pulling observations from NOAA HPSS, the data retrieved will be placed in the ``CCPA_OBS_DIR`` directory. This path must be defind as ``/<full-path-to-obs>/ccpa/proc``. METplus is configured to verify 01-, 03-, 06-, and 24-h accumulated precipitation using hourly CCPA files. | ||
|
||
.. note:: | ||
There is a problem with the valid time in the metadata for files valid from 19 - 00 UTC (i.e., files under the "00" directory). The script to pull the CCPA data from the NOAA HPSS (``scripts/exregional_get_obs_ccpa.sh``) has an example of how to account for this and organize the data into a more intuitive format. When a fix is provided, it will be accounted for in the ``exregional_get_obs_ccpa.sh`` script. | ||
|
||
``NOHRSC_OBS_DIR``: (Default: "") | ||
User-specified location of top-level directory where NOHRSC 06- and 24-hour snowfall accumulation files (available every 6 and 12 hours respectively) used by METplus are located. This parameter needs to be set for both user-provided observations and for observations that are retrieved from the NOAA :term:`HPSS` (if the user has access) via the ``GET_OBS_NOHRSC`` task. (This task is activated in the workflow by using the taskgroup file ``parm/wflow/verify_pre.yaml``). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change the task name to lowercase, i.e. GET_OBS_NOHRSC
--> get_obs_nohrsc
.
@@ -827,18 +831,24 @@ In addition to the baseline tasks described in :numref:`Table %s <WorkflowTasksT | |||
| **Workflow Task** | **Task Description** | | |||
+=======================+============================================================+ | |||
| GET_OBS_CCPA | Retrieves and organizes hourly :term:`CCPA` data from NOAA | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@willmayfield @gspetro This is not directly related to this PR, but we will have to update the names of the vx and related tasks in this table, e.g. GET_OBS_CCPA
becomes get_obs_ccpa
, etc. I think I'll do that as part of Issue #630.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gsketefian I'm actually working on updates now! The code is in my text/ug-updates branch.
RTD version here: https://srw-ug.readthedocs.io/en/text-ug-updates/BuildingRunningTesting/RunSRW.html#vxworkflowtaskstable
I'm generally not touching much of the VX stuff, but since I'm updating the run chapter, I did get update some of the info there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One thing I was trying to decide was whether to name the (meta)tasks based on what is in the YAML config file or based on what the Rocoto/log output is. I went with what is in the YAML because that is the name users may actually add/change/remove in their config file.
@@ -246,13 +246,13 @@ GRID_STAT_OUTPUT_FLAG_NBRCNT = STAT | |||
|
|||
# NetCDF matched pairs output file | |||
#GRID_STAT_NC_PAIRS_VAR_NAME = | |||
GRID_STAT_NC_PAIRS_FLAG_LATLON = TRUE | |||
GRID_STAT_NC_PAIRS_FLAG_RAW = TRUE | |||
GRID_STAT_NC_PAIRS_FLAG_LATLON = FALSE |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just curious why we're turning off GRID_STAT_NC_PAIRS_FLAG_LATLON
, GRID_STAT_NC_PAIRS_FLAG_RAW
, and GRID_STAT_NC_PAIRS_FLAG_NBRHD
. See here for descriptions of these options (or their conterparts in MET).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gsketefian I discussed this with @michelleharrold and we decided it would be best to have these turned off by default. The ncpairs files are useful for checking that verification fields are created correctly, but the files are not needed in order to create the verification statistics. They can be rather large files as well, so it will help with reducing the footprint of the verification WE2E tests.
scripts/exregional_get_obs_nohrsc.sh
Outdated
#----------------------------------------------------------------------- | ||
# | ||
|
||
set -x |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably should remove this set -x
since it adds a lot of clutter to the log file. User can add it when debugging (or you can put this in an if-statement on the DEBUG
or similar flag; or maybe that already exists in $USHdir/preamble.sh
).
FCST_INPUT_DIR="${vx_fcst_input_basedir}" | ||
;; | ||
"ASNOW") | ||
OBS_INPUT_FN_TEMPLATE="${OBS_NOHRSC_ASNOW_FN_TEMPLATE}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@willmayfield I will try to run this PR myself to understand better what's going on, but can you say why FCST_INPUT_DIR
for ASNOW
is set to vx_output_basedir
? Is there a vx pre-processing task (like PCP_COMBINE
for APCPgt01h
) that first needs to run and that places output in vx_output_basedir
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gsketefian Yes, pcpcombine_fcst runs for ASNOW prior to this task.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, just saw that the answer to my question is "yes"!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So what do the PCP_COMBINE
tasks do exactly for ASNOW
? Are the hours in the obs every 6 hours, and those get combined to 12, (not 18?), 24, (not 30?), 36, etc? And same for forecasts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The obs are available as 6- and 24-hour accumulations in grib2 format from nohrsc on HPSS (and those are the accumulations recommended to be verified in the Metrics Spreadsheet). Therefore those are simply retrieved and pcp_bombine_obs is not needed. Note that the 6-h accumulations are available every 6 hours and the 24-h accumulations are available every 12 hours, so I am verifying them at that pace.
The forecasts, on the other hand, are hourly 1-h accumulations output in post. So they are run with pcp_combine_fcst to combine them into the 6- and 24-h accumulations to match the obs.
@willmayfield I've some some additional testing today and it looks like incorporating some of @danielabdi-noaa's changes from PR #701 will allow both ASNOW to properly work with |
…ing shorter forecast lengths with 2-digit ACCUM_HH..
…/ufs-srweather-app into feature/snowfall_vx
@MichaelLueken @gsketefian @gspetro-NOAA thank you for your feedback. I fixed the typos/task names in the documentation, and incorporated the changes to ush/python_utils/environment.py that Michael suggested to solve the issue with ACCUM_HH. I also needed to add the line from ush/set_FV3nml_ens_stoch_seeds.py that is a part of @danielabdi-noaa's PR #701, in order for stochastic physics to run. With these changes the case I ran with ASNOW turned on for a 6 hour forecast was successful. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested these changes as part of my PR #864 and confirmed the snow accumulation verification gives the expected output for a winter case (2023021700).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@willmayfield Thanks for working with me to apply several of @danielabdi-noaa's bug fixes from PR #701 into this work! Testing the current verification WE2E tests have passed, as well as testing ASNOW with a 6 hour forecast. Approving now.
I'll go ahead and launch the automated Jenkins tests now.
Since @gsketefian noted that he is wrapping up his review of these changes, I hold off merging this PR until he has given his approval.
The Cheyenne Intel tests successfully passed on Hera:
Will manually run the Cheyenne GNU tests first thing on Monday, then get this PR merged. |
The Cheyenne GNU tests were ran on Hera using the GNU compiler. All tests successfully passed:
Following either an approval or comment saying that @gsketefian is okay with the changes, this work can get merged. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Glad @MichaelLueken and @willmayfield figured out the ACCUM_HH
bug. Looks good to me and I'm approving. I just have one suggestion. It would be a good idea to either modify one of the WE2E tests to include snowfall accum vx or to add a new one that does this. I've now learned to add tests consistently for features I know at least I (or projects I'm involved with) will use often.
DESCRIPTION OF CHANGES:
This PR completes the final deliverable of the DTC UFS SRW project to include verification support of new variables in the workflow via MET. This PR adds relevant METplus configuration files and modifies various scripts to allow for verification via the NOHRSC gridded accumulated snowfall product. The forecast variable created by post is 'TSNOWP' for 6-and 24-hour snowfall accumulation with a fixed density, which will be verified on a 6- and 12-hour cadence, respectively. When the GSL variable-density snowfall product is included in post in the future, the variable in the METplus configuration files may be changed to 'ASNOW'.
Currently this addition does not turn on accumulated snowfall verification by default. This is because NOHRSC observations were not located on NOAA HPSS previous to March 2020, as well as the idea that many non-winter cases likely will not desire to run the additional verification tasks. This PR achieves removal of snowfall verification tasks by omitting it by default from the VX_FIELDS variable. Therefore, to turn on snowfall verification, users may simply include VX_FIELDS in their configuration with "ASNOW" added.
This PR also turns off all “ncpairs” file creation in METplus configuration files. These files represent the majority of the filespace footprint for METplus outputs, and are not necessary to produce verification statistical output.
Type of change
TESTS CONDUCTED:
MET_verification and MET_ensemble_verification tests were run without error. Note that features were not tested as a part of these because the new "ASNOW" tasks are not run by default, as noted above. A winter case (20220202) was run for a 36 hour ensemble forecast that included "ASNOW" in VX_FIELDS. The experiment directory on hera is in /scratch2/BMC/fv3lam/mayfield/srw_vx/snowfall_vx/expt_dirs/test_community_nohrsc. METplus stat files were reviewed for meaningful statistics, and “ncpairs” files were turned on and reviewed to show that both observations and forecasts as processed by METplus are verifiable.
DEPENDENCIES:
None
DOCUMENTATION:
Documentation was updated to include additional information about NOHRSC observation data in verification sections.
ISSUE:
None
CHECKLIST
REQUEST SPECIFIC REVIEWERS:
@gsketefian @mkavulich