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.

Sign up for GitHub

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

Jump to bottom

Simplify snapshot checking reporting and functionality #269

Merged
merged 3 commits into from
Jun 13, 2024
Merged

Simplify snapshot checking reporting and functionality #269

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
03c786f
Cleanup 'ncdata_check' comparisions and reporting.
nusbaume Jun 12, 2024
cb4f941
Fix formatting of the timestep log print statement.
nusbaume Jun 12, 2024
fc53040
Add new timestep number variable and root processor check.
nusbaume Jun 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions src/control/cam_comp.F90
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -455,7 +455,7 @@ end subroutine cam_run4
!
!-----------------------------------------------------------------------
!
subroutine cam_timestep_final()
subroutine cam_timestep_final(do_ncdata_check)
!-----------------------------------------------------------------------
!
! Purpose: Timestep final runs at the end of each timestep
Expand All @@ -464,12 +464,15 @@ subroutine cam_timestep_final()

use phys_comp, only: phys_timestep_final

!Flag for whether a snapshot (ncdata) check should be run or not
logical, intent(in) :: do_ncdata_check

!
!----------------------------------------------------------
! PHYS_TIMESTEP_FINAL Call the Physics package
!----------------------------------------------------------
!
call phys_timestep_final()
call phys_timestep_final(do_ncdata_check)

end subroutine cam_timestep_final

Expand Down
17 changes: 11 additions & 6 deletions src/cpl/nuopc/atm_comp_nuopc.F90
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1049,6 +1049,7 @@ subroutine ModelAdvance(gcomp, rc)
integer :: lbnum
integer :: localPet, localPeCount
logical :: first_time = .true.
logical :: do_ncdata_check !Flag notifying SIMA if it is OK to perform a snapshot check
character(len=*),parameter :: subname=trim(modName)//':(ModelAdvance) '
!-------------------------------------------------------------------------------

Expand Down Expand Up @@ -1117,13 +1118,15 @@ subroutine ModelAdvance(gcomp, rc)
end if

dosend = .false.
do_ncdata_check = .false.
do while (.not. dosend)

! TODO: This is currently hard-wired - is there a better way for nuopc?
! Need to not return when nstep = 0 and return when nstep = 1
! Note that the model clock is updated at the end of the time step not at the beginning
if (get_nstep() > 0) then
dosend = .true.
do_ncdata_check = .true.
end if

! Determine if time to write restart
Expand Down Expand Up @@ -1152,6 +1155,7 @@ subroutine ModelAdvance(gcomp, rc)
endif

! Run CAM (run2, run3, run4)
! This includes the "physics_after_coupler" CCPP physics group.

call t_startf ('CAM_run2')
call cam_run2( cam_out, cam_in )
Expand All @@ -1165,7 +1169,7 @@ subroutine ModelAdvance(gcomp, rc)
call cam_run4( cam_out, cam_in, rstwr, nlend, &
yr_spec=yr_sync, mon_spec=mon_sync, day_spec=day_sync, sec_spec=tod_sync)
call t_stopf ('CAM_run4')
call cam_timestep_final()
call cam_timestep_final(do_ncdata_check=do_ncdata_check)

! Advance cam time step

Expand All @@ -1174,7 +1178,8 @@ subroutine ModelAdvance(gcomp, rc)
call t_stopf ('CAM_adv_timestep')
call cam_timestep_init()

! Run cam radiation/clouds (run1)
! Run CAM (run1)
! This includes the "physics_before_coupler" CCPP physics group.

call t_startf ('CAM_run1')
call cam_run1 ( cam_in, cam_out )
Expand Down Expand Up @@ -1401,11 +1406,11 @@ subroutine ModelFinalize(gcomp, rc)

rc = ESMF_SUCCESS

call shr_log_getLogUnit (shrlogunit)
call shr_log_setLogUnit (iulog)
call shr_log_getLogUnit(shrlogunit)
call shr_log_setLogUnit(iulog)

call cam_timestep_final()
call cam_final( cam_out, cam_in )
call cam_timestep_final(do_ncdata_check=.false.)
call cam_final(cam_out, cam_in)

if (masterproc) then
write(iulog,F91)
Expand Down
8 changes: 7 additions & 1 deletion src/data/registry.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -313,12 +313,18 @@
phys_timestep_init_zero="true">
<long_name>Total tendency from physics suite</long_name>
</variable>
<!-- Physics timestep -->
<!-- Timestep variables -->
<variable local_name="dtime_phys"
standard_name="timestep_for_physics"
units="s" type="real" kind="kind_phys">
<long_name>timestep for physics</long_name>
</variable>
<variable local_name="nstep"
standard_name="current_timestep_number"
units="count" type="integer">
<long_name>current timestep number</long_name>
<initial_value>0</initial_value>
</variable>
<!-- Error handling variables -->
<variable local_name="errcode"
standard_name="ccpp_error_code"
Expand Down
29 changes: 15 additions & 14 deletions src/physics/utils/phys_comp.F90
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,11 +185,9 @@ subroutine phys_timestep_init()
! Physics needs to read in all data not read in by the dycore
ncdata => initial_file_get_id()

