[develop] Add workflow for radar reflectivity processing. by christinaholtNOAA · Pull Request #807 · ufs-community/ufs-srweather-app

christinaholtNOAA · 2023-05-19T19:22:43Z

DESCRIPTION OF CHANGES:

Add MRMS data to the get_da_obs task, and turn on the radar reflectivity processing task in the workflow for data assimilation pre-processing.

This is an addition of a new feature that should not impact any other features (outside recent RRFS-feature additions)

Type of change

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
This change requires a documentation update

TESTS CONDUCTED:

DEPENDENCIES:

none

DOCUMENTATION:

None.

ISSUE:

Issue #782

CHECKLIST

My code follows the style guidelines in the Contributor's Guide
I have performed a self-review of my own code using the Code Reviewer's Guide
I have commented my code, particularly in hard-to-understand areas
My changes need updates to the documentation. I have made corresponding changes to the documentation
My changes do not require updates to the documentation (explain).
My changes generate no new warnings
New and existing tests pass with my changes
Any dependent changes have been merged and published

LABELS (optional):

A Code Manager needs to add the following labels to this PR:

- "archive_internal_dir" must be an empty string for top-level archive files - Rename "file_type" argument to "file_fmt" - Rename "external_model" argument to more generic/correct "data_type" - If only some files can not be un-zipped, do not fail. We'll handle this more gracefully later if it needs to be an error - Remove default location logic for now; couldn't get it working - Re-add explicit disk path requirement; may re-visit later - For fatal error, do not imply that script continues - Remove debug prints

"rap_e" files in addition to "rap".

specifically HPSS fails with some kind of logging error. Needs more investigation. This commit removes some un-needed, un-initialized variables, adds modulefiles for the new task on each platform, and adds some unit tests for retrieve_data.py to test new capabilities. The new task entry has been added to parm/wflow/coldstart.yaml for testing, but it should be removed prior to merging this PR since the task is not integrated with the rest of the workflow yet.

… tasks going forward. Clearly the boundary conditions tasks will need to eventually change also.

- Link obs needed for task_process_bufr - Put obs in $DATA/obs rather than $DATA/gsi - Put in logic for getting "rap_e" files at 00 and 12z cycles; I hope we can extract this logic into the config file later with Jinja - Add "task_process_bufr" entry to warmstart.yaml with a dependency on task get_da_obs - Add "CYCLE_TYPE" variable to config_defaults - Start a new config.rrfs.yaml file

- Set machine-specific paths in machine files, this should have been obvious... - Use RRFS_NA_13km domain for RRFS - Re-organize Jet and Hera machine files for section consistency - Fix grammar error in retrieve_data.py

- Move date variables to J-job level - Keep da_obs in same directory for community and nco mode (since process_bufr does the same) - In process_bufr, look for da_obs in staging directory

- Add default directory to Jet machine file...only available there for now - Add task_process_lightning to warmstart workflow

…ve to Hera. Also fix config.rrfs.yaml to better settings for test case.

…to avoid duplicate error messages, search for RRFS data on disk as well as hpss

… on Hera!

Removing the CYCLE_TYPE from config_defaults because it is a run-time, cycle-dependent setting, not an experiment level one.

This is similar to what is done for external model files so that we can use the machine file information about which data stores a platform has access to.

Matching new standard outlined in Issue ufs-community#633

…obs_task Had to manually revert some renaming changes in ush/test_retrieve_data.py

…obs_task

MichaelLueken

@christinaholtNOAA Overall, these changes look good. I was able to run the process_obs test on both Hera and Jet and the test successfully ran on both machines.

I have noted that the paths on ush/machine/jet.yaml should use /mnt/lfs4, rather than /scratch2.

I also encountered failures running the fundamental tests on both Hera and Jet. The nco_grid_RRFS_CONUS_25km_ics_FV3GFS_lbcs_FV3GFS_timeoffset_suite_GFS_v16 test fails on both machines in the make_ics/lbcs step on both machines. Removing the expand_source_paths and going back to source_paths in ush/retrieve_data.py allows the test to run smoothly. It's not clear to me why the modifications you made to expand_source_paths are causing the necessary data not to be pulled.

