-
Notifications
You must be signed in to change notification settings - Fork 0
Fixes to glc-ocn flux and pressure coupling #128
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
base: master
Are you sure you want to change the base?
Fixes to glc-ocn flux and pressure coupling #128
Conversation
|
This branch is currently based off of E3SM-Project#7215 and will need to be rebased once that gets merged. |
eb9603c to
3ec2596
Compare
Testing round oneFor now, this is just a test of melt rates computed in the coupler vs. in the ocean model, with Results are in: and are being compared against a baseline from: Here's the new analysis: Here is the comparison with the baseline: |
|
To do:
|
bd1c436 to
bb656dd
Compare
|
Okay, switching to Melt is indistinguishable mostly: Temperature shows enough difference to at least know the simulations aren't identical (phew!): |
|
Notes:
|
bb656dd to
df0d7db
Compare
df0d7db to
116c63e
Compare
Testing SummaryI have run 3 simulations to test this work. They are:
The comparison analysis is more illuminating:
|
|
Differences between |
| landIceInterfaceTracers(indexIT, i) = x2o_o % rAttr(index_x2o_Sg_blit, n) | ||
| landIceInterfaceTracers(indexIS, i) = x2o_o % rAttr(index_x2o_Sg_blis, n) |
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.
Why does the ocean need this in the coupled case? Is this just present for debugging?
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.
Yes, we output them as part of the monthly means. This is for us but also has been requested by collaborators when we've shared our simulation data.
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.
Ah, thanks!
116c63e to
6a9eac5
Compare
Presumably corresponding to MALI mesh cells that lie outside the ocean domain? What do you think the path forward is? |
Ignore it for now and count on the dry region to deal with this? |
Yes, that's what it looks like. I still need to compute the totals. |
|
Here's what I see: That's This was at the end of day 5. I think MALI is probably getting daily accumulated values and MPAS-Ocean 10-minute values so these may not be equal for that reason as well as the different coverage. |
|
The dry region would deal with this as long as those cells are below sea level and I imagine practically all of them are. |
Most should be, yes, because MALI wouldn't think that ice was floating if it though they were above sea level. But its always possible that a floating MALI cell falls within a cell that MPAS-Ocean decided was above sea level. We could route all MALI floating cells to some MPAS-Ocean cell but that feels to me like more trouble than it's worth. It might be better to remap the MPAS-Ocean mask to MALI and use that as a mask for where fluxes are allowed. But, again, I think that's overkill for now. |
|
I agree. I think we can cross that bridge when we review the ocean mesh with dry cells. |
Is this other reason equivalent to ignoring the covariational component of melt rates, e.g., |
|
@cbegeman, I think that's right but I also think it's not going to be a small term during the first 5 days of spin-up. But maybe later in the simulation. |
Previously, the coupler fields associated with ice-shelf boundary layer fields were being updated in "standalone" mode as well, even though they are not used.
When computing prognostic ice-shelf melt rates in the coupler, use the boundary-layer fields time averaged over the coupling interval, not the instantaneous fields.
This avoids checking a string inside of nested loops
If ice-shelf fluxes are computed in the coupler, update the associated MPAS-ocean fields. Also, update the temperature and salinity at the ice-ocean interface.
... and associated unused module variable `rhoLandIce`
Before this fix, the sea-ice density was incorrectly being used.
6a9eac5 to
bb4522d
Compare
bb4522d to
85a626a
Compare
|
@stephenprice, @matthewhoffman and @cbegeman, I have done my best to get the coupler budget terms added in: But something isn't quite right and I want to kick this over to the 3 of you until I'm back working with FAnSSIE. Each of the qiceli, qicelo, qicehi and qiceho terms are designed to be positive into their respective components, see: However, I have a 3-month SORRM G-case with static MALI running on Chrysalis with the budget terms turned on: One month in, the budget I see is: It's hard for me to tell why the ocean is getting a lot more fresh water than the ice is providing but that could be icebergs. But the troubling bit is that the glc contribution to hpolar is 0.0000000, whereas there should be the latent heat leaving the ice sheet and entering the ocean. I will try to track down the cause in the next couple of hours but after that I'll have to kick it over to you. Sorry for the slow progress. I tried to juggle this work with the All-Hands but couldn't manage to concentrate very well this week. |
85a626a to
8ec175b
Compare
|
Update: I think Copilot found the issue and it has been fixed in 8ec175b. Fingers crossed. |
|
Okay, I haven't been able to prove that the budgets are right but at least |
|
@xylar Thanks for trying to solve this before your break. I'll look into it. If you have a chance, I think it would be helpful to look at your baseline with ISMF in the ocean. I tried to look at |
|
@cbegeman, I had to move that run to HPSS at NERSC because we are continually running out of space on Chrysalis. I can bring it back. |
|
@cbegeman, I have restored the files in: I believe permissions are such that you should be able to read them. |
| budg_dataL(nf,ic,ip) = budg_dataL(nf,ic,ip) + ca_c*x2o_o%rAttr(index_x2o_Fogx_qicelo,n) | ||
| ! latent heat from ice shelf melt fluxes in the coupler | ||
| nf = f_hpolar | ||
| budg_dataL(nf,ic,ip) = budg_dataL(nf,ic,ip) + ca_c*x2o_o%rAttr(index_x2o_Fogx_qicelo,n)*shr_const_latice |
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.
| budg_dataL(nf,ic,ip) = budg_dataL(nf,ic,ip) + ca_c*x2o_o%rAttr(index_x2o_Fogx_qicelo,n)*shr_const_latice | |
| budg_dataL(nf,ic,ip) = budg_dataL(nf,ic,ip) - ca_c*x2o_o%rAttr(index_x2o_Fogx_qicelo,n)*shr_const_latice |
The sum of the area-weighted qicelo is positive in the coupler history files. Since I assume the area-weighted melt rate consistent with melting, we need a negative sign here so that latent heat leaves the ocean component.
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.
We should think about whether the sign should really be changed in the x2o_Fogx_qicelo field itself. The same is true for the comment below with x2g_Fogx_qiceli.
| if (index_x2g_Fogx_qiceli /= 0) then | ||
| ! qiceli and qicehi are positive into glc | ||
| nf = f_wpolar | ||
| budg_dataL(nf,ic,ip) = budg_dataL(nf,ic,ip) + ca_g*x2g_g%rAttr(index_x2g_Fogx_qiceli,n) |
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.
| budg_dataL(nf,ic,ip) = budg_dataL(nf,ic,ip) + ca_g*x2g_g%rAttr(index_x2g_Fogx_qiceli,n) | |
| budg_dataL(nf,ic,ip) = budg_dataL(nf,ic,ip) - ca_g*x2g_g%rAttr(index_x2g_Fogx_qiceli,n) |
The sum of the area-weighted qiceli is negative in the coupler history files. Since I assume the area-weighted melt rate consistent with melting, we need a negative sign here so that latent heat enters the glc component.
|
@xylar More significant changes are needed to the sensible heat term as I have written here: https://e3sm.atlassian.net/wiki/spaces/FAN/pages/5511118871/2025-09-30+FAnSSIE+Ocean+Team+meeting. We can discuss more at the meeting on Tuesday or you can reach out if you want to talk through some of this beforehand. |
|
@cbegeman, thanks for thinking about these things! Yes, I think I would benefit from chatting about these ahead of time, perhaps during our usual Tuesday meeting? |
matthewhoffman
left a 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.
@xylar , this is looking very close to complete. I went through each commit by inspection, and all the changes were clearly documented and easy to follow so thanks for that. I made two comments that may or may not need addressing.
| end if | ||
| ! Okay, now if we're in standalone or data mode, time to compute | ||
| ! the total land-ice freshwater flux | ||
| call compute_total_land_ice_freshwater_flux(meshPool, forcingPool) |
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.
Does this need to be inside a check of landIceCoupledOn?
| g2x_g % rAttr(index_g2x_Sg_icemask_floating, n) = li_mask_is_floating_ice_int(cellMask(i)) | ||
| g2x_g % rAttr(index_g2x_Sg_icemask_grounded, n) = li_mask_is_grounded_ice_int(cellMask(i)) | ||
| g2x_g % rAttr(index_g2x_Sg_icemask_below_sea_level, n) = belowSeaLevelMask * li_mask_is_ice_int(cellMask(i)) | ||
| g2x_g % rAttr(index_g2x_Sg_icemask_floating, n) = belowSeaLevelMask * li_mask_is_floating_ice_int(cellMask(i)) |
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.
We may want to change this to floating locations below sea level and connected to the open ocean (i.e. ignoring subglacial lakes). We've had to implement such a mask elsewhere in MALI to deal with unintentionally applying ocean-driven melt rates at lakes.





In addition to cosmetic changes (removing commented-out code), this merge includes:
landIceFluxesCoupledlogical to avoid checking a string inside of nested loops.This branch implements (or will implement) the changes described in:
https://acme-climate.atlassian.net/wiki/spaces/FAN/pages/4200497474/Design+Document+Prognostic+ice-shelf+melting+in+E3SM+coupler