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.

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

SFS Runs at C96mx100 #2960

Open
wants to merge 81 commits into
base: develop
Choose a base branch
from
Open

Conversation

NeilBarton-NOAA
Copy link
Contributor

Description

Adds the ability to run C96mx100 with SFS options. Included in this PR is the ability to use the interpolated/coarse grained 1 degree MOM6 ICs. Staging the ATM and OCN perturbation files without using the +03 replay ICs.

Type of change

  • Bug fix (fixes something broken)
  • New feature (adds functionality)
  • Maintenance (code refactor, clean-up, new CI test, etc.)

Change characteristics

  • Is this a breaking change (a change in existing functionality)? YES
  • Does this change require a documentation update? NO
  • Does this change require an update to any of the following submodules? NO
    • EMC verif-global
    • GDAS
    • GFS-utils
    • GSI
    • GSI-monitor
    • GSI-utils
    • UFS-utils
    • UFS-weather-model
    • wxflow

How has this been tested?

This has been tested in the gefs forecast-only configurations on hera and hercules.

Checklist

  • Any dependent changes have been merged and published
  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have documented my code, including function, input, and output descriptions
  • My changes generate no new warnings
  • New and existing tests pass with my changes
  • This change is covered by an existing CI test or a new one has been added
  • I have made corresponding changes to the system documentation if necessary

NeilBarton-NOAA and others added 30 commits July 2, 2024 15:04
Fix a bug in a variable check in the extractvars task where parenthesis in the variable name will prompt a false warning message.
Ice variables are extracted from the history files instead of the product files. This is necessary because there are certain ice variables that are needed that are only available in the history files and not the product files.
Add suggested patches to gefs_replay_ci
Shellcheck disable has been added. Also, f000 is skipped in extractvars for ocn and ice since there are no products for f000 for ocn and ice. This change will remove warning messages.
@NeilBarton-NOAA NeilBarton-NOAA marked this pull request as ready for review October 15, 2024 18:15
export nthreads_fv3=1
export nthreads_fv3_gfs=1
export nthreads_ufs=1
export nthreads_ufs_gfs=1
export xr_cnvcld=".false." # Do not pass conv. clouds to Xu-Randall cloud fraction
export xr_cnvcld=".true." # Pass conv. clouds to Xu-Randall cloud fraction

Choose a reason for hiding this comment

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

@NeilBarton-NOAA we could perhaps just remove this xr_cnvcld variable from the workflow to avoid any confusion. It is default .true. in the code, and when we wanted it to be false for C96 only it made sense to keep it here. But now when we moved it back to true, and instead changed the scaling in the code - it is probably cleaner to just remove.

@aerorahul
Copy link
Contributor

@pjpegion has proposed a system=sfs in addition to gfs|gefs variants.
@WalterKolczynski-NOAA we should ensure that it is brought in sooner rather than later, before conditionals for SFS get if-ed into the RUN=gefs code-blocks.
@JacobCarley-NOAA FYI

@@ -0,0 +1,20 @@
experiment:
system: gefs
Copy link
Contributor

Choose a reason for hiding this comment

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

This is now a SFS test using gefs system.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The SFS_POST=YES is currently a proxy to system=sfs and will need to be edited.

@@ -37,6 +37,8 @@ Usage: ${BASH_SOURCE[0]} [-a UFS_app][-c build_config][-d][-f][-h][-j n][-v][-w]
Execute all build scripts with -v option to turn on verbose where supported
-w:
Use structured wave grid
-y:
Use hystrostatic version of FV3
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Use hystrostatic version of FV3
Use hydrostatic version of FV3

Copy link
Contributor

@aerorahul aerorahul left a comment

Choose a reason for hiding this comment

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

How is this actually tested? It appears that the model has to be built with a hydrostatic option -y but that is missing from build_ufs.sh.
Also, this requires the CI system to build the ufs-weather-model in 3 configurations; unstructured waves for GFS, structured waves for GEFS (-w) and hydrostatic dynamical code for SFS (-y).
I think we need to re-consider the CI system where we are cloning 2 copies of global-workflow one each for GFS and GEFS, and now this third one for SFS.
Instead, we could refactor the current CI system to build 3 executables with a suffix of .gfs.x, .gefs.x, .sfs.x or gfs_model.x, gefs_model.x, sfs_model.x for the variants of the model options and still use the same HOMEgfs.

This will require some prep-work in the global-workflow.

@@ -129,7 +133,7 @@ declare -A build_opts
big_jobs=0
build_jobs["ufs"]=8
big_jobs=$((big_jobs+1))
build_opts["ufs"]="${_wave_opt} ${_verbose_opt} ${_build_ufs_opt} ${_build_debug}"
build_opts["ufs"]="${_wave_opt} ${_hydro_opt} ${_verbose_opt} ${_build_ufs_opt} ${_build_debug}"
Copy link
Contributor

Choose a reason for hiding this comment

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

This requires build_ufs.sh to be updated as well to make use of the -y.

Copy link
Contributor Author

@NeilBarton-NOAA NeilBarton-NOAA Oct 16, 2024

Choose a reason for hiding this comment

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

@aerorahul I didn't catch an error with the merge/commit. The updated build_all.sh and other files are now uploaded

@aerorahul
Copy link
Contributor

