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

This adds additional write restart at end of run functionality. #984

Merged
merged 13 commits into from
Oct 28, 2024

Conversation

dabail10
Copy link
Contributor

For detailed information about submitting Pull Requests (PRs) to the CICE-Consortium,
please refer to: https://github.com/CICE-Consortium/About-Us/wiki/Resource-Index#information-for-developers

PR checklist

  • Short (1 sentence) summary of your PR:
    This checks for REST_OPTION = 'end' in the NUOPC/CMEPS driver and also a case where stop_ymd is not the same interval as the restarts.
    It also fixes some pointer file naming when multiple instances are turned on.
  • Developer(s):
    @jedwards4b
  • Suggest PR reviewers from list in the column to the right.
  • Please copy the PR test results link or provide a summary of testing completed below.
    This was tested using the nuopc/cmeps driver.
  • How much do the PR code changes differ from the unmodified code?
    • bit for bit
    • different at roundoff level
    • more substantial
  • Does this PR create or have dependencies on Icepack or any other models?
    • Yes
    • No
  • Does this PR update the Icepack submodule? If so, the Icepack submodule must point to a hash on Icepack's main branch.
    • Yes
    • No
  • Does this PR add any new test cases?
    • Yes
    • No
  • Is the documentation being updated? ("Documentation" includes information on the wiki or in the .rst files from doc/source/, which are used to create the online technical docs at https://readthedocs.org/projects/cice-consortium-cice/. A test build of the technical docs will be performed as part of the PR testing.)
    • Yes
    • No, does the documentation need to be updated at a later time?
      • Yes
      • No
  • Please document the changes in detail, including why the changes are made. This will become part of the PR commit log.

io_pio2/ice_restart.F90 has been modified to handle the multiple instances
nuopc/cmeps driver is modified

@anton-seaice
Copy link
Contributor

There was already a "write_restart_at_endofrun" option added to the nuopc driver : - https://github.com/CICE-Consortium/CICE/pull/969/files

Is this doing the same thing ?

@jedwards4b
Copy link
Contributor

Just making it functional with cesm.

@@ -1331,51 +1329,51 @@ subroutine ModelSetRunClock(gcomp, rc)
call ESMF_LogWrite(subname//'setting alarms for ' // trim(name), ESMF_LOGMSG_INFO)

!----------------
! Restart alarm
! Stop alarm
Copy link
Contributor

Choose a reason for hiding this comment

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

No - the reason they are reordered is so that a REST_OPTION=end can be initialized using the stop alarm. If you do the rest alarm first you can not initialize this option.

Copy link
Contributor

@anton-seaice anton-seaice left a comment

Choose a reason for hiding this comment

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

This builds and runs

cicecore/cicedyn/infrastructure/io/io_pio2/ice_restart.F90 Outdated Show resolved Hide resolved
cicecore/cicedyn/infrastructure/io/io_pio2/ice_restart.F90 Outdated Show resolved Hide resolved
character(len=*), parameter :: subname = '(init_restart_read)'

if (present(ice_ic)) then
filename = trim(ice_ic)
else
if (my_task == master_task) then
#ifdef CESMCOUPLED
write(pointer_file,'(a,i4.4,a,i2.2,a,i2.2,a,i5.5)') &
'rpointer.ice'//trim(inst_suffix)//'.',myear,'-',mmonth,'-',mday,'-',msec
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this duplicate https://github.com/ESCOMP/CICE/blob/6d8c7f68c2ef80da9973f27c0077e54018bdbbfe/cicecore/cicedyn/general/ice_init.F90#L1202
?
which says:

#ifdef CESMCOUPLED
      pointer_file = trim(pointer_file) // trim(inst_suffix)
#endif

Copy link
Contributor

Choose a reason for hiding this comment

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

No. That adds the inst_suffix which allows for single executable ensembles. This adds a date stamp.

Copy link
Contributor

Choose a reason for hiding this comment

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

In ice_init it uses pointer_file from the ice_in namelist but here it's hardcoded to 'rpointer.ice', I dont think we need both

Copy link
Contributor

Choose a reason for hiding this comment

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

I think either:

  • in ice_init you can abort with "'pointer_file' namelist not used in CESMCOUPLED,
  • use 'pointer_file' in ice_restart.F90 and remove the three lines in ice_init

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So, our thought here is to add lpointer_file for the purposes of the multiple instances. When not in CESM:

lpointer_file = pointer_file

Does this work?

Copy link
Contributor

Choose a reason for hiding this comment

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

Why not just this ?:

The inst_suffix was already added in ice_init

#ifdef CESMCOUPLED
            write(pointer_file,'(a,i4.4,a,i2.2,a,i2.2,a,i5.5)') &
                 pointer_file//'.',myear,'-',mmonth,'-',mday,'-',msec
            inquire(file=pointer_file, exist=file_exist)
            if (.not. file_exist) pointer_file = 'rpointer.ice'//trim(inst_suffix)
#endif

Copy link
Contributor

Choose a reason for hiding this comment

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

Because that ends up repeating the timestamp if you are starting from a restart.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh got it - thanks both, adding lpointer_file sounds good.

@DeniseWorthen
Copy link
Contributor

DeniseWorthen commented Oct 21, 2024

@dabail10 What is the current functionality (ie, why is it there?) of the stop_now variable.

call ESMF_ClockGetAlarm(clock, alarmname='alarm_stop', alarm=alarm, rc=rc)
if (ChkErr(rc,__LINE__,u_FILE_u)) return
if (ESMF_AlarmIsRinging(alarm, rc=rc)) then
if (ChkErr(rc,__LINE__,u_FILE_u)) return
stop_now = .true.
call ESMF_AlarmRingerOff( alarm, rc=rc )
if (ChkErr(rc,__LINE__,u_FILE_u)) return
else
stop_now = .false.
endif
call t_stopf ('cice_run_total')
! Need to stop this at the end of every run phase in a coupled run.
call ice_timer_stop(timer_total)
if (stop_now) then
call ice_timer_print_all(stats=.true.) ! print timing information
call release_all_fileunits
endif

@jedwards4b
Copy link
Contributor

Is there a reason that

  call ice_timer_print_all(stats=.true.) ! print timing information 
   call release_all_fileunits 

are not in the ice_finalize routine? I also think that turning off the alarm at line 1248 may be problematic - why do you need to turn off a stop alarm anyway?

@dabail10
Copy link
Contributor Author

@dabail10 What is the current functionality (ie, why is it there?) of the stop_now variable.

call ESMF_ClockGetAlarm(clock, alarmname='alarm_stop', alarm=alarm, rc=rc)
if (ChkErr(rc,__LINE__,u_FILE_u)) return
if (ESMF_AlarmIsRinging(alarm, rc=rc)) then
if (ChkErr(rc,__LINE__,u_FILE_u)) return
stop_now = .true.
call ESMF_AlarmRingerOff( alarm, rc=rc )
if (ChkErr(rc,__LINE__,u_FILE_u)) return
else
stop_now = .false.
endif
call t_stopf ('cice_run_total')
! Need to stop this at the end of every run phase in a coupled run.
call ice_timer_stop(timer_total)
if (stop_now) then
call ice_timer_print_all(stats=.true.) ! print timing information
call release_all_fileunits
endif

I'm not sure.

@eclare108213
Copy link
Contributor

@dabail10 What is the current functionality (ie, why is it there?) of the stop_now variable.

I'm not sure.

stop_now is used in the standalone driver to end the run based on the value of npt.
It appears in various other drivers, which might or might not have their own methods for ending runs. Feel free to clean up your driver if it's not needed.

@apcraig
Copy link
Contributor

apcraig commented Oct 23, 2024

@anton-seaice @dabail10 @DeniseWorthen

I will merge after you review and approve. This is all CMEPS driver stuff. Thanks.

Copy link
Contributor

@DeniseWorthen DeniseWorthen left a comment

Choose a reason for hiding this comment

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

I think this is fine for UFS.

@anton-seaice
Copy link
Contributor

There's a couple of minor review comments from me which are still open

@dabail10
Copy link
Contributor Author

@anton-seaice , have we addressed all of your comments?

Copy link
Contributor

@anton-seaice anton-seaice left a comment

Choose a reason for hiding this comment

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

Looks good ! - Thanks

@dabail10 dabail10 merged commit 7a4eb51 into CICE-Consortium:main Oct 28, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants