Skip to content
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.

Sign up for GitHub

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

Fix time offset issue on ICS with GFS nemsio and netcdf files and add new archive file name on HPSS #457

Merged
merged 12 commits into from
Nov 9, 2022
Merged

Fix time offset issue on ICS with GFS nemsio and netcdf files and add new archive file name on HPSS #457

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
a7fef59
fix time offset with gfs
chan-hoo Nov 4, 2022
9b9658a
add new archive file name and we2e test
chan-hoo Nov 5, 2022
7a80c20
Merge branch 'develop' of https://github.com/ufs-community/ufs-srweat…
chan-hoo Nov 5, 2022
7d6d252
add ics_or_lbcs to test_retrieve.py
chan-hoo Nov 5, 2022
313a3bb
Add check availability of nemsio and netcdf
chan-hoo Nov 5, 2022
37ed64f
Copy file_templates dict before modifying it.
danielabdi-noaa Nov 6, 2022
f5e0918
remove native from xml for dev_transfer on wcoss2
chan-hoo Nov 7, 2022
5c8844e
Merge branch 'bugfix/timeoffset_gfs' of https://github.com/chan-hoo/u…
chan-hoo Nov 7, 2022
1d38fca
correct data locations on wcoss2
chan-hoo Nov 7, 2022
a021690
remove met_verification from fundamental.cheyenne.gnu
chan-hoo Nov 7, 2022
593cde9
replace cyc to hh in data locations
chan-hoo Nov 8, 2022
4a3e92d
make lbcs condition general
chan-hoo Nov 9, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions parm/data_locations.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,17 +64,20 @@ FV3GFS:
- gfs.t{hh}z.sfcanl.nemsio
fcst:
- gfs.t{hh}z.atmf{fcst_hr:03d}.nemsio
- gfs.t{hh}z.sfcf{fcst_hr:03d}.nemsio
netcdf:
anl:
- gfs.t{hh}z.atmanl.nc
- gfs.t{hh}z.sfcanl.nc
fcst:
- gfs.t{hh}z.atmf{fcst_hr:03d}.nc
- gfs.t{hh}z.sfcf{fcst_hr:03d}.nc
hpss:
protocol: htar
archive_path:
- /NCEPPROD/hpssprod/runhistory/rh{yyyy}/{yyyymm}/{yyyymmdd}
- /NCEPPROD/hpssprod/runhistory/rh{yyyy}/{yyyymm}/{yyyymmdd}
- /NCEPPROD/hpssprod/runhistory/rh{yyyy}/{yyyymm}/{yyyymmdd}
archive_internal_dir:
- ./gfs.{yyyymmdd}/{hh}
- ./gfs.{yyyymmdd}/{hh}/atmos
Expand All @@ -83,9 +86,11 @@ FV3GFS:
anl:
- gpfs_dell1_nco_ops_com_gfs_prod_gfs.{yyyymmdd}_{hh}.gfs_pgrb2.tar
- com_gfs_prod_gfs.{yyyymmdd}_{hh}.gfs_pgrb2.tar
- com_gfs_v16.2_gfs.{yyyymmdd}_{hh}.gfs_pgrb2.tar
fcst:
- gpfs_dell1_nco_ops_com_gfs_prod_gfs.{yyyymmdd}_{hh}.gfs_pgrb2.tar
- com_gfs_prod_gfs.{yyyymmdd}_{hh}.gfs_pgrb2.tar
- com_gfs_v16.2_gfs.{yyyymmdd}_{hh}.gfs_pgrb2.tar
nemsio:
anl:
- gpfs_dell1_nco_ops_com_gfs_prod_gfs.{yyyymmdd}_{hh}.gfs_nemsioa.tar
Expand All @@ -97,9 +102,11 @@ FV3GFS:
anl:
- gpfs_dell1_nco_ops_com_gfs_prod_gfs.{yyyymmdd}_{hh}.gfs_nca.tar
- com_gfs_prod_gfs.{yyyymmdd}_{hh}.gfs_nca.tar
- com_gfs_v16.2_gfs.{yyyymmdd}_{hh}.gfs_nca.tar
fcst:
- ['gpfs_dell1_nco_ops_com_gfs_prod_gfs.{yyyymmdd}_{hh}.gfs_nca.tar', 'gpfs_dell1_nco_ops_com_gfs_prod_gfs.{yyyymmdd}_{hh}.gfs_ncb.tar']
- ['com_gfs_prod_gfs.{yyyymmdd}_{hh}.gfs_nca.tar', 'com_gfs_prod_gfs.{yyyymmdd}_{hh}.gfs_ncb.tar']
- ['com_gfs_v16.2_gfs.{yyyymmdd}_{hh}.gfs_nca.tar', 'com_gfs_v16.2_gfs.{yyyymmdd}_{hh}.gfs_ncb.tar']
file_names:
<<: *gfs_file_names
aws:
Expand Down
1 change: 1 addition & 0 deletions scripts/exregional_get_extrn_mdl_files.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,7 @@ python3 -u ${USHdir}/retrieve_data.py \
--data_stores ${data_stores} \
--external_model ${EXTRN_MDL_NAME} \
--fcst_hrs ${fcst_hrs[@]} \
--ics_or_lbcs ${ICS_OR_LBCS} \
--output_path ${EXTRN_MDL_STAGING_DIR} \
--summary_file ${EXTRN_DEFNS} \
$additional_flags"
Expand Down
28 changes: 28 additions & 0 deletions ..._nco/config.nco_grid_RRFS_CONUS_25km_ics_FV3GFS_lbcs_FV3GFS_timeoffset_suite_GFS_v16.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
metadata:
description: |-
This test is to ensure that the workflow running in nco mode completes
successfully on the RRFS_CONUS_25km grid using the FV3_GFS_v16 physics
suite with time-offset ICs/LBCs derived from the FV3GFS.
user:
RUN_ENVIR: nco
workflow:
CCPP_PHYS_SUITE: FV3_GFS_v16
DATE_FIRST_CYCL: '2022081012'
DATE_LAST_CYCL: '2022081012'
FCST_LEN_HRS: 6
PREEXISTING_DIR_METHOD: rename
workflow_switches:
RUN_TASK_MAKE_GRID: false
RUN_TASK_MAKE_OROG: false
RUN_TASK_MAKE_SFC_CLIMO: false
task_get_extrn_ics:
EXTRN_MDL_NAME_ICS: FV3GFS
FV3GFS_FILE_FMT_ICS: netcdf
EXTRN_MDL_ICS_OFFSET_HRS: 6
task_get_extrn_lbcs:
EXTRN_MDL_NAME_LBCS: FV3GFS
FV3GFS_FILE_FMT_LBCS: netcdf
LBC_SPEC_INTVL_HRS: 3
EXTRN_MDL_LBCS_OFFSET_HRS: 6
task_run_fcst:
PREDEF_GRID_NAME: RRFS_CONUS_25km
13 changes: 13 additions & 0 deletions ush/retrieve_data.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,13 @@ def get_file_templates(cla, known_data_info, data_store, use_cla_tmpl=False):
"""

file_templates = known_data_info.get(data_store, {}).get("file_names")

# Remove sfc files from fcst in file_names of FV3GFS for LBCs
# sfc files needed in fcst when time_offset is not zero.
if cla.external_model == "FV3GFS" and cla.ics_or_lbcs == "LBCS":
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this just be if "lbcs" and "fcst"? That way we don't tightly couple the tool to specifics of the models we use as inputs?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@christinaholtNOAA, I am afraid that it will cause another unexpected errors with other models. For example, 'GDAS', 'GSMGFS', 'RAP', 'HRRR', and 'NAM' have only one file name for 'fcst'. In these cases, we will have the same index error. For 'GEFS', we should not remove the second array from the file names. We just want to remove the 'sfc' file from the list. 'FV3GFS' only has this issue. What do you think of this?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This logic is still pretty brittle. It assumes that there must be at least 2 and that the 2nd one is the surface file. I think that if we want to protect that assumption, we need a functional test.

I also think it's probably safer to do something like delete the entry if "sfc" is in the name of the file. That type of logic may get us the additional functionality we'd need to use offset GDAS netcdf/nemsio as ICS and expand the logic to "lbcs" and "fcst" instead of FV3GFS-specific logic. In general that says "if we're looking for lbcs from forecasts and a surface file is listed, don't try to get it".

As best I can tell, sfc files only show up for GDAS, FV3GFS, and GSMGFS.

I'd prefer to see something like this:

if cla.ics_or_lbcs == "LBCS":
    for format in ['netcdf', 'nemsio']:
        for i, tmpl in enumerate(file_templates.get('format', {}).get('fcst', [])):
            if "sfc" in tmpl:
                 del file_templates[format]['fcst'][i]

It reduces the assumptions about order and the length of the list provided. It assures that we're only removing surface files, and not atm files accidentally. It also helps us if we want to expand our experiments run with offset GDAS files as ICs -- we can just add the sfc file to the GDAS fcst entry.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@christinaholtNOAA, I tested your script, but sfc file was not removed from the list for LBCs: DEBUG: Looking for files like ['gfs.t{hh}z.atmf{fcst_hr:03d}.nemsio', 'gfs.t{hh}z.sfcf{fcst_hr:03d}.nemsio']. Any suggestion?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found a typo there: 'format' => format. I am testing it again.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apologies...I was just writing directly in the text box here and didn't do a full test.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@christinaholtNOAA , the tests were completed successfully!! The part has been replaced with your script. Thank you!

del file_templates['nemsio']['fcst'][1]
del file_templates['netcdf']['fcst'][1]

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you may need to add a similar entry to the "nemsio" format for this to work. Changing the format to nemsio for the new test case you added I get this error:

Traceback (most recent call last):
  File "/scratch2/BMC/gsd-hpcs/Daniel.Abdi/ufs-srweather-app/ush/retrieve_data.py", line 1012, in <module>
    main(sys.argv[1:])
  File "/scratch2/BMC/gsd-hpcs/Daniel.Abdi/ufs-srweather-app/ush/retrieve_data.py", line 814, in main 
    file_templates = get_file_templates(
  File "/scratch2/BMC/gsd-hpcs/Daniel.Abdi/ufs-srweather-app/ush/retrieve_data.py", line 296, in get_file_templates
    del file_templates['nemsio']['fcst'][1]
IndexError: list assignment index out of range

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is happening because the gfs_file_names anchor is shared for all sources, so when it tries hpss and doesn't succeed it deletes the sfc entry. Then when it tries aws, the entry is already gone. I think you can solve this by not using anchors in the yaml file (bad) or by making a deep copy of the dictionary before deleting the entries.

Copy link
Collaborator Author

@chan-hoo chan-hoo Nov 5, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@danielabdi-noaa, nemsio is not available for the date (08/2022). netcdf and grib2 are only available. Do we need to add any other conditions for this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@danielabdi-noaa, I've added if-statements to check the availability of netcdf and nemsio for the cycle date in jobs/JREGIONAL_GET_EXTRN_MDL_FILES. If nemsio (or netcdf) is not available, the get_extrn_ics/lbcs will fail with an error message.

Copy link
Collaborator

@danielabdi-noaa danielabdi-noaa Nov 6, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chan-hoo I still think we need a safeguard against multiple deletions of the sfc file -- best would be to find another way that does not delete entries in the data_locations dictionary. If hpss does not have the file for some reason (maybe it is down), and we want to try aws next, it will fail because the sfc entry has been deleted by HPSS. The invalid nemsio date run i did (although wrong) shows what could happen if we can't find the file on hpss but could be available on aws. I added this before the del if statement to make it try aws successfully when it could not find it in hpss

file_templates = deepcopy(file_templates)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@danielabdi-noaa, yes, I agree with you. Can you help me? I've sent an invitation to you.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chan-hoo No big deal, i've pushed the change i think will avoid multiple deletions of the sfc file. It is not an ideal solution so lets wait for @christinaholtNOAA .

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

if use_cla_tmpl:
file_templates = cla.file_templates if cla.file_templates else file_templates

Expand Down Expand Up @@ -946,6 +953,12 @@ def parse_args(argv):
required=True,
type=os.path.abspath,
)
parser.add_argument(
"--ics_or_lbcs",
choices=("ICS", "LBCS"),
help="Flag for whether ICS or LBCS.",
required=True,
)

# Optional
parser.add_argument(
Expand Down
10 changes: 10 additions & 0 deletions ush/test_retrieve_data.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ def test_fv3gfs_grib2_lbcs_from_hpss(self):
'--external_model', 'FV3GFS',
'--fcst_hrs', '6', '12', '3',
'--output_path', tmp_dir,
'--ics_or_lbcs', 'LBCS',
'--debug',
'--file_type', 'grib2',
]
Expand Down Expand Up @@ -80,6 +81,7 @@ def test_fv3gfs_netcdf_lbcs_from_hpss(self):
'--external_model', 'FV3GFS',
'--fcst_hrs', '24', '48', '24',
'--output_path', tmp_dir,
'--ics_or_lbcs', 'LBCS',
'--debug',
'--file_type', 'netcdf',
]
Expand Down Expand Up @@ -112,6 +114,7 @@ def test_gdas_ics_from_aws(self):
'--external_model', 'GDAS',
'--fcst_hrs', '6', '9', '3',
'--output_path', out_path_tmpl,
'--ics_or_lbcs', 'LBCS',
'--debug',
'--file_type', 'netcdf',
'--members', '9', '10',
Expand Down Expand Up @@ -147,6 +150,7 @@ def test_gefs_grib2_ics_from_aws(self):
'--external_model', 'GEFS',
'--fcst_hrs', '6',
'--output_path', out_path_tmpl,
'--ics_or_lbcs', 'ICS',
'--debug',
'--file_type', 'netcdf',
'--members', '1', '2',
Expand Down Expand Up @@ -180,6 +184,7 @@ def test_hrrr_ics_from_hpss(self):
'--external_model', 'HRRR',
'--fcst_hrs', '0',
'--output_path', tmp_dir,
'--ics_or_lbcs', 'ICS',
'--debug',
]
# fmt: on
Expand Down Expand Up @@ -209,6 +214,7 @@ def test_hrrr_lbcs_from_hpss(self):
'--external_model', 'HRRR',
'--fcst_hrs', '3', '24', '3',
'--output_path', tmp_dir,
'--ics_or_lbcs', 'LBCS',
'--debug',
]
# fmt: on
Expand Down Expand Up @@ -237,6 +243,7 @@ def test_hrrr_ics_from_aws(self):
'--external_model', 'HRRR',
'--fcst_hrs', '0',
'--output_path', tmp_dir,
'--ics_or_lbcs', 'ICS',
'--debug',
]
# fmt: on
Expand Down Expand Up @@ -265,6 +272,7 @@ def test_hrrr_lbcs_from_aws(self):
'--external_model', 'HRRR',
'--fcst_hrs', '3', '24', '3',
'--output_path', tmp_dir,
'--ics_or_lbcs', 'LBCS',
'--debug',
]
# fmt: on
Expand Down Expand Up @@ -294,6 +302,7 @@ def test_rap_ics_from_aws(self):
'--external_model', 'RAP',
'--fcst_hrs', '3',
'--output_path', tmp_dir,
'--ics_or_lbcs', 'ICS',
'--debug',
]
# fmt: on
Expand Down Expand Up @@ -323,6 +332,7 @@ def test_rap_lbcs_from_aws(self):
'--external_model', 'RAP',
'--fcst_hrs', '3', '30', '6',
'--output_path', tmp_dir,
'--ics_or_lbcs', 'LBCS',
'--debug',
]
# fmt: on
Expand Down