Co-authored-by: Michael Lueken <63728921+MichaelLueken@users.noreply.github.com>

MichaelLueken

@christinaholtNOAA Thank you very much for finding the issue with the failure of the test so quickly! The fundamental tests were rerun and all successfully passed. As noted previously, the process_obs test was successfully run on Hera and Jet and now that the fundamental tests are passing, I will give my approval to this PR.

christinaholtNOAA · 2023-05-23T14:39:20Z

@MichaelLueken Thanks for bringing my attention to it and rerunning the tests! I opened a separate PR to more directly address the fix, but wanted to also test it with this PR to make sure it wasn't a problem here.

christinaholtNOAA · 2023-05-24T20:59:33Z

Is anyone else available to provide a 2nd review for this PR? It would be helpful to get it merged so that the cycled DA work could follow it in.

EdwardSnyder-NOAA

I left a few comments in the get_mrms_file.sh script.

When retrieving the da data from either NSSLMOSAIC or RAP_OBS_BUFR, these locations on Hera and Jet are the current live stream and not a historical location? Is that correct?

EdwardSnyder-NOAA · 2023-05-25T18:16:01Z

+  do
+    min=$( printf %2.2i $((timelevel+min)) )
+    echo "Looking for data valid:"${YYYY}"-"${MM}"-"${DD}" "${cyc}":"${min}
+    nsslfiles=${NSSL}/*${mrms}_00.50_${YYYY}${MM}${DD}-${cyc}${min}??.${OBS_SUFFIX}


Would the '00.50' need to be a wildcard? In line 50, it is a wildcard.

Given the original logic, I believe this is the level that needs to exist before all the others are gathered.

Ah okay. That makes sense but wasn't sure. Thanks for clarifying and making those changes. Approving.

EdwardSnyder-NOAA · 2023-05-25T18:17:15Z

+  for min in ${RADARREFL_MINS[@]}
+  do
+    min=$( printf %2.2i $((timelevel+min)) )
+    echo "Looking for data valid:"${YYYY}"-"${MM}"-"${DD}" "${cyc}":"${min}


Maybe there is a reason behind all these double quotes, but you could just have one set of double quotes around this string.

EdwardSnyder-NOAA · 2023-05-25T18:18:38Z

+        # 10 represents a significant number of vertical levels of data
+        if [ ${numgrib2} -ge 10 ] && [ ! -e filelist_mrms ]; then
+          cp_vrfy ${NSSL}/${nsslfile1} ${output_path}
+          ls ${nsslfile1} > ${output_path}/filelist_mrms


I believe you need to add ${NSSL}/ in front of ${nsslfile1} in order for this command to work.

EdwardSnyder-NOAA · 2023-05-25T18:20:36Z

+        if [ ${numgrib2} -ge 10 ] && [ ! -e filelist_mrms ]; then
+          cp_vrfy ${NSSL}/${nsslfile1} ${output_path}
+          ls ${nsslfile1} > ${output_path}/filelist_mrms
+          echo 'Copying mrms files for ${YYYY}${MM}${DD}-${cyc}${min}'


This prints out: Copying mrms files for ${YYYY}${MM}${DD}-${cyc}${min}. Replacing the single quotes with double quotes will allow the variables to be substituted.

EdwardSnyder-NOAA · 2023-05-25T18:21:17Z

+      if [ -s $nsslfile ]; then
+        echo 'Found '${nsslfile}
+        nsslfile1=*${mrms}_*_${YYYY}${MM}${DD}-${cyc}${min}*.${OBS_SUFFIX}
+        numgrib2=${#nsslfiles1}


I believe this should be nsslfile1

MichaelLueken · 2023-05-26T13:23:12Z

@christinaholtNOAA Since potential changes to the ush/get_mrms_files.sh script won't affect current WE2E tests, I will go ahead and submit the automated Jenkins tests at this time. However, this PR won't be merged until the comments made by @EdwardSnyder-NOAA have been resolved.

MichaelLueken · 2023-05-30T15:33:36Z

@christinaholtNOAA The Jenkins functional tests failed on Hera and due to issues on Orion, the coverage WE2E tests failed there as well. The coverage WE2E tests were run on Hera and Orion. The tests successfully passed. Once you and @EdwardSnyder-NOAA are happy with the changes in ush/get_mrms_files.sh and the conflict in ush/retrieve_data.py has been addressed (likely due to PR #810 having been merged before this work), then I will move forward and merge this update.

christinaholtNOAA · 2023-05-30T15:48:37Z

Thanks @MichaelLueken. I just finished the fundamental tests after the conflict resolution and addressing @EdwardSnyder-NOAA's comments.

MichaelLueken · 2023-05-30T15:57:34Z

@EdwardSnyder-NOAA Please look over @christinaholtNOAA's latest modifications to ush/get_mrms_files.sh. If you are happy with this update, I will go ahead and merge this PR. Thanks!

MichaelLueken · 2023-05-30T19:08:47Z

@EdwardSnyder-NOAA Thank you very much for looking back over the changes! @christinaholtNOAA I will now proceed with merging this work.

mkavulich and others added 30 commits April 19, 2023 20:27

First draft, doesnt work

7cbc445

Emergency commit, still not working

340f9ab

First draft of get obs exscript. Modified data_locations to look for

61af001

"rap_e" files in addition to "rap".

It works! Just needed to specify HPSS partition

5429625

Start new workflow yaml "warmstart.yaml" that will be used for GSI/DA…

d0472d8

… tasks going forward. Clearly the boundary conditions tasks will need to eventually change also.

Prevent user from entering empty --tests argument to run_WE2E_tests.py

7471a0a

Fix incorrect path after rebase, add alternate path for older RAP data

9d1b37b

Some better design choices after putting some more thought into it

158b7e1

- Set machine-specific paths in machine files, this should have been obvious... - Use RRFS_NA_13km domain for RRFS - Re-organize Jet and Hera machine files for section consistency - Fix grammar error in retrieve_data.py

Data now successfully staged for process_bufr task!

24aac12

- Move date variables to J-job level - Keep da_obs in same directory for community and nco mode (since process_bufr does the same) - In process_bufr, look for da_obs in staging directory

Add NLDN lightning data to get_da_obs

a99fd5b

- Add default directory to Jet machine file...only available there for now - Add task_process_lightning to warmstart workflow

Adding NLDN lightning to get_da_obs. Not fully tested yet, need to mo…

6679d59

…ve to Hera. Also fix config.rrfs.yaml to better settings for test case.

Use "else" statement in final try block for generate_FV3LAM_wflow.py …

64dbc63

…to avoid duplicate error messages, search for RRFS data on disk as well as hpss

Hot diggity, got the process_bufr and process_lightning tasks working…

1f1c497

… on Hera!

Untested first pass at defining the early workflow

252eade

Partial incorporation of Christina's changes from ufs-community#725

a612a0f

CYCLE_TYPE is a run-time arg.

870722e

Removing the CYCLE_TYPE from config_defaults because it is a run-time, cycle-dependent setting, not an experiment level one.

Make paths and data stores configurable.

769c1af

This is similar to what is done for external model files so that we can use the machine file information about which data stores a platform has access to.

Remove duplicative information from machine files.

de99914

Renaming process_bufr to process_bufrobs.

4f56a54

Matching new standard outlined in Issue ufs-community#633

Working prepbufr getting and processing workflow.

412fa6d

This version of rrfs_beta might work.

5530100

Turn it off since we can't use it.

c9fe408

Merge remote-tracking branch 'upstream/develop' into feature/add_get_…

ec0eae1

…obs_task Had to manually revert some renaming changes in ush/test_retrieve_data.py

Updates to fix failing unit tests

f2aca37

WIP. Waiting on HPSS maintenance.

7e3171e

Retrieve data task works here.

7ae52b5

Merge remote-tracking branch 'upstream/develop' into feature/add_get_…

c88704a

…obs_task

christinaholtNOAA requested review from EdwardSnyder-NOAA, MichaelLueken, dmwright526, gspetro-NOAA and natalie-perlin as code owners May 19, 2023 19:22

Fix a bad merge on this renamed file.

d668a0b

MichaelLueken linked an issue May 22, 2023 that may be closed by this pull request

Get and process radar data in separate workflow tasks. #782

Closed

MichaelLueken requested changes May 22, 2023

View reviewed changes

Comment thread ush/machine/jet.yaml Outdated

Comment thread ush/machine/jet.yaml Outdated

Comment thread ush/retrieve_data.py

christinaholtNOAA and others added 4 commits May 22, 2023 14:16

Update ush/machine/jet.yaml

aaa987f

Co-authored-by: Michael Lueken <63728921+MichaelLueken@users.noreply.github.com>

Update ush/machine/jet.yaml

e773db0

Co-authored-by: Michael Lueken <63728921+MichaelLueken@users.noreply.github.com>

Attempt to fix failure with nco input data.

1689e43

Merge remote-tracking branch 'crh/add_radar_obs' into add_radar_obs

4d7ece9

MichaelLueken approved these changes May 23, 2023

View reviewed changes

JeffBeck-NOAA approved these changes May 25, 2023

View reviewed changes

EdwardSnyder-NOAA reviewed May 25, 2023

View reviewed changes

MichaelLueken added the run_we2e_coverage_tests Run the coverage set of SRW end-to-end tests label May 26, 2023

MichaelLueken mentioned this pull request May 26, 2023

[develop] Updating RAP AWS test date. #814

Closed

23 tasks

christinaholtNOAA added 2 commits May 30, 2023 15:00

Addressing Eddie's comments.

c36dccc

Merge remote-tracking branch 'origin/develop' into add_radar_obs

48856d6

christinaholtNOAA self-assigned this May 30, 2023

EdwardSnyder-NOAA approved these changes May 30, 2023

View reviewed changes

MichaelLueken merged commit 696f080 into ufs-community:develop May 30, 2023

christinaholtNOAA deleted the add_radar_obs branch July 2, 2024 16:48

Conversation

christinaholtNOAA commented May 19, 2023

DESCRIPTION OF CHANGES:

Type of change

TESTS CONDUCTED:

DEPENDENCIES:

DOCUMENTATION:

ISSUE:

CHECKLIST

LABELS (optional):

Uh oh!

MichaelLueken left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

MichaelLueken left a comment

Choose a reason for hiding this comment

Uh oh!

christinaholtNOAA commented May 23, 2023

Uh oh!

christinaholtNOAA commented May 24, 2023

Uh oh!

EdwardSnyder-NOAA left a comment

Choose a reason for hiding this comment

Uh oh!

EdwardSnyder-NOAA May 25, 2023

Choose a reason for hiding this comment

Uh oh!

christinaholtNOAA May 30, 2023

Choose a reason for hiding this comment

Uh oh!

EdwardSnyder-NOAA May 30, 2023

Choose a reason for hiding this comment

Uh oh!

EdwardSnyder-NOAA May 25, 2023

Choose a reason for hiding this comment

Uh oh!

EdwardSnyder-NOAA May 25, 2023

Choose a reason for hiding this comment

Uh oh!

EdwardSnyder-NOAA May 25, 2023

Choose a reason for hiding this comment

Uh oh!

EdwardSnyder-NOAA May 25, 2023

Choose a reason for hiding this comment

Uh oh!

MichaelLueken commented May 26, 2023

Uh oh!

MichaelLueken commented May 30, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

christinaholtNOAA commented May 30, 2023

Uh oh!

MichaelLueken commented May 30, 2023

Uh oh!

MichaelLueken commented May 30, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

MichaelLueken commented May 30, 2023 •

edited

Loading