! data_frame is the next input frame for physics input fields
! Frame 1 is skipped for snapshot files
!!XXgoldyXX: This section needs to have better logic once we know if
!! this is a physics test bench run.
data_frame = get_nstep() + 2
! data_frame is the next input frame for
! physics fields that must be read from a file:
data_frame = get_nstep()

! Initialize host model variables that must be done each time step:
call physics_types_tstep_init()
Expand Down Expand Up @@ -237,32 +235,35 @@ subroutine phys_run2()

end subroutine phys_run2

subroutine phys_timestep_final()
subroutine phys_timestep_final(do_ncdata_check)
use time_manager, only: get_nstep
use cam_abortutils, only: endrun
use cam_initfiles, only: unset_path_str
use cam_ccpp_cap, only: cam_ccpp_physics_timestep_final
use physics_inputs, only: physics_check_data

! Subroutine inputs
logical, intent(in) :: do_ncdata_check

! Local variables
integer :: data_frame
integer :: data_frame

! Finalize the time step
call cam_ccpp_physics_timestep_final(phys_suite_name)
if (errcode /= 0) then
call endrun('cam_ccpp_physics_timestep_final: '//trim(errmsg))
end if

! data_frame is the next input frame for physics input fields
! Frame 1 is skipped for snapshot files
!!XXgoldyXX: This section needs to have better logic once we know if
!! this is a physics test bench run.
data_frame = get_nstep() + 2
! data_frame is the next input frame for
! physics snapshot validation fields
data_frame = get_nstep()

! Determine if physics_check should be run:
if (trim(ncdata_check) /= trim(unset_path_str)) then
call physics_check_data(ncdata_check, suite_names, data_frame, &
min_difference, min_relative_value)
if (do_ncdata_check) then
Copy link
Collaborator

Choose a reason for hiding this comment

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

do we still want to include the check if ncdata_check is populated in order to proceed to the check? I see that there's logic in physics_check data to issue a warning ('WARNING: Namelist variable ncdata_check is UNSET. Model will run, but physics check data will not be printed') if it's not, but I can also imagine that warning would be confusing if you had no intention of running the check.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That's a good question! I guess my opinion would be to just skip the call to physics_check_data entirely if we can avoid it, so I would prefer to leave this ncdata_check if-statement here as-is.

That being said, I am also not sure if the extra internal logic inside of physics_check_data you mentioned is really hurting anything either (beyond maybe a few wasted clock cycles). So that combined with the fact that I am not totally sure what the long-term plan for the physics_check_data routine will be (e.g. will it eventually be called elsewhere in SIMA? Removed entirely?) makes me lean towards just leaving everything as-is on that front for now as well. Of course if you or @cacraigucar feel strongly one way or the other please let me know. Thanks!

Copy link
Collaborator

Choose a reason for hiding this comment

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

I like avoiding writing out the WARNING message for runs where we are not performing checks (normal CAM-SIMA runs in the future?) I figure when we get to that stage, we can make sure that everything is running "best" for CAM-SIMA and make any required tweeks at that time. So I don't have a strong feeling one way or the other.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Well, turns out I can't read. The if (trim(ncdata_check) /= trim(unset_path_str)) then is still there and I thought it was gone. So.. agreed and nevermind!

call physics_check_data(ncdata_check, suite_names, data_frame, &
min_difference, min_relative_value)
end if
end if

end subroutine phys_timestep_final
Expand Down
18 changes: 18 additions & 0 deletions src/utils/time_manager.F90
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -520,6 +520,13 @@ subroutine advance_timestep()

! Increment the timestep number.

! Use statements
use ESMF, only: ESMF_ClockAdvance
use cam_logfile, only: iulog
use physics_types, only: nstep
use spmd_utils, only: masterproc
use string_utils, only: stringify

! Local variables
character(len=*), parameter :: sub = 'advance_timestep'
integer :: rc
Expand All @@ -528,6 +535,17 @@ subroutine advance_timestep()
call ESMF_ClockAdvance( tm_clock, rc=rc )
call chkrc(rc, sub//': error return from ESMF_ClockAdvance')

! Set current timestep number for use in CCPP physics schemes:
nstep = get_nstep()

! Write new timestep to CAM log file.

if (masterproc) then
write(iulog,*) '------------------------'
write(iulog,*) 'CAM-SIMA time step advanced (nstep = '//stringify([nstep])//')'
write(iulog,*) '------------------------'
end if

! Set first step flag off.

tm_first_restart_step = .false.
Expand Down
Loading