-
Notifications
You must be signed in to change notification settings - Fork 121
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
Fix time offset issue on ICS with GFS nemsio and netcdf files and add new archive file name on HPSS #457
Fix time offset issue on ICS with GFS nemsio and netcdf files and add new archive file name on HPSS #457
Changes from 10 commits
a7fef59
9b9658a
7a80c20
7d6d252
313a3bb
37ed64f
f5e0918
5c8844e
1d38fca
a021690
593cde9
4a3e92d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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 |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -34,8 +34,8 @@ task_run_fcst: | |
FIXlut: /lfs/h2/emc/lam/noscrub/UFS_SRW_App/develop/fix/fix_lut | ||
data: | ||
GSMGFS: compath.py ${envir}/gsmgfs/${gsmgfs_ver}/gsmgfs.${PDY} | ||
FV3GFS: compath.py ${envir}/gfs/${gfs_ver}/gfs.${PDY} | ||
FV3GFS: compath.py ${envir}/gfs/${gfs_ver}/gfs.${PDY}/${cyc}/atmos | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why not use ${hh} here instead of ${cyc}? This is a bit misleading because this ${cyc} is not necessarily aligned with the Then, there's no need to rename cyc in the ex-script. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes, I agree. it has been replaced with 'hh'. |
||
RAP: compath.py ${envir}/rap/${rap_ver}/rap.${PDY} | ||
NAM: compath.py ${envir}/nam/${nam_ver}/nam.${PDY} | ||
HRRR: compath.py ${envir}/hrrr/${hrrr_ver}/hrrr.${PDY} | ||
HRRR: compath.py ${envir}/hrrr/${hrrr_ver}/hrrr.${PDY}/conus | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -34,6 +34,7 @@ | |
import sys | ||
from textwrap import dedent | ||
import time | ||
from copy import deepcopy | ||
|
||
import yaml | ||
|
||
|
@@ -289,6 +290,14 @@ 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") | ||
file_templates = deepcopy(file_templates) | ||
|
||
# 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": | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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:
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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I found a typo there: There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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] | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this is happening because the There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @danielabdi-noaa, I've added if-statements to check the availability of There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
|
||
|
@@ -946,6 +955,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( | ||
|
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.
This is potentially a problem. It resets an NCO required variable. This is the GFS cyc that we're looking for, which is a namespace collision with the existing
cyc
for RRFS.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.
removed.