-
Notifications
You must be signed in to change notification settings - Fork 122
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 RAP/GFS bufr and GSI-fix file retrieval options #535
[develop] Add RAP/GFS bufr and GSI-fix file retrieval options #535
Conversation
…s to jet machine file. Add in Bruce's retrieve_data.py updates to accomodate local file staging. Add JJOB and exregional jobs for observation data staging (preliminary).
…ufs-srweather-app into feature/add_rrfs_obs
…gle-line method is working.
…isk, machine specific) file paths should be located in the respective machine file rather than data_locations.yaml, so this is a start.
…a_locations.yaml a bit.
…ufs-srweather-app into feature/add_rrfs_obs
@christinaholtNOAA i went through and addressed your recent comments. thanks so much for looking again. i apologize for the discordance between my remote and local (i can't figure out why it is acting this way). |
@ulmononian If @MichaelLueken hasn't reached out to you yet he will shortly. We had to rewrite history to remove a commit from the repository last week, so that is likely why you are seeing unexpected behavior with your fork. It should be a simple fix but we will need to fix your fork to remove this commit as well. |
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.
@ulmononian This looks good to me! Thanks for the iterations. :)
Edit: Make sure it passes the tests though.
@christinaholtNOAA thanks for the final look!! may i ask which tests you are referring to (a set of WE2E's or something else)? |
@ulmononian It looks like both of the unit tests have failed for this PR. If you merge the latest develop into your feature/add_rrfs_obs branch, the Looking at the
It looks like you will need to make modifications to Both of these tests will need to pass before this work can be merged into develop. I think this is what @christinaholtNOAA meant by making sure that the changes in this PR pass the tests. |
Change "--anl_or_fcst" to "--file_set" to match update in retrieve_data.py
thanks so much for clarifying (and identifying a root cause of the unit test failures) @MichaelLueken :) i merged in develop and updated |
Replace anl_or_fcst arg. with file_set (in all instances!)
last time
Machine: hera |
for the test "grid_RRFS_CONUS_25km_ics_FV3GFS_lbcs_FV3GFS_suite_GFS_v16" , i see this call in the log "/scratch1/BMC/zrtrr/rrfs_ci/autoci/pr/1195772843/20230210155017/expt_dirs/../nco_dirs/output/20190701/get_extrn_ics_2019070100.id_1676045187.log": i am not sure why @mkavulich i i grepped "anl_or_fcst" at the top of ufs-srweather-app: looks like i need to update the arg. name in |
@MichaelLueken @venitahagerty necessary updates made to |
Machine: hera |
@MichaelLueken @venitahagerty looks like an unbound variable issue with |
Machine: hera |
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.
@ulmononian These changes look good to me! Thanks for iterating through the GHA Hera WE2E test errors! The non-Hera Jenkins tests have all completed successfully, as have the Hera GHA tests. I will now approve these changes and move forward with merging this work.
DESCRIPTION OF CHANGES:
This PR is part of a series that will facilitate integration of RRFS features into srw-dev (e.g.: #526 by @EdwardSnyder-NOAA ).
RRFS requires certain observation files and data products to be retrieved & staged before the data processing and assimilation tasks are run. This work introduces some small changes to the retrieve_data.py script to allow retrieval/staging of local (machine-hosted; e.g.: Jet, Hera, noaacloud) and general, remotely located (extending beyond nomads/aws) files, primarily in the relaxation of/introduction of new parse_args function arguments.
Minor updates to the data_locations.yaml are included here (more will follow), including the addition of RAP bufr files and GSI-fix files from NOAA AWS S3 buckets. FV3GFS & RAP bufr data stream locations on Jet and Hera have been added to the respective ush/machine YAMLS.
Type of change
TESTS CONDUCTED:
Currently, the functionality has only been tested through direct call of the retrieve_data.py script on Jet and Hera (i.e.: "python retrieve_data.py --external_model RAP --data_stores local --cycle 2023011100 --config $path_to_srw_top_level/ush/machine/jet.yaml --anl_or_fcst obs --output_path $arbitrary_staging_area --symlink). However, once the RRFS observation retrieval/staging/processing JJOBS and exregional scripts are developed, this retrieval functionality will be added into the workflow.
Compiled/built srw-dev on Jet/Hera with changes from this fork w/out issue.
DEPENDENCIES:
None
ISSUE:
#484
CHECKLIST
LABELS (optional):
A Code Manager needs to add the following labels to this PR:
CONTRIBUTORS (optional):
Bruce Kropp (@BruceKropp-Raytheon)