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

On branch jm-pr-multiple-instances-of-ccpp-physics #1000

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

On branch jm-pr-multiple-instances-of-ccpp-physics #1000

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
71a5bae
On branch jm-pr-multiple-instances-of-ccpp-physics
Feb 28, 2023
d90e5fe
On branch jm-pr-multiple-instances-of-ccpp-physics
Mar 1, 2023
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
27 changes: 16 additions & 11 deletions physics/GFS_phys_time_vary.fv3.F90
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ module GFS_phys_time_vary

public GFS_phys_time_vary_init, GFS_phys_time_vary_timestep_init, GFS_phys_time_vary_timestep_finalize, GFS_phys_time_vary_finalize

logical :: is_initialized = .false.
logical, dimension(200) :: is_initialized = .false. ! why 200?
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe another option would be to define "is_initialized" as an interstitial variable? Then each instance would have its own copy. This also keeps the instance business out of the physics schemes, but does come with the burden of adding more interstitials. Either way, changes to the schemes are needed, but we may need to discuss this more.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we could leave this as is. The logic to have is_initialized at the scheme-module level is already there, all that this is doing is make it a vector. But if there's a good reason for doing it differently (because similar changes were made to other schemes since this PR was created years ago), then that's fine too.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@climbfuji I made this change for Thompson, see https://github.com/ufs-community/ccpp-physics/blob/ufs/dev/physics/MP/Thompson/mp_thompson.F90#L57. I prefer the interstitial approach, which wouldn't be limited to an arbitrary limit on the number of instances (200).

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok, that's fine with me. Do we want to finish this off and make the same change for the other scheme that needs it?


real(kind=kind_phys), parameter :: con_hr = 3600.0_kind_phys
real(kind=kind_phys), parameter :: con_99 = 99.0_kind_phys
Expand Down Expand Up @@ -80,7 +80,7 @@ subroutine GFS_phys_time_vary_init (
zwtxy, xlaixy, xsaixy, lfmassxy, stmassxy, rtmassxy, woodxy, stblcpxy, fastcpxy, &
smcwtdxy, deeprechxy, rechxy, snowxy, snicexy, snliqxy, tsnoxy , smoiseq, zsnsoxy, &
slc, smc, stc, tsfcl, snowd, canopy, tg3, stype, con_t0c, lsm_cold_start, nthrds, &
errmsg, errflg)
instance, errmsg, errflg)

implicit none