@WalterKolczynski-NOAA @DavidHuber-NOAA Please see this comment and provide your thoughts.

@WalterKolczynski-NOAA
Copy link
Contributor

How is this actually tested? It appears that the model has to be built with a hydrostatic option -y but that is missing from build_ufs.sh. Also, this requires the CI system to build the ufs-weather-model in 3 configurations; unstructured waves for GFS, structured waves for GEFS (-w) and hydrostatic dynamical code for SFS (-y). I think we need to re-consider the CI system where we are cloning 2 copies of global-workflow one each for GFS and GEFS, and now this third one for SFS. Instead, we could refactor the current CI system to build 3 executables with a suffix of .gfs.x, .gefs.x, .sfs.x or gfs_model.x, gefs_model.x, sfs_model.x for the variants of the model options and still use the same HOMEgfs.

This will require some prep-work in the global-workflow.

I agree. We've already talked about doing this for the two existing variations, though it hasn't been a priority as yet. I'd like to name the executables by options rather than system, since there is nothing inherent preventing GEFS from running with an unstructured wave grid, for example. But that also may get cumbersome if we accumulate too many different build options.

I can roll this into my existing compute-build effort (indeed was already considering doing so).

@NeilBarton-NOAA
Copy link
Contributor Author

The GEFS and GFS coupled forecasts CIs are successful with the latest commits. The pynorm error is unclear to me. @WalterKolczynski-NOAA any suggestions on the pynorm error?

@WalterKolczynski-NOAA
Copy link
Contributor

The GEFS and GFS coupled forecasts CIs are successful with the latest commits. The pynorm error is unclear to me. @WalterKolczynski-NOAA any suggestions on the pynorm error?

If you click on 'details' it will take you to the pynorms log. In this case, it is just one thing:

checking ./scripts/exglobal_stage_ic.py
./scripts/exglobal_stage_ic.py:26:40: E231 missing whitespace after ','

Copy link
Contributor

Choose a reason for hiding this comment

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

This test should be disabled on wcoss2 like the other gefs tests until we change the build system to build different executables.

@@ -39,7 +39,7 @@ if [[ "${RUN}" == "enkfgfs" ]]; then
fi

# Stochastic physics parameters (only for ensemble forecasts)
export DO_SKEB="YES"
export DO_SKEB=${DO_SKEB:-"YES"} # SKEB turned off for C96
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this is going to work, because DO_SKEB is set in config.fcst first, so the default here will never be used.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

DO_SKEB="NO" for C96 in config.ufs which is being propagated to the namelist. See log on hercules at /home/nbarton/GW/EXPDIR/SFS_C96mx100_S2S/LOGS_COMROOT/1994050100/fcst_mem001_seg0.log

@@ -43,6 +43,7 @@ export FHOUT=${FHOUT_GFS}
export FHOUT_HF=${FHOUT_HF_GFS}
export FHOUT_OCN=${FHOUT_OCN_GFS}
export FHOUT_ICE=${FHOUT_ICE_GFS}
export FHZER=@FHZER@
Copy link
Contributor

Choose a reason for hiding this comment

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

This variable is not used.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

? The variable is being propagated to the input.nml file in my test runs. see hercules at /home/nbarton/GW/EXPDIR/SFS_C96mx100_S2S/LOGS_COMROOT/1994050100/fcst_mem001_seg0.log

FHOUT_OCN_GFS: 6
FHOUT_ICE_GFS: 6
HPSSARCH: "NO"
LOCALARCH: "NO"
SFS_POST: "NO"
fcst:
reforecast: "YES"
Copy link
Contributor

Choose a reason for hiding this comment

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

The default should be "NO" and turned on for the tests that need it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed the default to NO

Comment on lines 30 to 31
USE_OCN_ENS_PERTURB_FILES: "YES"
USE_ATM_ENS_PERTURB_FILES: "YES"
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we really want to default to using these?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed the default to NO

sorc/build_ufs.sh Show resolved Hide resolved
Comment on lines +27 to +33
MOM6_INTERP_ICS=@MOM6_INTERP_ICS@
if [[ "${MOM6_INTERP_ICS}" == "YES" ]]; then
export MOM6_RESTART_SETTING='n'
export MOM6_WARMSTART_FILE="MOM.res.nc"
export MOM6_INIT_FROM_Z='False'
export MOM6_INIT_UV='file'
fi
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't we need a similar block in gefs/config.ocn?

Copy link
Contributor

Choose a reason for hiding this comment

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

Or maybe these should be set elsewhere in the run scripts based on $MOM6_INTERP_ICS

Copy link
Contributor Author

Choose a reason for hiding this comment

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

gefs/config.ocn is currently a soft link from gfs/config.ocn. i.e., each is set. If preferred to be set someplace else, I'm happy to edit

ush/forecast_predet.sh Outdated Show resolved Hide resolved
ush/forecast_predet.sh Outdated Show resolved Hide resolved
ush/forecast_predet.sh Outdated Show resolved Hide resolved
NeilBarton-NOAA and others added 4 commits October 17, 2024 16:37
Co-authored-by: Walter Kolczynski - NOAA <[email protected]>
Co-authored-by: Walter Kolczynski - NOAA <[email protected]>
Co-authored-by: Walter Kolczynski - NOAA <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants