-
Notifications
You must be signed in to change notification settings - Fork 371
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
Adding cryo fields to coupler budget accounting #6229
Adding cryo fields to coupler budget accounting #6229
Conversation
I was trying to confirm that this did not change coupler budget values in |
components/mpas-ocean/src/analysis_members/Registry_conservation_check.xml
Outdated
Show resolved
Hide resolved
@jonbob and I are in discussions on how to avoid sending additional coupling fields in non-polar configurations. |
Added Do not merge tag as current implementation is not backwards compatible due to new drv namelist option. |
Note I moved the Registry conservation check unit changes to it's own PR, since it's strictly not needed here: #6257 |
Testing, water budgetResults from a one month Coupler budget:
Ocean conservation analysis:
|
The coupler From the ocean conservation analysis: So the totals calculated by the coupler and ocean agree to the first 3 digits. |
Testing, heat budgetResults from a one month Coupler budget:
Ocean conservation analysis:
|
The coupler From the ocean conservation analysis: So the totals calculated by the coupler and ocean agree to the first 3 digits. |
Testing, no impact on non-polar configurationsConfirmed with one month |
Testing, backwards compatibilityI ran a In the situation where a user has an existing case before this PR, then updates the code that includes this PR, rebuilds, and tries to run, the model will error and abort looking for the new driver namelist. Adding |
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.
Now that we've resolved the question of the land ice heat flux sign, I'm happy with these changes. You've shown that the coupler accounting agrees with the ocean component's accounting, and that's the main thing I was looking for. Let me know if you would like any of my help with further testing. Thanks for your work on this @darincomeau!
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 good to me. It has already caught one mistake at least (the sign of the DISMF heat flux) and will be a big improvement in Polar runs going forward.
Additional TestingPasses SMS_Lm1_P512.ne30pg2_r05_IcoswISC30E3r5.WCYCL1850.chrysalis_intel
Also ran some G-case tests, where the budgets aren't useful but wanted to make sure nothing unexpected happened: SMS_Lm1_P512.TL319_IcoswISC30E3r5.GMPAS-JRA1p5.chrysalis_intel |
ba34b6a
to
8ab1f4e
Compare
Rebased on v3.0.0 Reran and PASSes SMS_Lm1_P512.ne30pg2_r05_IcoswISC30E3r5.WCYCL1850.chrysalis_intel ERS_Ld31_P512.ne30pg2_r05_IcoswISC30E3r5.WCYCL1850.chrysalis_intel |
@darincomeau What do you think about including these changes to I'll let you make the changes to What do you think about including these 2 changes in this PR and I'll open a separate one to fix up the ocean analysis members (conservation check and global stats)? |
@stephenprice @proteanplanet this one is ready to merged soon, I don't think we need anymore careful reviews but wanted to give you a heads up in case you had any concerns. @rljacob this one adds coupling fields, and a new driver namelist option. The namelist option was added to only use the new coupling fields in polar configurations, in an attempt to minimize disruption with backwards compatibility and avoid throwing errors. I also made the same changes in driver-mct to driver-moab to keep the code in sync, but I don't know how to test that - I can remove those changes if that's preferred. Also wanted to give you a heads up here in case you had any concerns. |
@darincomeau The cryo configuration is being added to https://github.com/E3SM-Project/SimulationScripts/blob/master/archive/PolarGroup/E3SM-Polar-Developer.sh, and we're also setting up an e3sm_cryo_developer test suite in https://github.com/E3SM-Project/E3SM/blob/master/cime_config/tests.py. I didn't see any PEM or PET tests. Even though the code suggests these won't be affected, I'll run them to be sure. |
Ok thanks @proteanplanet ! I agree it would be good to check PEM/PET, I've been trying to be as careful as possible here to avoid being disruptive. I can also run those since I'm already setup locally. |
Actually I'm just remembering I did run the |
passes:
waiting on:
|
Adding cryo fields to coupler budget accounting Adds new o2x coupling fields and terms to coupler budget for polar configurations: * Foxo_ismw - water from ice shelf basal melting (either prognostic or data) * Foxo_rrofl - water removed from Antarctica liquid runoff * Foxo_rrofi - water removed from Antarctica solid runoff * Foxo_ismh - heat from ice shelf basal melting * Foxo_rrofih - heat from removed ice runoff * Foxo_frazil_li - frazil ice formed in ice-shelf cavities ('land ice frazil') * Foxo_frazil_q - latent heat associated with land ice frazil These new fields are used to calculate wpolar and hpolar, which are Antarctic Ice Sheet imbalances in polar configurations. These coupler budget terms replace the existing iceberg wberg and hberg entries in the budget table, which are included in the wpolar and hpolar totals. Adds a driver namelist option (flds_polar) to only send these coupling fields in polar configurations. Changes to ocean conservation analysis member: * Turns on terms in ocean conservation analysis member when data ice-shelf melt fluxes are used. * Removes landIceFreshwaterFluxesOn conditional on writing land ice frazil terms * Removes accumulated land ice frazil salinity terms for salt conservation, since land ice frazil is assumed to have 0 salinity. [NML] [BFB]
Passed PEM_Ln9.ne30pg2_r05_IcoswISC30E3r5.WCYCL1850.chrysalis_intel as well. Merged to next |
<values match="last"> | ||
<value compset="_MPASO%IBPISMF_">TRUE</value> | ||
<value compset="_MPASO%IBDISMF_">TRUE</value> | ||
<value compset="_MPASSI%DIB_">TRUE</value> |
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 logic is missing the case when DIB is not active, but DISMF or PISMF is active.
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.
Can you make an E3SM issue? I already merged this to master and have been in the process of blessing the expected NML and regular DIFFs (which are just due to a change in the field list)
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, will do
This appears to have already been merged to next. Can @darincomeau 's changes be included in that? |
merged to master and expected NML and regular DIFFs blessed -- except for pm-cpu which may not report today |
NML and regular DIFFs blessed on pm-cpu |
Fix logical on flds_polar driver namelist In PR #6229, the logic on the new driver namelist option flds_polar was not set to true in G-cases where data icebergs were not active, but ice-shelf melt fluxes were. This caused the model to crash on ocn init in ocn_comp_mct.F. This expands the logic for defining that namelist option to TRUE to include these cases. Fixes #6349 [BFB]
Fix logical on flds_polar driver namelist In PR #6229, the logic on the new driver namelist option flds_polar was not set to true in G-cases where data icebergs were not active, but ice-shelf melt fluxes were. This caused the model to crash on ocn init in ocn_comp_mct.F. This expands the logic for defining that namelist option to TRUE to include these cases. Fixes #6349 [BFB]
Adds five new o2x coupling fields and terms to coupler budget for polar configurations:
Foxo_ismw
- water from ice shelf basal melting (either prognostic or data)Foxo_rrofl
- water removed from Antarctica liquid runoffFoxo_rrofi
- water removed from Antarctica solid runoffFoxo_ismh
- heat from ice shelf basal meltingFoxo_rrofih
- heat from removed ice runoffFoxo_frazil_li
- frazil ice formed in ice-shelf cavities ('land ice frazil')Foxo_frazil_q
- latent heat associated with land ice frazilThese new fields are used to calculate
wpolar
andhpolar
, which are Antarctic Ice Sheet imbalances in polar configurations. These coupler budget terms replace the existing icebergwberg
andhberg
entries in the budget table, which are included in thewpolar
andhpolar
totals.Adds a driver namelist option (
flds_polar
) to only send these coupling fields in polar configurations.Changes to ocean conservation analysis member:
landIceFreshwaterFluxesOn
conditional on writing land ice frazil termsFurther discussion in E3SM-Ocean-Discussion#74
[BFB]
[NML]