Skip to content

Ensure the model clock is advanced consistently for single and multiple NUOPC run phases#489

Closed
rmontuoro wants to merge 40 commits into
NOAA-EMC:developfrom
rmontuoro:bugfix/clock
Closed

Ensure the model clock is advanced consistently for single and multiple NUOPC run phases#489
rmontuoro wants to merge 40 commits into
NOAA-EMC:developfrom
rmontuoro:bugfix/clock

Conversation

@rmontuoro
Copy link
Copy Markdown
Collaborator

Description

Intermediate restarts are generated based on the number of times the model clock is advanced (na):
https://github.com/NOAA-EMC/fv3atm/blob/bc562d79211bb643e9b1ee9aa9831895d103efba/module_fcst_grid_comp.F90#L1021-L1024

https://github.com/NOAA-EMC/fv3atm/blob/bc562d79211bb643e9b1ee9aa9831895d103efba/module_fcst_grid_comp.F90#L1036-L1063

This update makes sure na is consistently computed when using either a single or multiple NUOPC run phases by rewinding the model clock by one time step after NUOPC run phase 1.

Issue(s) addressed

Testing

The changes were tested on Orion/Intel. Regression test logs from Orion are provided.

Dependencies

None

rmontuoro and others added 30 commits December 3, 2019 23:42
large scale rain, and convective rain at the end of
each coupling step if coupling with chemistry model.
in noah/osu land-surface model subdriver.
band layer cloud optical depths (0.55 and 10 mu channels)
to prevent floating invalid errors due to uninitialized
optical depth arrays.
coupling array at the beginning of each coupling step
if coupled with chemistry model.
the NUOPC Realize phase since it breaks coupling
with aerosol component.
@junwang-noaa
Copy link
Copy Markdown
Collaborator

@theurich @rsdunlapiv May I ask why the advanceCount increases in both the run phase1 and phase2 with run sequence:

 runSeq::
  @720
     ATM phase1
     ATM phase2
   @
 ::

while it does not in run sequence:

 runSeq::
  @720
     ATM
   @
 ::

Thanks.

@DusanJovic-NOAA
Copy link
Copy Markdown
Collaborator

@theurich @rsdunlapiv May I ask why the advanceCount increases in both the run phase1 and phase2 with run sequence:

 runSeq::
  @720
     ATM phase1
     ATM phase2
   @
 ::

while it does not in run sequence:

 runSeq::
  @720
     ATM
   @
 ::

Thanks.

What will happen if we have a third atm phase? Will that require reversing the clock twice, first after phase 1 and then again after phase 2? Something is not correct here, and adding this fix that reverses clock manually seems ad hoc. As far as I can tell we do not advance the model clock in the fv3 cap, which means it should be unnecessary to reverse it manually.

@DusanJovic-NOAA
Copy link
Copy Markdown
Collaborator

Instead of:
na = advanceCount
can the value of na be computed as:
na = (currTime - startTime) / timeStep

@DeniseWorthen
Copy link
Copy Markdown
Collaborator

I think we'd need to do

    elapsedTime = currTime - startTime
    call ESMF_TimeIntervalGet(elapsedTime, s_i8=elapsed_secs,rc=rc)
    na = elapsed_secs / timestep

where elapsedTime is an ESMF_TimeInterval

@DusanJovic-NOAA
Copy link
Copy Markdown
Collaborator

Looking at the block of code in fcst_run_phase_2 where intermediate restarts are generated I see that the time of restart is not actually determined by the value of na at all. The value of na is only used in an if test in which the number of seconds since the initial time is computed from Atmos%Time which does not depend on advanceCount. So, my suggestion is to simply remove na (advanceCount) from both fcst_run_phase_1 and fcst_run_phase_2, which will eliminate the need to rewind the clock manually in the cap.

@junwang-noaa
Copy link
Copy Markdown
Collaborator

@DusanJovic-NOAA Then we need an alternative way to for the if statement not to write out the restart files at the end of forecast, which is called in atmos_model_end. Otherwise it will be written out twice and delay the whole forecast time.

@DusanJovic-NOAA
Copy link
Copy Markdown
Collaborator

@DusanJovic-NOAA Then we need an alternative way to for the if statement not to write out the restart files at the end of forecast, which is called in atmos_model_end. Otherwise it will be written out twice and delay the whole forecast time.

Why do we need that if tests? Does the frestart array contain the end of the forecast? It should not.

@SMoorthi-emc
Copy link
Copy Markdown
Contributor

SMoorthi-emc commented Mar 2, 2022 via email

@junwang-noaa
Copy link
Copy Markdown
Collaborator

I think we can change it to:

if(atm_int_state%Time_atmos /= atm_int_state%Time_end) then

@DusanJovic-NOAA
Copy link
Copy Markdown
Collaborator

We need to write restart at the end of the forecast too (so that we can continue the forecast, for example in a long climate simulation). Moorthi

I think restart files are always written at the end of forecast regardless of what's in the frestart array. Is this correct? If it is then the forecast hour of the end of forecast should not be in the frestart array, and we don't need that if test.

@junwang-noaa
Copy link
Copy Markdown
Collaborator

That is not always true. We write out restart at fh=3 and 6 for gdas 12 hours forecast.

@SMoorthi-emc
Copy link
Copy Markdown
Contributor

We need to be able to write the restart at certain interval. For example, in the operations we write restart every 24 hours. For long integrations like CFS, we need to write restart every 10 or 15 days so that if the model crashes we don't have to start from 0 hour.

@junwang-noaa
Copy link
Copy Markdown
Collaborator

An fix without using the advanceCount is implemented and committed in PR#491. Will close PR.

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.

Restart files are incorrectly generated when using multiple run phases

5 participants