-
Notifications
You must be signed in to change notification settings - Fork 3
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
Redefine grounding line mask #37
Redefine grounding line mask #37
Conversation
Define grounding line for cells as grounded cells with at least one dynamic floating neighbor, and for cells and vertices as those with one grounded and one dynamic floating cell. Previously the grounding line was defined as where a grounded cell was adjacent to a floating (dynamic or non-dynamic) or open ocean cell. This definition is problematic because it prevents us from closing a mass budget, since flux at the grounding line in the previous definition was not necessarily leading to loss of grounded ice.
Update logic for ablation velocity after redefining grounding line. We will still use applyToGroundingLine to mean apply the ablation velocity at the margin of grounded ice, whether there is an ice shelf or not. However, the new definition of the grounding line required a change from using li_mask_is_grounding_line to li_mask_is_grounded_ice in one place.
@trhille , I ran this through COMPASS again today and confirmed it still gives the same result you indicated above - all tests pass except for |
The COMPASS results look acceptable, but we have some further questions about the impacts of this elsewhere in the code in areas that are not strongly tested by COMPASS currently. Based on our discussion here is a task list of things we need to understand before we can merge this:
|
Update on item 4 above. Here is grounding line flux for 8km AIS ISMIP6 runs for four configurations all using the same initial condition. The difference in the first years between blue/orange and green/red is the effect of redefining the GL mask. I.e., by making the GL mask only consider regions going from grounded to floating and excluding grounded marine termini, the GL flux is reduced ~10%. That feels about right to me for the fraction of flux leaving AIS through grounded marine termini, but I don't know actual numbers. |
Results from the
Execution error, also in baseline:
Fails validation and baseline comparison:
|
Results from the
Execution errors, also in baseline:
Execution errors, not in baseline:
Fails validation and baseline comparison:
Passes validation, fails baseline comparison:
|
Commit d65a0ab fixes a bug that was causing divergent behavior over several decades in Humboldt 1km test runs (HadGEM2, q=1/7, sigma_max = 170 kPa run from the Humboldt paper ensemble). The new mask definition was allowing ice-shelf melt to be applied on the floating non-dynamic fringe adjacent to the grounded margin, which was already being melted by the ISMIP6 face-melting parameterization. |
|
Fails validation and baseline comparison:
|
Execution errors, also in baseline:
Fails validation and baseline comparison:
However, the above test also fails validation on Passes validation, fails baseline comparison:
|
Baseline comparison failures in the following test cases are due to the new treatment of damage for floating non-dynamic cells:
The verbose output below shows that damage is different between Damage results:
calvingThickness results:
thickness results:
surfaceSpeed results:
|
|
||
floatingBasalMassBal(iCell) = 0.0_RKIND | ||
|
||
cycle ! No need to look over other neighbors |
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.
Should this be exit
instead of cycle
? I believe cycle just skips the rest of this iteration of the do loop, but we're already at the end here, so that won't actually do anything. exit will skip the rest of the iterations.
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 updated and tested changing this.
When zeroing melt on floating non-dynamic fringe, use li_mask_is_grounded_ice instead of li_mask_is_grounding_line. Using the grounding line mask here is not consistent with the new definition of the grounding line, and leads to ice-shelf melt being applied to the floating non-dynamic fringe, resulting in spuriously high mass loss.
After various attempts, it was found that the best way to ensure BFB behavior in the hydro model was to re-create the old GL mask (that includes both grounded to floating and grounded open ocean) as a new mask variable in the hydro model. More work could be done to make a more univerasl hydro mask and apply it uniformly, but this commit does the minimum to keep the hydro model BFB using the existing logic in the hydro model.
28afc92
to
1bb4c76
Compare
I have rewritten the changes to the hydro model to replicate the old GL mask just in the hydro module (1bb4c76), and that leads to my hydro tests being BFB with the old version of the code. With that, we have checked all to-do items, and this is ready to merge. I will perform a final full COMPASS test first. |
The final COMPASS test yields a baseline difference for the
This seems consistent with the differences @trhille identified above. |
One is new in this branch, the other was longstanding. In both cases, the old usage was not hurting anything, but was not actually doing anything. With this change, the code will stop looing over neighbors when it finds what it's looking for, which will be slightly more efficient. COMPASS testing indicates no change in results, as should be the case.
Previously said the change occured in May, but it is being merged in Oct 2022.
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.
@matthewhoffman, your changes to the SGH code all make sense to me. I pushed one trivial commit to update comments that refer to the grounding line mask change happening in May 2022, which I've updated to say "Oct 2022". I think it's ready to merge!
Previously the the mask referred to as the grounding line was in fact the marine margin of grounded ice, whether or not it was at the floatation thickness. This led to trouble with mass budget calculations, which had to approximated using extensive and arduous post-processing with verbose model output, rather than calculated to machine precision on-line. Now, instead of defining the grounding line as a grounded cell with a floating or open ocean neighbor, this PR defines the grounding line as a grounded cell with a dynamic floating neighbor. Logic has been updated in calving and subglacial hydrology routines to be consistent with this new definition.
This addresses the first bullet point in the list of known issues in #23.