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.

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

Simplify logic for glc_renormalize_smb #133

Closed
billsacks opened this issue Nov 14, 2020 · 7 comments · Fixed by #495
Closed

Simplify logic for glc_renormalize_smb #133

billsacks opened this issue Nov 14, 2020 · 7 comments · Fixed by #495
Assignees
Labels
bug Something isn't working

Comments

@billsacks
Copy link
Member

billsacks commented Nov 14, 2020

I think that this logic for glc_renormalize_smb in CMEPS:

! TODO: talk to Bill Sacks to determine if this is the correct logic
glc_coupled_fluxes = is_local%wrap%med_coupling_active(compglc,complnd)
! Note glc_coupled_fluxes should be false in the no_evolve cases
! Goes back to the zero-gcm fluxes variable - if zero-gcm fluxes is true than do not renormalize
! The user can set this to true in an evolve cases
select case (glc_renormalize_smb)
case ('on')
smb_renormalize = .true.
case ('off')
smb_renormalize = .false.
case ('on_if_glc_coupled_fluxes')
if (.not. glc_coupled_fluxes) then
! Do not renormalize if med_coupling_active is not true for compglc->complnd
! In this case, conservation is not important
smb_renormalize = .false.
else
smb_renormalize = .true.
end if

differs from the equivalent logic in MCT:

https://github.com/ESMCI/cime/blob/9db73e464988bbc10779128b350f6681205696c7/src/drivers/mct/main/prep_glc_mod.F90#L334-L356

In particular, note that the MCT side has a flag glc_coupled_fluxes. This flag is set by CISM based on whether CISM is operating in a fully-coupled sense. The need for this arises from the possibility that CISM can be running in a mode where it is doesn't send liquid/ice fluxes to the ocean (instead relying on CTSM to send the fluxes needed for conservation). (This is always the case when ice evolution is turned off, but it can also be the case even when ice evolution is turned on, depending on how a flag is set.) When we first implemented the renormalization, land ice scientists felt that the default behavior should be for renormalization to be off in this case.

@mvertens if it's straightforward to set the glc_coupled_fluxes logical in CISM's nuopc_cap, similarly to what's done in the MCT cap, then this could be made consistent. Note that this glc_coupled_fluxes flag potentially differs for each ice sheet when running with multiple ice sheets. If there's some challenge with doing this, then we could check with @whlipscomb and others if it's acceptable to change the default behavior.

Update (2021-10-18): See #133 (comment) for the latest plan.

@billsacks billsacks added the bug Something isn't working label Nov 14, 2020
@mvertens
Copy link
Collaborator

@billsacks - where are we with this issue? Has this been resolved?

@billsacks
Copy link
Member Author

Thanks for checking. I believe this is still an issue: I think the logic here differs from the logic that was used in MCT. Before spending time changing the logic, though, it's probably worth talking with @whlipscomb and possibly others about whether we definitely want to maintain the MCT logic or use something different here.

@mvertens
Copy link
Collaborator

@billsacks @whlipscomb - should we set up a quick meeting to talk about this. Since we have moved to CMEPS/CDEPS as the defaults - it would be important to sort this out as soon as possible.

@whlipscomb
Copy link

