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

Flexible restart write times (restart_fh) for med, ocn, ice, wav #2419

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

Conversation

NickSzapiro-NOAA
Copy link
Collaborator

@NickSzapiro-NOAA NickSzapiro-NOAA commented Aug 28, 2024

Commit Queue Requirements:

  • Fill out all sections of this template.
  • All sub component pull requests have been reviewed by their code managers.
  • Run the full Intel+GNU RT suite (compared to current baselines) on either Hera/Derecho/Hercules
  • Commit 'test_changes.list' from previous step

Description:

This PR enables writing forecast hour defined restarts ("restart_fh") to have option of forecast hour restarts in addition to existing functionality. Restart writes are triggered when input restart_fh forecast hours are evenly divisible by a component's timestep (internally compared in units of integer seconds) and skipped otherwise. The implementation builds on current restart_fh in MOM6 for CMEPS, MOM6, CICE, and WW3 via a new shr_is_restart_fh_mod shared module (located at UFS level in CDEPS-interface).

Currently, space-delimited floating point forecast hours are input in model_configure attributes, like restart_fh: 0.25 2.5 6 17 24. Such an attribute is well tested (e.g., output_fh for FV3 implemented in NOAA-EMC/fv3atm#345), but it may be more natural to move attribute to ufs.configure for all components once issue with reading list of floating points is resolved (https://github.com/orgs/esmf-org/discussions/291).

Testing with cpld_control_gfsv17 confirms that restarts are bit-for-bit when (1) sharing common times with RESTART_N and (2) instead of RESTART_N. This option is exercised in the cpld_control_p8 RT for med, ocn, ice, wav with restart files added to the baselines for comparison. Baselines are otherwise unchanged.

Commit Message:

* UFSWM - Flexible restart write times (restart_fh) for med, ocn, ice, wav
  * CICE - Flexible restart write times (restart_fh)
  * CMEPS - Flexible restart write times (restart_fh)
  * MOM6 - Flexible restart write times (restart_fh)
  * WW3 - Flexible restart write times (restart_fh)

Priority:

  • Normal

Git Tracking

UFSWM:

Sub component Pull Requests:

UFSWM Blocking Dependencies:


Changes

Regression Test Changes (Please commit test_changes.list):

  • Adds additional restart files to baseline

Input data Changes:

  • None

Library Changes/Upgrades:

  • No Updates

Testing Log:

  • RDHPCS
    • Hera
    • Orion
    • Hercules
    • Jet
    • Gaea
    • Derecho
  • WCOSS2
    • Dogwood/Cactus
    • Acorn
  • CI
  • opnReqTest (complete task if unnecessary)

@NickSzapiro-NOAA NickSzapiro-NOAA marked this pull request as draft September 8, 2024 19:40
@NickSzapiro-NOAA NickSzapiro-NOAA self-assigned this Sep 19, 2024
@DeniseWorthen
Copy link
Collaborator

DeniseWorthen commented Oct 15, 2024

@NickSzapiro-NOAA Could you start making the required changes in WW3 also, starting with my PR branch for the netCDF restarts? That branch is NOAA-EMC/WW3#1303. That would allow all components to have this feature available at the same time, which I think would be a priority.

There are script level changes required in UWM also to enable PIO in WW3. See my UWM PR #2445.

@NickSzapiro-NOAA
Copy link
Collaborator Author

Sounds great, @DeniseWorthen!

So the "restart_fh" module is now shared at:
https://github.com/NickSzapiro-NOAA/ufs-weather-model/blob/restart_fh_marine/CDEPS-interface/ufs/cdeps_share/shr_is_restart_fh_mod.F90

For WW3, it looks like it makes sense to keep restart_fh near alarm_restart in model/src/wav_comp_nuopc.F90.
If we init in ModelSetRunClock and check in ModelAdvance, this is the same as in MOM6 and CICE.

May I check that there is no concern about conflicts between the restart strides with the netcdf output option?

@DeniseWorthen
Copy link
Collaborator

@NickSzapiro-NOAA Thanks. My gut instinct is that there will be no similar stride conflict. When use_restartnc is set, the code totally bypasses the current code which determines whether a restart will be written. See for example the use of the logical flag (and it's .not. variant) in w3wavemd.F90 in my feature branch.

CF_mc = ESMF_ConfigCreate(rc=rc)
call ESMF_ConfigLoadFile(config=CF_mc,filename='model_configure' ,rc=rc)
!if (ChkErr(rc,__LINE__,__FILE__)) return
if (ESMF_LogFoundError(rcToCheck=rc, msg=ESMF_LOGERR_PASSTHRU, line=__LINE__, file=__FILE__)) return
Copy link
Collaborator

@DeniseWorthen DeniseWorthen Oct 16, 2024

Choose a reason for hiding this comment

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

Why can't you use the cleaner form ? I see it commented out.

if (ChkErr(rc,__LINE__,u_FILE_u)) return

Add

use dshr_methods_mod , only : chkerr

character(*), parameter :: u_FILE_u = &
       __FILE__

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We can, but that brings additional dependencies into this module besides ESMF. There are also quite some duplicate ChkErr functions floating around all just wrapping an ESMF function

@NickSzapiro-NOAA NickSzapiro-NOAA changed the title Flexible restart write times (restart_fh) for med, ocn, ice Flexible restart write times (restart_fh) for med, ocn, ice, wav Oct 17, 2024
@NickSzapiro-NOAA NickSzapiro-NOAA added the Baseline Updates Current baselines will be updated. label Nov 14, 2024
@@ -63,7 +63,7 @@ list(APPEND lib_src_files
${icepack_files}
${cice_mpi_comm_files}
${cice_nuopc_cmeps_driver_files}
${cice_cdeps_inline_files})
${cice_cdeps_share_files})
Copy link
Collaborator

Choose a reason for hiding this comment

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

I know this is a temporary solution. Does this mean that now cice/mom/ww3 caps now have dependency on the CDEPS share code?

Copy link
Collaborator Author

@NickSzapiro-NOAA NickSzapiro-NOAA Nov 15, 2024

Choose a reason for hiding this comment

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

Yes.

CICE and CMEPS already have dependency for CDEPS inline functionality.

MOM6 and WW3 now have restart_fh module in cmakes. While this is currently under CDEPS-interface, it is a ufs-weather-model file (not a CDEPS subcomponent file)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Baseline Updates Current baselines will be updated.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants