-
Notifications
You must be signed in to change notification settings - Fork 38
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
Create test for extracting a MALI subdomain from a larger domain #562
Create test for extracting a MALI subdomain from a larger domain #562
Conversation
TestingI created an Amery subdomain with the following cfg file on perlmutter:
|
@matthewhoffman, is this expected behavior for |
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, thanks for creating this great tool! See my comments regarding a small amount of repeated code for the flood-fill, and also my question about the desired behavior of extend_mesh
. I'll wait to approve until you respond to those.
compass/landice/tests/mesh_modifications/subdomain_extractor/subdomain_extractor.cfg
Show resolved
Hide resolved
compass/landice/tests/mesh_modifications/subdomain_extractor/extract_region.py
Outdated
Show resolved
Hide resolved
compass/landice/tests/mesh_modifications/subdomain_extractor/extract_region.py
Outdated
Show resolved
Hide resolved
compass/landice/tests/mesh_modifications/subdomain_extractor/extract_region.py
Show resolved
Hide resolved
Using ncremap will allow interpolatingn forcing files in addition to an initial condition file. But because this method has more dependencies and potential fragile points, plus it is also slower and will likely need to be run from a compute node, I am also retaining the old method using the MALI interp script.
This fixes an ncks error, because fields on vertices or edges pass through ncremap with the dimension size unchanged from the source mesh.
These could be SMB, TF, or other files on the same mesh
ce34c15
to
9bf5e7d
Compare
@trhille , I addressed the comments from your review and added a few significant new features:
On Perlmutter this cfg can be used to test this PR out. As set up, setting
|
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.
Nice work, @matthewhoffman! I'm approving based on inspection and your testing. Someone else should test independently, as we discussed.
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.
Looked through the code and ran the compass test case on Perlmutter. Everything seems to be working as intended
@@ -0,0 +1,58 @@ | |||
.. _mesh_modifications: |
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, this should have been named dev_mesh_modifications
. It has the same name as the section in the user's guide, which is producing warnings when building the documentation. Could you open a small PR to fix this when you have time?
(I accidentally commented previously on the commit, not the 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.
Addressed in #808
This merge adds a test case for a subdomain extractor. It allows a regional domain to be quickly created from an existing whole-ice-sheet domain using a mask. The regional mask can come from a region_mask MPAS netCDF file or a geojson file. Optionally, ancillary files (e.g. forcing or parameter files) can be remapped at the same time.
Checklist
api.rst
) has any new or modified class, method and/or functions listedTesting
in this PR) any testing that was used to verify the changes