-
Notifications
You must be signed in to change notification settings - Fork 137
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
Add support for disabling thermodynamics #389
Conversation
Since CICE-Consortium/CICE@83686a3f (Implement box model test from 2001 JCP paper (CICE-Consortium#151), 2018-10-22) one can set 'ktherm = -1' in the CICE namelist to disable the thermodynamics. This was added specifically for the "Box 2001" test case (configuration/scripts/options/set_nml.box2001 in CICE) which also sets 'calc_strair' to false. 'ktherm = -1' is documented as disabling the thermodynamics in the Icepack documentation since 2b27a78 (Update documentation including namelist tables (CICE-Consortium#322), 2020-06-05), although this capability does not exist in Icepack standalone, since the modifications to CICE_RunMod::ice_step in CICE-Consortium/CICE@83686a3f were not copied to the Icepack driver in icedrv_RunMod::ice_step. A simple implementation in Icepack would be to add the CICE driver logic added in CICE-Consortium/CICE@83686a3f in the Icepack driver. However, as explained below things are not as simple. It can be useful to disable thermodynamics but run the model with 'calc_strair = .true.'. This currently does not work as could be expected in CICE, since the atmospheric stresses are computed in icepack_atmo::icepack_atm_boundary, which is called from icepack_therm_vertical::icepack_step_therm1, itself called from ice_step_mod::step_therm1, which is only called if 'ktherm >= 0' (since CICE-Consortium/CICE@83686a3f). This leads to atmospheric stress being always zero under 'calc_strair = .true.', 'ktherm = -1'. In order to allow computing the atmospheric stresses in this case, add some logic the icepack_therm_vertical::icepack_step_therm1 to run some parts of the subroutine only if 'ktherm >= 0'. Namely, let the advanced snow physics, frzmlt_bottom_lateral, set_sfcflux, thermo_vertical, update_aerosol, update_isotope and compute_ponds_{cesm,lvl,topo} only be called if 'ktherm >=0'. Conversely, always run neutral_drag_coeffs, icepack_atm_boundary, increment_age, update_FYarea and merge_fluxes. This commit does changes behaviour for potential users of Icepack as a library who initialize Icepack with 'ktherm=-1', 'calc_strair=.true.' and call 'icepack_step_therm1', as this now results in most of the thermodynamics being disabled, and will yield non-zero atmospheric stresses. However, we do not anticipate this change to break a lot of users, since anyway setting 'ktherm=-1' before this commit had no effect whatsoever in Icepack. The new behaviour aruably makes more sense physically. An upcoming commit will implement support for setting 'ktherm=-1' in the Icepack driver. Since we modify the indentation of several lines to keep the code readable, this commit is best viewed using 'git diff --ignore-all-space'.
@JFLemieux73 asked me to PR that sooner than later so here it is :) A CICE PR will follow, maybe to apcraig/cgridDEV I guess? |
As noted in the previous commit, 'ktherm = -1' is documented as disabling the thermodynamics in the Icepack documentation since 2b27a78 (Update documentation including namelist tables (CICE-Consortium#322), 2020-06-05), although this capability does not exist in Icepack standalone. Update icedrv_RunMod::ice_step to call some subroutines only if the thermodynamics is enabled ('ktherm >= 0'). Namely, let prep_radiation, biogeochemistry, step_therm2 and step_radiation only be called when thermodynamics is enabled, while letting the rest of the subroutines always be called. As noted in the previous commit, 'step_therm1' in particular has to be always called since the computation of the atmospheric stresses are done in that subroutine.
2a1c2af
to
2419c72
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like a reasonable solution to me.
I had forgotten a |
In terms of implementation, I wonder if this will be confusing. I assume when ktherm=0, we will want to call into icepack_step_therm1 from CICE, but we do NOT call into the other icepack routines from CICE? It seems like we should be consistent. If icepack can be called when ktherm=0, then all the icepack methods should be able to handle a value of ktherm=0 and return cleanly. What we're doing here is creating a situation where some parts of icepack can (should) be called when ktherm=0 and others cannot. In terms of options, we could
Then in terms of the implementation in icepack_step_therm1, we have lots of "if (ktherm==0)" logic added. Should we just have a block at the top that is
Finally, are the changes in icedrv_RunMod just to make the code run faster? Are we calling subroutines now that we don't need to be? Should this also be implemented in CICE or is it already done there properly? One other thing. Since the variable ktherm lives in Icepack, we probably should update the icepack implementation so when icepack is called when ktherm=0, it behaves correctly. As currently implemented, I think we rely on the caller (i.e. CICE or E3SM) to manage calls to icepack when ktherm=0. That can still exist, but not requiring that logic is probably a good thing to do. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See #389 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was thinking we could just do:
if (ktherm < 0) return after the icepack_atmo call.
Dave
@dabail10, that's a valid approach as well. |
What's the status and consensus on this PR? still thinking about it? |
I think there is more discussion to be done before we merge. I think the implementation needs to be a bit more robust than what's implemented in this PR. |
Sorry, I was doing other stuff and have not had time to circle back to this. I agree that it might be too hacky as-is. I'll think of a cleaner approach and make this PR WIP in the meantime. Thanks to all of you for taking a look :) |
PR checklist
title
@phil-blain
ktherm=-1
modified in the namelist, it completed successfullyktherm=-1
)step_therm1
)set_nml.nothermo
?ktherm=-1
is already documentedSee commits messages.
This MR is best viewed with "hide whitespace changes" since I adjusted the indentation.