-
Notifications
You must be signed in to change notification settings - Fork 0
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
Separate removal of Antarctic river and ice runoff #103
Separate removal of Antarctic river and ice runoff #103
Conversation
@@ -707,10 +707,11 @@ add_default($nl, 'config_flux_attenuation_coefficient_runoff'); | |||
# Namelist group: coupling # | |||
############################ | |||
|
|||
add_default($nl, 'config_remove_ais_river_runoff'); |
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 is a non-BFB change for simulations with DIB. @darincomeau and @cbegeman, do you agree that we no longer want to remove river runoff? Or should we make this change a separate PR?
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 think we mostly likely want to make this change, but I'd like to see a 30-year B-case before deciding to make this the default.
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 think that will be a lot to be able to afford for a SORRM run just to test this config option, but we can discuss it.
@cbegeman and @darincomeau, I would appreciate your review of this work. It's a step needed before https://acme-climate.atlassian.net/wiki/spaces/PSC/pages/4210098268/Design+Document+Data+iceberg+and+ice-shelf+melt+flux+patterns+for+E3SM+spin-up+runs but it's not directly related to that work and seems like something we need for any simulations with DIB and either DISMF or PISMF. |
I haven't had a chance to test these changes yet, and probably won't have a chance to do that until Thursday so for now just a look at the code is what I'm after. |
02a78c5
to
c06108d
Compare
The totals are computed incorrectly (they are missing areaCell) and are not currently used for anything within MPAS-Ocean or any diagnostics.
This is needed so so we can remove ice (solid) runoff but not river (liquid) runoff.
c06108d
to
5cb37fb
Compare
I did test this after all with the
So that obviously needs to be fixed. |
I reran I will run a test that actually includes removing ice runoff. |
Should we run B-case tests both with and without @chloewhicker's snowcapping changes from E3SM-Project#6413? If I remember correctly, those significantly changed both liquid and solid runoff from AIS. |
@xylar Let me know if you'd like me to run particular tests on different machines/compilers. The code looks good to me from visual inspection. |
I think that would be ideal but I don't think we will use that in any configuration other than SORRM. So I don't think it makes sense to do a long B-case run until we have a new version of the SORRM mesh and I'm not sure whether we think we can afford long B-case runs with a bunch of different configurations for comparison. We can certainly discuss this further. But, as an example, I've been sitting in the queue all day waiting to run a 1-hour test of |
@xylar Bummer. Thanks for the update. |
I ran a
As expected, I'm seeing:
|
I agree, we probably want this change only after E3SM-Project#6413 goes in. |
This is needed so so we can remove ice (solid) runoff but not river (liquid) runoff.
This merge also removes
totalRemovedRiverRunoffFlux
andtotalRemovedIceRunoffFlux
variables, as these were computed incorrectly (they were missingareaCell
) and they are not currently used for anything within MPAS-Ocean or any diagnostics.