Expand Down Expand Up @@ -170,6 +170,7 @@ subroutine GFS_phys_time_vary_init (
real(kind_phys), intent(in) :: con_t0c

integer, intent(in) :: nthrds
integer, intent(in) :: instance
character(len=*), intent(out) :: errmsg
integer, intent(out) :: errflg

Expand All @@ -189,15 +190,15 @@ subroutine GFS_phys_time_vary_init (
errmsg = ''
errflg = 0

if (is_initialized) return
if (is_initialized(instance)) return
iamin=999
iamax=-999
jamin=999
jamax=-999

!$OMP parallel num_threads(nthrds) default(none) &
!$OMP shared (me,master,ntoz,h2o_phys,im,nx,ny,levs,idate) &
!$OMP shared (xlat_d,xlon_d,imap,jmap,errmsg,errflg) &
!$OMP shared (xlat_d,xlon_d,imap,jmap,instance,errmsg,errflg) &
!$OMP shared (levozp,oz_coeff,oz_pres,ozpl) &
!$OMP shared (levh2o,h2o_coeff,h2o_pres,h2opl) &
!$OMP shared (iamin, iamax, jamin, jamax) &
Expand Down Expand Up @@ -670,7 +671,7 @@ subroutine GFS_phys_time_vary_init (
endif noahmp_init
endif lsm_init

is_initialized = .true.
is_initialized(instance) = .true.

contains

Expand Down Expand Up @@ -718,7 +719,8 @@ subroutine GFS_phys_time_vary_timestep_init (
tsfc, tsfco, tisfc, hice, fice, facsf, facwf, alvsf, alvwf, alnsf, alnwf, zorli, zorll, &
zorlo, weasd, slope, snoalb, canopy, vfrac, vtype, stype, shdmin, shdmax, snowd, &
cv, cvb, cvt, oro, oro_uf, xlat_d, xlon_d, slmsk, landfrac, &
do_ugwp_v1, jindx1_tau, jindx2_tau, ddy_j1tau, ddy_j2tau, tau_amf, errmsg, errflg)
do_ugwp_v1, jindx1_tau, jindx2_tau, ddy_j1tau, ddy_j2tau, tau_amf, &
instance, errmsg, errflg)

implicit none

Expand Down Expand Up @@ -765,6 +767,7 @@ subroutine GFS_phys_time_vary_timestep_init (
snowd(:), cv(:), cvb(:), cvt(:), oro(:), oro_uf(:), slmsk(:)
integer, intent(inout) :: vtype(:), stype(:), slope(:)

integer, intent(in) :: instance
character(len=*), intent(out) :: errmsg
integer, intent(out) :: errflg

Expand All @@ -779,7 +782,7 @@ subroutine GFS_phys_time_vary_timestep_init (
errflg = 0

! Check initialization status
if (.not.is_initialized) then
if (.not.is_initialized(instance)) then
write(errmsg,'(*(a))') "Logic error: GFS_phys_time_vary_timestep_init called before GFS_phys_time_vary_init"
errflg = 1
return
Expand Down Expand Up @@ -912,13 +915,14 @@ end subroutine GFS_phys_time_vary_timestep_init
!!
!>\section gen_GFS_phys_time_vary_timestep_finalize GFS_phys_time_vary_timestep_finalize General Algorithm
!> @{
subroutine GFS_phys_time_vary_timestep_finalize (errmsg, errflg)
subroutine GFS_phys_time_vary_timestep_finalize (instance, errmsg, errflg)

implicit none

! Interface variables
character(len=*), intent(out) :: errmsg
integer, intent(out) :: errflg
integer, intent(in) :: instance

! Initialize CCPP error handling variables
errmsg = ''
Expand All @@ -930,19 +934,20 @@ end subroutine GFS_phys_time_vary_timestep_finalize
!> \section arg_table_GFS_phys_time_vary_finalize Argument Table
!! \htmlinclude GFS_phys_time_vary_finalize.html
!!
subroutine GFS_phys_time_vary_finalize(errmsg, errflg)
subroutine GFS_phys_time_vary_finalize(instance, errmsg, errflg)

implicit none

! Interface variables
character(len=*), intent(out) :: errmsg
integer, intent(out) :: errflg
integer, intent(in) :: instance

! Initialize CCPP error handling variables
errmsg = ''
errflg = 0

if (.not.is_initialized) return
if (.not.is_initialized(instance)) return

! Deallocate ozone arrays
if (allocated(oz_lat) ) deallocate(oz_lat)
Expand Down Expand Up @@ -970,7 +975,7 @@ subroutine GFS_phys_time_vary_finalize(errmsg, errflg)
if (allocated(tau_limb )) deallocate(tau_limb)
if (allocated(days_limb )) deallocate(days_limb)

is_initialized = .false.
is_initialized(instance) = .false.

end subroutine GFS_phys_time_vary_finalize

Expand Down
28 changes: 28 additions & 0 deletions physics/GFS_phys_time_vary.fv3.meta
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -902,6 +902,13 @@
dimensions = ()
type = integer
intent = in
[instance]
standard_name = ccpp_instance
long_name = argument so package knows which instance it is
units = index
dimensions = ()
type = integer
intent = in
[errmsg]
standard_name = ccpp_error_message
long_name = error message for error handling in CCPP
Expand All @@ -922,6 +929,13 @@
[ccpp-arg-table]
name = GFS_phys_time_vary_finalize
type = scheme
[instance]
standard_name = ccpp_instance
long_name = argument so package knows which instance it is
units = index
dimensions = ()
type = integer
intent = in
[errmsg]
standard_name = ccpp_error_message
long_name = error message for error handling in CCPP
Expand Down Expand Up @@ -1868,6 +1882,13 @@
type = real
kind = kind_phys
intent = inout
[instance]
standard_name = ccpp_instance
long_name = argument so package knows which instance it is
units = index
dimensions = ()
type = integer
intent = in
[errmsg]
standard_name = ccpp_error_message
long_name = error message for error handling in CCPP
Expand All @@ -1888,6 +1909,13 @@
[ccpp-arg-table]
name = GFS_phys_time_vary_timestep_finalize
type = scheme
[instance]
standard_name = ccpp_instance
long_name = argument so package knows which instance it is
units = index
dimensions = ()
type = integer
intent = in
[errmsg]
standard_name = ccpp_error_message
long_name = error message for error handling in CCPP
Expand Down
9 changes: 7 additions & 2 deletions physics/h2ointerp.f90
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,12 @@ subroutine read_h2odata (h2o_phys, me, master)
!--- h2o_pres - vertical pressure level (mb)
!--- h2o_time - time coordinate (days)
!---
allocate (h2o_lat(latsh2o), h2o_pres(levh2o),h2o_time(timeh2o+1))
! NOTE: If there are multiple instances of CCPP physics, only the first instance
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm surprised that this works. Only one instance is allocating space, but all instances are reading and writing to the same memory?

! allocates these. All instances must use the same number of identical ozone values
! jm 20230228
if ( .not.allocated(h2o_lat) ) allocate (h2o_lat(latsh2o))
if ( .not.allocated(h2o_pres)) allocate (h2o_pres(levh2o))
if ( .not.allocated(h2o_time)) allocate (h2o_time(timeh2o+1))
allocate (h2o_lat4(latsh2o), h2o_pres4(levh2o),h2o_time4(timeh2o+1))
rewind (kh2opltc)
read (kh2opltc) h2o_coeff, latsh2o, levh2o, timeh2o, h2o_lat4, h2o_pres4, h2o_time4
Expand All @@ -69,7 +74,7 @@ subroutine read_h2odata (h2o_phys, me, master)
!--- assume latitudes is on a uniform gaussian grid
!---
allocate (tempin(latsh2o))
allocate (h2oplin(latsh2o,levh2o,h2o_coeff,timeh2o))
if (.not.allocated(h2oplin)) allocate (h2oplin(latsh2o,levh2o,h2o_coeff,timeh2o))
DO i=1,timeh2o
do n=1,h2o_coeff
DO k=1,levh2o
Expand Down
24 changes: 16 additions & 8 deletions physics/mp_thompson.F90
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ module mp_thompson

private

logical :: is_initialized = .False.
logical, dimension(200) :: is_initialized = .False. ! why 200?

integer, parameter :: ext_ndiag3d = 37

Expand All @@ -39,6 +39,7 @@ subroutine mp_thompson_init(ncol, nlev, con_g, con_rd, con_eps, &
nwfa, nifa, tgrs, prsl, phil, area, &
aerfld, mpicomm, mpirank, mpiroot, &
threads, ext_diag, diag3d, &
instance, &
errmsg, errflg)

implicit none
Expand Down Expand Up @@ -83,6 +84,8 @@ subroutine mp_thompson_init(ncol, nlev, con_g, con_rd, con_eps, &
! Extended diagnostics
logical, intent(in ) :: ext_diag
real(kind_phys), intent(in ) :: diag3d(:,:,:)
! Which instance
integer, intent(in ) :: instance
! CCPP error handling
character(len=*), intent( out) :: errmsg
integer, intent( out) :: errflg
Expand All @@ -101,7 +104,7 @@ subroutine mp_thompson_init(ncol, nlev, con_g, con_rd, con_eps, &
errmsg = ''
errflg = 0

if (is_initialized) return
if (is_initialized(instance)) return

! Consistency checks
if (imp_physics/=imp_physics_thompson) then
Expand Down Expand Up @@ -133,7 +136,7 @@ subroutine mp_thompson_init(ncol, nlev, con_g, con_rd, con_eps, &

! For restart runs, the init is done here
if (restart) then
is_initialized = .true.
is_initialized(instance) = .true.
return
end if

Expand Down Expand Up @@ -304,7 +307,7 @@ subroutine mp_thompson_init(ncol, nlev, con_g, con_rd, con_eps, &
end if
end if

is_initialized = .true.
is_initialized(instance) = .true.

end subroutine mp_thompson_init

Expand Down Expand Up @@ -334,6 +337,7 @@ subroutine mp_thompson_run(ncol, nlev, con_g, con_rd, &
spp_prt_list, spp_var_list, &
spp_stddev_cutoff, &
cplchm, pfi_lsan, pfl_lsan, &
instance, &
errmsg, errflg)

implicit none
Expand Down Expand Up @@ -397,6 +401,8 @@ subroutine mp_thompson_run(ncol, nlev, con_g, con_rd, &
logical, intent(in) :: ext_diag
real(kind_phys), target, intent(inout) :: diag3d(:,:,:)
logical, intent(in) :: reset_diag3d
! Which instance
integer, intent(in ) :: instance

! CCPP error handling
character(len=*), intent( out) :: errmsg
Expand Down Expand Up @@ -501,7 +507,7 @@ subroutine mp_thompson_run(ncol, nlev, con_g, con_rd, &

if (first_time_step .and. istep==1 .and. blkno==1) then
! Check initialization state
if (.not.is_initialized) then
if (.not.is_initialized(instance)) then
write(errmsg, fmt='((a))') 'mp_thompson_run called before mp_thompson_init'
errflg = 1
return
Expand Down Expand Up @@ -861,22 +867,24 @@ end subroutine mp_thompson_run
!> \section arg_table_mp_thompson_finalize Argument Table
!! \htmlinclude mp_thompson_finalize.html
!!
subroutine mp_thompson_finalize(errmsg, errflg)
subroutine mp_thompson_finalize(instance,errmsg, errflg)

implicit none

! Which instance
integer, intent(in ) :: instance
character(len=*), intent( out) :: errmsg
integer, intent( out) :: errflg

! Initialize the CCPP error handling variables
errmsg = ''
errflg = 0

if (.not.is_initialized) return
if (.not.is_initialized(instance)) return

call thompson_finalize()

is_initialized = .false.
is_initialized(instance) = .false.

end subroutine mp_thompson_finalize

Expand Down
21 changes: 21 additions & 0 deletions physics/mp_thompson.meta
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,13 @@
type = real
kind = kind_phys
intent = in
[instance]
standard_name = ccpp_instance
long_name = argument to thompson mp so it knows which instance it is
units = index
dimensions = ()
type = integer
intent = in
[errmsg]
standard_name = ccpp_error_message
long_name = error message for error handling in CCPP
Expand Down Expand Up @@ -750,6 +757,13 @@
type = real
kind = kind_phys
intent = inout
[instance]
standard_name = ccpp_instance
long_name = argument to thompson mp so it knows which instance it is
units = index
dimensions = ()
type = integer
intent = in
[errmsg]
standard_name = ccpp_error_message
long_name = error message for error handling in CCPP
Expand All @@ -770,6 +784,13 @@
[ccpp-arg-table]
name = mp_thompson_finalize
type = scheme
[instance]
standard_name = ccpp_instance
long_name = argument to thompson mp so it knows which instance it is
units = index
dimensions = ()
type = integer
intent = in
[errmsg]
standard_name = ccpp_error_message
long_name = error message for error handling in CCPP
Expand Down
9 changes: 7 additions & 2 deletions physics/ozinterp.f90
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,12 @@ SUBROUTINE read_o3data (ntoz, me, master)
!--- oz_pres - vertical pressure level (mb)
!--- oz_time - time coordinate (days)
!---
allocate (oz_lat(latsozp), oz_pres(levozp),oz_time(timeoz+1))
! NOTE: If there are multiple instances of CCPP physics, only the first instance
! allocates these. All instances must use the same number of identical ozone values
! jm 20230228
if ( .not. allocated(oz_lat) ) allocate(oz_lat(latsozp))
if ( .not. allocated(oz_pres) ) allocate(oz_pres(levozp))
if ( .not. allocated(oz_time) ) allocate(oz_time(timeoz+1))
allocate (oz_lat4(latsozp), oz_pres4(levozp),oz_time4(timeoz+1))
rewind (kozpl)
read (kozpl) oz_coeff, latsozp, levozp, timeoz, oz_lat4, oz_pres4, oz_time4
Expand All @@ -78,7 +83,7 @@ SUBROUTINE read_o3data (ntoz, me, master)
!--- assume latitudes is on a uniform gaussian grid
!---
allocate (tempin(latsozp))
allocate (ozplin(latsozp,levozp,oz_coeff,timeoz))
if ( .not. allocated(ozplin) ) allocate (ozplin(latsozp,levozp,oz_coeff,timeoz))
DO i=1,timeoz
DO n=1,oz_coeff
DO k=1,levozp
Expand Down