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

Update implementation of some optional arguments in Icepack #429

Merged
merged 3 commits into from
Mar 1, 2023
Merged

Update implementation of some optional arguments in Icepack #429

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
67c8741
Update implementation of some optional arguments in Icepack
apcraig Feb 14, 2023
e607599
Add write_diags argument to icepack_init_fsd_bounds
apcraig Feb 15, 2023
4c9d45a
Clean up meltsliq implementation in icepack_step_therm1
apcraig Feb 15, 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
12 changes: 3 additions & 9 deletions columnphysics/icepack_fsd.F90
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,18 +120,10 @@ subroutine icepack_init_fsd_bounds(nfsd, &
real (kind=dbl_kind), dimension(:), allocatable :: &
lims

logical (kind=log_kind) :: &
l_write_diags ! local write diags

character(len=8) :: c_fsd1,c_fsd2
character(len=2) :: c_nf
character(len=*), parameter :: subname='(icepack_init_fsd_bounds)'

l_write_diags = .true.
if (present(write_diags)) then
l_write_diags = write_diags
endif

if (nfsd.eq.24) then

allocate(lims(24+1))
Expand Down Expand Up @@ -230,7 +222,8 @@ subroutine icepack_init_fsd_bounds(nfsd, &
c_fsd_range(n)=c_fsd1//'m < fsd Cat '//c_nf//' < '//c_fsd2//'m'
enddo

if (l_write_diags) then
if (present(write_diags)) then
if (write_diags) then
Comment on lines +225 to +226
Copy link
Member

Choose a reason for hiding this comment

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

This change, along with the deletions above, change the behaviour of the subroutine if write_diags is absent. Before, in that case we would write diagnostics since l_write_diags was initialized to .true. . Now we check if the argument is present, and so callers must always set write_diags to true if they want diagnostics.

I checked we already pass this argument in CICE drivers, bu we do not do so in the Icepack driver:

Icepack/configuration/driver/icedrv_InitMod.F90

Lines 100 to 105 in 67c8741

call icepack_init_fsd_bounds( &
nfsd=nfsd, & ! floe size distribution
floe_rad_l=floe_rad_l, & ! fsd size lower bound in m (radius)
floe_rad_c=floe_rad_c, & ! fsd size bin centre in m (radius)
floe_binwidth=floe_binwidth, & ! fsd size bin width in m (radius)
c_fsd_range=c_fsd_range) ! string for history output

I think this should be pointed out in the commit message, and maybe in the release notes ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are correct, thanks for noticing this @phil-blain. I missed that and assume it behaved like everything else in the code, didn't look closely enough. Overall, the diagnostics should either be on all the time OR they should be turned on via the interface. It doesn't make sense to me that they be on by default but can be turned OFF by the interface. That's counter to how everything works in CICE/Icepack in general. What do others think? For now, I think I'll update the call to icepack_init_fsd_bounds and set the flag to true.

write(warnstr,*) ' '
call icepack_warnings_add(warnstr)
write(warnstr,*) subname
Expand All @@ -244,6 +237,7 @@ subroutine icepack_init_fsd_bounds(nfsd, &
write(warnstr,*) ' '
call icepack_warnings_add(warnstr)
endif
endif

end subroutine icepack_init_fsd_bounds

Expand Down
10 changes: 6 additions & 4 deletions columnphysics/icepack_itd.F90
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -997,10 +997,12 @@ subroutine cleanup_itd (dt, ntrcr, &
faero_ocn(it) = faero_ocn(it) + dfaero_ocn(it)
enddo
endif
if (tr_iso) then
do it = 1, n_iso
fiso_ocn(it) = fiso_ocn(it) + dfiso_ocn(it)
enddo
if (present(fiso_ocn)) then
if (tr_iso) then
do it = 1, n_iso
fiso_ocn(it) = fiso_ocn(it) + dfiso_ocn(it)
enddo
endif
Comment on lines +1000 to +1005
Copy link
Member

Choose a reason for hiding this comment

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

OK, this change is in a different category, no ? here we are hardening the code since before we were using fiso_ocn without checking that it was present, instead assuming tr_iso would be true in that case.

This is a good improvement.

endif
if (present(flux_bio)) then
do it = 1, nbtrcr
Expand Down
39 changes: 11 additions & 28 deletions columnphysics/icepack_mechred.F90
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,7 @@ module icepack_mechred
implicit none

private
public :: ridge_ice, &
asum_ridging, &
ridge_itd, &
icepack_ice_strength, &
public :: icepack_ice_strength, &
icepack_step_ridge

real (kind=dbl_kind), parameter :: &
Expand Down Expand Up @@ -113,7 +110,7 @@ subroutine ridge_ice (dt, ndtd, &
dardg1ndt, dardg2ndt, &
dvirdgndt, &
araftn, vraftn, &
closing_flag,closing )
closing )

integer (kind=int_kind), intent(in) :: &
ndtd , & ! number of dynamics subcycles
Expand Down Expand Up @@ -161,7 +158,6 @@ subroutine ridge_ice (dt, ndtd, &
krdg_redist ! selects redistribution function

logical (kind=log_kind), intent(in) :: &
closing_flag, &! flag if closing is valid
tr_brine ! if .true., brine height differs from ice thickness

! optional history fields
Expand Down Expand Up @@ -296,7 +292,7 @@ subroutine ridge_ice (dt, ndtd, &
! Compute the area opening and closing.
!-----------------------------------------------------------------

if (closing_flag) then
if (present(opening) .and. present(closing)) then

opning = opening
closing_net = closing
Expand Down Expand Up @@ -595,11 +591,13 @@ subroutine ridge_ice (dt, ndtd, &
faero_ocn(it) = faero_ocn(it) + maero(it)*dti
enddo
endif
if (tr_iso) then
! check size fiso_ocn vs n_iso ???
do it = 1, n_iso
fiso_ocn(it) = fiso_ocn(it) + miso(it)*dti
enddo
if (present(fiso_ocn)) then
if (tr_iso) then
! check size fiso_ocn vs n_iso ???
do it = 1, n_iso
fiso_ocn(it) = fiso_ocn(it) + miso(it)*dti
enddo
endif
endif
if (present(fpond)) then
fpond = fpond - mpond ! units change later
Expand Down Expand Up @@ -1826,12 +1824,6 @@ subroutine icepack_step_ridge (dt, ndtd, &
real (kind=dbl_kind) :: &
dtt ! thermo time step

real (kind=dbl_kind) :: &
l_closing ! local rate of closing due to divergence/shear (1/s)

logical (kind=log_kind) :: &
l_closing_flag ! flag if closing is passed

logical (kind=log_kind), save :: &
first_call = .true. ! first call flag

Expand Down Expand Up @@ -1859,14 +1851,6 @@ subroutine icepack_step_ridge (dt, ndtd, &
! it may be out of whack, which the ridging helps fix).-ECH
!-----------------------------------------------------------------

if (present(closing)) then
l_closing_flag = .true.
l_closing = closing
else
l_closing_flag = .false.
l_closing = c0
endif

call ridge_ice (dt, ndtd, &
ncat, n_aero, &
nilyr, nslyr, &
Expand All @@ -1892,8 +1876,7 @@ subroutine icepack_step_ridge (dt, ndtd, &
dardg1ndt, dardg2ndt, &
dvirdgndt, &
araftn, vraftn, &
l_closing_flag, &
l_closing )
closing )
if (icepack_warnings_aborted(subname)) return

!-----------------------------------------------------------------
Expand Down
15 changes: 15 additions & 0 deletions columnphysics/icepack_orbital.F90
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,11 +177,24 @@ subroutine compute_coszen (tlat, tlon, &

real (kind=dbl_kind) :: ydayp1 ! day of year plus one time step

logical (kind=log_kind), save :: &
first_call = .true. ! first call flag

character(len=*),parameter :: subname='(compute_coszen)'

! Solar declination for next time step

#ifdef CESMCOUPLED
if (icepack_chkoptargflag(first_call)) then
if (.not.(present(days_per_year) .and. &
present(nextsw_cday) .and. &
present(calendar_type))) then
call icepack_warnings_add(subname//' error in CESMCOUPLED args')
call icepack_warnings_setabort(.true.,__FILE__,__LINE__)
return
endif
endif

Comment on lines +188 to +197
Copy link
Member

Choose a reason for hiding this comment

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

OK, so if I understand correctly these flags are not optional for CESM, so we add a check to that effect here?

If so, that makes sense.

Copy link
Contributor

Choose a reason for hiding this comment

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

This might complicate coupling in E3SM, if it's using CESMCOUPLED (I'm not sure). @njeffery

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, there are very few CESMCOUPLED cpps left in Icepack, we tried to clear them out a while ago and shift more to namelist settings. Use of CESMCOUPLED in icepack_orbital was discussed of that at the time. If this causes E3SM a problem, we can refactor it. The problem is that it will probably require a change in all the models using CESMCOUPLED in terms of how the orbital stuff is handled, so we're trying not to change that until we have to. But maybe we should do this at some point with consultation with CESM, RASM, etc so it's well coordinated. New implementation still TBD as well.

if (calendar_type == "GREGORIAN") then
ydayp1 = min(nextsw_cday, real(days_per_year,kind=dbl_kind))
else
Expand All @@ -206,6 +219,8 @@ subroutine compute_coszen (tlat, tlon, &
endif
#endif

first_call = .false.

end subroutine compute_coszen

!===============================================================================
Expand Down
Loading