Add a circular ice sheet test for the coupled MALI-Sea level model#748
Add a circular ice sheet test for the coupled MALI-Sea level model#748hollyhan wants to merge 30 commits intoMPAS-Dev:mainfrom
Conversation
matthewhoffman
left a comment
matthewhoffman
left a comment
There was a problem hiding this comment.
@hollyhan and @xylar , I did a quick pass through this PR and don't see any concerns with the overall structure. @hollyhan , this is great work, and I am very happy we have this idealized setup fully automated within compass! Once this is complete, I will think about how to add a version of this to our full_integration suite to be sure we are testing SLM with every PR.
I added a number of comments as I went - I'm sure you are aware of many of these clean up items, but I figured I might as well flag them as I went. There are a few slightly more significant items I mention - feel free if you want to discuss any of them. And like we talked about, for anything that seems beyond what you actually care about, let me know and I am happy to contribute. Xylar and I can do a more thorough review once the docs are ready and any other details are finalized.
|
|
||
| # other constants used in the fuction | ||
| # pi = 3.1415926535898 # pi value used in the SLM | ||
| rhoi = 910.0 # ice density |
There was a problem hiding this comment.
May want to get this from namelist or cfg to be safe. Could be added later
| smbfile.close() | ||
|
|
||
|
|
||
| def _build_mapping_files(config, logger, res, nglv, mali_mesh_file): |
There was a problem hiding this comment.
Is this identical (or very similar) to the version in the ISMIP6-2300 SLM test case PR (#749)? If so, we should move this to a compass landice framework module and call it from both places. That does not need to need to be in the scope of your revisions - I can do that in a follow up PR.
There was a problem hiding this comment.
Sounds good - I'll leave this comment unresolved so you can follow up with it.
| # adjust the lat-lon values | ||
| args = ['set_lat_lon_fields_in_planar_grid.py', | ||
| '--file', mali_mesh_file, | ||
| '--proj', 'ais-bedmap2-sphere'] |
There was a problem hiding this comment.
Flagging again that we need to add this in an MPAS-Tools PR. (Already mentioned in #749)
|
@hollyhan and @matthewhoffman, I just wanted to check in on this one. What's the status? |
hollyhan
force-pushed
the
hollyhan/compass/landice/slm_circ_icesheet
branch
from
77d6c8a to
27ad364
Compare
This is largely copied from the dome smoke test and has stretches of code from the test left in place as an example.
for the ice mass change variables
MPE: mean percentage error SLC: sea-level change Also enable the SLM output class to take in an assigned number of time indices of outputs to be analyzied
matthewhoffman
force-pushed
the
hollyhan/compass/landice/slm_circ_icesheet
branch
from
27ad364 to
e4e98c9
Compare
|
Rebased and force-pushed to refresh this PR. |
Review Findings from local Copilot reviewI used copilot locally to review this branch after I rebased it. I am recording the results here for me to address in subsequent commits and get this test case running again!
Nothing in the branch makes those paths configurable, so anyone outside that exact environment will fail as soon as
The config advertises
The runtime namelist hard-codes
In
Because Assumptions
|
* Fix heatmap facecolor keyword typo * Close visualization dataset after timestep loop * Set SLM data path to NERSC location
3 participants
Checklist
api.rst) has any new or modified class, method and/or functions listedE3SM-Projectsubmodule has been updated with relevant E3SM changesMALI-Devsubmodule has been updated with relevant MALI changesTestingin this PR) any testing that was used to verify the changes