@billsacks – I have a couple of questions. Could you please remind me how renormalization is handled for multiple ice sheets? For Greenland only, we do a global sum on the CTSM side, and another global sum on the CISM side, and when these disagree, we multiply the CISM fluxes by a renormalization term to force agreement. (Actually, I think there are two global sums – one for accumulation and one for ablation – but it's the same idea.) Are these sums now done independently for each ice sheet, with a different CTSM sum for each CISM overlap region?

If so, then it makes sense that the value of smb_renormalize for each ice sheet would be linked to the value of glc_coupled_fluxes for each ice sheet. Is this what's now done in CMEPS, or is glc_coupled_fluxes still a global parameter?

Assuming glc_coupled_fluxes is ice-sheet specific, then the only logical difference I see is that the MCT logic checks lnd_prognostic before checking glc_coupled_fluxes. I can imagine a case where we have Antarctic CISM coupled to an active ocean with a data land model. In that case, we could have glc_coupled_fluxes = T for Antarctica, while lnd_prognostic = F. The CMEPS logic implemented by @mvertens would then force renormalization over Antarctica for the case 'on_if_glc_coupled_fluxes'. I don't have a strong intuition about this, but my first thought is that renormalization will do no harm. @billsacks, do you agree, or have I missed something?

@billsacks
Copy link
Member Author

For multiple ice sheets, @mvertens has made this work so that the "global" sums are actually just done over each ice sheet independently (this is accomplished by using a multiplying factor of icemask for the given ice sheet). I have verified that, with renormalization on, the results for Greenland are identical between a two-ice sheet run and a single ice sheet run, and similarly for Antarctica, so this seems to be working correctly (assuming you agree that this is the correct way to do the renormalization).

However, what is not implemented is an ice-sheet specific determination of smb_renormalize. In fact, even for a single ice sheet run, the logic for smb_renormalize differs from what it was for mct/cpl7: in mct/cpl7, the default value for smb_renormalize depended on whether CISM was operating in a fully-coupled sense, where it is sending fluxes to the ocean. Currently in CMEPS, the logic is just based on whether there is any land-glc coupling. This difference isn't obvious from looking at the code snippets in the original issue comment, and that's my fault for not being clearer there. In mct/cpl7, glc_coupled_fluxes is a flag set by CISM, and is based on whether zero_gcm_fluxes is true or false:

https://github.com/ESCOMP/CISM-wrapper/blob/6040f09b6f4baac7598ffe136de6b8829061238c/drivers/cpl/mct/glc_comp_mct.F90#L212-L221

In CMEPS, the logic is just based on whether there is any coupling between glc and lnd:

https://github.com/billsacks/CMEPS/blob/b9cdf1fde5c7b6fefa7c4c0c4294d0a84d032ed7/mediator/med_phases_prep_glc_mod.F90#L268

So there are two questions here:

  1. Is it important to maintain the same default behavior that was implemented in mct/cpl7, or is there some other (possibly simpler) default behavior that we want moving forward?
  2. Do we want to allow one ice sheet to have renormalization on and the other to have renormalization off, (a) for the defaults and (b) as a possibility set by the user?

For (1), I have personally found the current default rules confusing for myself and in helping others. I know there were good reasons behind the current rules, but I'm beginning to feel that it might be better to have a much simpler default: by default, this renormalization is always on, even if conservation isn't strictly necessary in this case. The reason I suggest this is that I remember there being some confusion in the past when people were comparing two different runs, where one had renormalization on and the other had renormalization off, as a result of the ice sheet being fully-coupled in one but not the other. But @whlipscomb, I'd like to hear your thoughts here (and possibly thoughts from others in the LIWG if appropriate).

For (2): the answer to (2a) will depend on what we do for (1).

For (2b) I assume we'd want this to be ice sheet-specific, though how to do this is not immediately clear to me: We have a mechanism for setting ice sheet-specific config values, but that's inside CISM. This namelist parameter is in the mediator. We could add an ice sheet-specific namelist setting to CMEPS, but that might be tricky. Alternatively, we could set this in the CISM config file and have each ice sheet pass this scalar flag to the mediator, but this could be ugly. I'm not sure what's best here. Honestly, for now, I'm sort of inclined to keep this a global flag until it becomes obvious that it's actually important to allow this to differ for ice sheets (unless, @whlipscomb, it's already pretty clear to you that this really will be needed).

@whlipscomb
Copy link

@billsacks – Thanks for the clarifications. I think I understand the logic now.

I agree with what you say above. I think it's simpler to have renormalization always on (for all ice sheets) by default, for just the reason you mention.

For 2b, I'm having trouble thinking of a case where a user would want to turn on renormalization for (say) Greenland but not Antarctica. So let's go with the simpler global approach: either on or off for all instances.

@billsacks
Copy link
Member Author

Thanks, @whlipscomb

What's needed in this case is actually pretty simple – though will change answers for T compsets:

(1) Remove all logic related to the glc_renormalize_smb option on_if_glc_coupled_fluxes. This namelist flag will now just have two possible values, on and off, with a default of on.

(2) Change the land ice documentation accordingly (there are a couple of mentions of glc_renormalize_smb in the land ice documentation that will need to be updated with this new behavior).

I am renaming this issue accordingly.

@billsacks billsacks changed the title Logic for glc_renormalize_smb differs from mct Simplify logic for glc_renormalize_smb Oct 18, 2021
@billsacks billsacks moved this from Needs Prioritization to Todo in CESM: infrastructure / cross-component SE priorities Nov 3, 2021
@billsacks billsacks moved this to To do in Land Ice Jun 5, 2024
billsacks added a commit that referenced this issue Aug 9, 2024
Simplify logic for glc_renormalize_smb

### Description of changes

Remove the glc_renormalize_smb option on_if_glc_coupled_fluxes. This had been the default. Change the default to 'on'. This maintains consistent behavior between different configurations.

### Specific notes

Contributors other than yourself, if any:

CMEPS Issues Fixed (include github issue #): Resolves #133

Are changes expected to change answers? YES: changes answers by greater than roundoff for T compsets (active GLC with DLND) (now SMB is remapped with renormalization for T compsets by default); answers should be bit-for-bit in all other configurations (unless there is some configuration/combination that I'm not thinking of...)

Any User Interface Changes (namelist or namelist defaults changes)? Removes `on_if_glc_coupled_fluxes` option to `glc_renormalize_smb`

### Testing performed
Please describe the tests along with the target model and machine(s) 
If possible, please also added hashes that were used in the testing

Tested in the context of `cesm3_0_beta02`: ran:
- SMS_Ld5.f10_f10_ais8gris4_mg37.I1850Clm50SpGag.derecho_intel.cism-test_coupling
- SMS_Ld5.f10_f10_mg37.1850_DATM%GSWP3v1_CLM50%SP_SICE_SOCN_MOSART_DGLC%NOEVOLVE_SWAV.derecho_intel [Like an I compset but with DGLC%NOEVOLVE]
- SMS_Ly2.f09_g17_gris20.T1850Gg.derecho_intel

All tests passed; the T compset test changed answers as expected but the other two were bit-for-bit.
@github-project-automation github-project-automation bot moved this from Todo ~ weeks to Done (or no longer holding things up) in CESM: infrastructure / cross-component SE priorities Aug 9, 2024
@github-project-automation github-project-automation bot moved this from To do to Done in Land Ice Aug 9, 2024
@github-project-automation github-project-automation bot moved this from To do to Done in Land Ice Aug 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: Done (or no longer holding things up)
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants