CICE-Consortium / Icepack

Development repository for sea-ice column physics
Other
25 stars 131 forks source link

Deprecate zsalinity #439

Closed apcraig closed 1 year ago

apcraig commented 1 year ago

I'm in the process of deprecating zsalinity. It looks like the "UNDEPRECATE" implementation done earlier just made it impossible to run with zsalinity, but there is lots of code that actually has to still be removed. Is this correct? Should I try to do that or is this better done by someone with a better understanding of zsalinity?

eclare108213 commented 1 year ago

Is the code that still needs to be removed in the zsalinity module, or elsewhere?

apcraig commented 1 year ago

There is stuff other places. The zsalinity module will be deleted. But for instance, see below. The icepack parameters is fine, I guess we want to leave the setting there, but abort if it's turned on for backwards compatibility. I'm happy to start stripping out the solve_zsal blocks of code. Is solve_zsal just for zsalinity or are there other uses of that flag? Are there some other variables we can key off of to verify the zsalinity is completely removed?

icepack.depzsal/columnphysics>grep -i solve_zsal *.F90
icepack_itd.F90:      use icepack_parameters, only: solve_zsal, skl_bgc, z_tracers
icepack_itd.F90:            if (solve_zsal) then
icepack_itd.F90:            if (solve_zsal) then
icepack_itd.F90:            endif ! solve_zsal
icepack_parameters.F90:         solve_zsal   = .false. ,&! if true, update salinity profile from solve_S_dt
icepack_parameters.F90:         modal_aero_in, skl_bgc_in, solve_zsal_in, grid_o_in, l_sk_in, &
icepack_parameters.F90:         solve_zsal_in          ! if true, update salinity profile from solve_S_dt
icepack_parameters.F90:      if (present(solve_zsal_in)) then
icepack_parameters.F90:         if (solve_zsal_in) then
icepack_parameters.F90:         modal_aero_out, skl_bgc_out, solve_zsal_out, grid_o_out, l_sk_out, &
icepack_parameters.F90:         solve_zsal_out          ! if true, update salinity profile from solve_S_dt
icepack_parameters.F90:      if (present(solve_zsal_out)        ) solve_zsal_out   = solve_zsal
icepack_parameters.F90:        write(iounit,*) "  solve_zsal = ", solve_zsal
icepack_therm_bl99.F90:      use icepack_parameters, only: conduct, calc_Tsfc, solve_zsal
icepack_therm_bl99.F90:      if (solve_zsal) sw_dtemp = p1  ! lower tolerance with dynamic salinity
icepack_therm_itd.F90:      use icepack_parameters, only: z_tracers, solve_zsal, hfrazilmin
icepack_therm_itd.F90:         if (solve_zsal .or. z_tracers) &
icepack_therm_itd.F90:      if (solve_zsal) fzsal = fzsal - rhosi*vi0new/dt*p001*sss*salt_loss
icepack_therm_itd.F90:                     if (.not. solve_zsal) &
icepack_therm_itd.F90:               if (.NOT. solve_zsal)&
icepack_zbgc.F90:      use icepack_parameters, only: scale_bgc, ktherm, skl_bgc, solve_zsal
icepack_zbgc.F90:                                       solve_zsal,   bgrid, &
icepack_zbgc.F90:                                       solve_zsal,      bgrid,  &
icepack_zbgc.F90:            if (solve_zsal .and. vsnon1 .le. c0) then
icepack_zbgc.F90:            endif        ! solve_zsal
icepack_zbgc.F90:      if (solve_zsal) then
icepack_zbgc.F90:                                        solve_zsal, bgrid, &
icepack_zbgc.F90:         solve_zsal
icepack_zbgc.F90:         if (solve_zsal) then
icepack_zbgc.F90:         endif  ! solve_zsal
icepack_zbgc.F90:         if (solve_zsal) then
icepack_zbgc.F90:         endif           ! solve_zsal
icepack_zbgc.F90:            if (solve_zsal .and. k < nblyr + 1) then
icepack_zbgc.F90:            endif                    ! solve_zsal
icepack_zbgc.F90:         if (solve_zsal) then
icepack_zbgc.F90:         endif  ! solve_zsal
icepack_zbgc.F90:            if (scale_bgc .and. solve_zsal) then ! bulk concentration (mmol or mg per m^3)
icepack_zbgc.F90:            if (solve_zsal) trcrn(nt_bgc_S:nt_bgc_S+nblyr-1,n) = c0
icepack_zbgc.F90:               if (solve_zsal)  then
icepack_zbgc.F90:               endif ! solve_zsal
eclare108213 commented 1 year ago

Wow, I missed a lot. I think it's fine to remove all of it, and run the various test suites to make sure it doesn't mess up something else. Removing all of it might affect the hbrine tracer - that's the only thing I can think of to keep an eye on. zsalinity was originally developed for zBGC, which we will update and thoroughly test as part of the Icepack-E3SM merge; now the BGC uses the mushy thermo salinity rather than this zsalinity parameterization.

apcraig commented 1 year ago

OK, let me take a crack at removing the code and see what happens. Will keep you posted. I'm pretty sure you intentionally just turned off the zsalinity code during the initial deprecation without removing all the code because it was a bit invasive. That was a reasonable first step that would be easier to do and undo if we decided not to deprecate the feature. But now it does need to happen.

apcraig commented 1 year ago

When we deprecate zsalinity in Icepack, I assume we also want to deprecate zsalinity in the Icepack driver and CICE? In Icepack, I kept the option to set solve_zsal to true, but there is no more solve_zsal code and setting solve_zsal to true will lead to an Icepack abort. I would take the same approach in the Icepack driver and CICE, keep solve_zsal as a namelist, abort if it's set to true, and otherwise remove code we no longer need. Does that sound like the way we want to go? An alternative is just to keep all the solve_zsal code in the Icepack driver and CICE, Icepack will abort if we try to use it. I don't think we want the solve_zsal code laying around in the drivers though. Thoughts?

eclare108213 commented 1 year ago

I agree, it's better to remove as much of the zsal code as possible but keep the namelist options so the abort will be clean and provide useful diagnostic information.

apcraig commented 1 year ago

Looks like nt_bgc_S and bgc_S in general is only used with zsalinity. Does that sound right? Is there any reason to keep this tracer index for some future need? Otherwise, I'll remove it, right?

eclare108213 commented 1 year ago

Yes, I think that's right.