Implement cell contouring on the native mesh#52
Implement cell contouring on the native mesh#52andrewdnolan merged 23 commits intoE3SM-Project:mainfrom
Conversation
|
yes! excited for this one! |
Eularian checks/cycles are unnecessarily expensive because contour boundaries should only ever be "cycle graphs" with a maximum vertex degree of two. After line profiling and refactor we've sped things up by a factor of about 10.
andrewdnolan
force-pushed
the
contouring
branch
from
7ec7d85 to
9c700c5
Compare
andrewdnolan
force-pushed
the
contouring
branch
from
9c700c5 to
5b55bc2
Compare
tests for spatial components of contours (i.e. no self intersection)
networkx is overkill, with significant overhead, for the cycle/path graphs produced by contours
andrewdnolan
force-pushed
the
contouring
branch
from
aec3f33 to
34c80a1
Compare
There was a problem hiding this comment.
Pull request overview
Implements “discrete” contouring directly on the native MPAS mesh by treating contour boundaries as graph components, and exposes a Matplotlib-like mosaic.contour / mosaic.contourf API with accompanying tests and documentation.
Changes:
- Added native-mesh contour/contourf implementation (
mosaic/contour.py) with a lightweightContourGraphtraversal utility. - Centralized mesh-location inference by moving array-location logic into
Descriptor._get_array_location()and updatingpolypcolor/contour code to use it. - Added property/unit tests and new user/developer documentation pages/notebooks for contouring.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
mosaic/contour.py |
New contouring implementation (ContourSet subclass + MPAS contour generator + ContourGraph). |
mosaic/__init__.py |
Exposes contour/contourf at the top-level API. |
mosaic/descriptor.py |
Adds Descriptor._get_array_location() for shared mesh-location inference. |
mosaic/polypcolor.py |
Switches to Descriptor._get_array_location() for location inference. |
tests/test_contour_graph.py |
Adds unit/property tests for ContourGraph, including NetworkX cross-validation. |
tests/test_contour.py |
Adds Hypothesis-driven property tests and unit tests for contour/filled-contour geometry/topology. |
pyproject.toml |
Adds networkx to dev extras. |
dev-environment.txt |
Adds profiling tools plus networkx and hypothesis to the dev environment list. |
docs/user_guide/contouring.ipynb |
New user guide example for contouring usage. |
docs/developers_guide/design_docs/contouring.ipynb |
New/updated design doc notebook describing the contouring approach. |
docs/developers_guide/design_docs.md |
New index page for design docs section. |
docs/developers_guide/api.md |
Updates API reference to include contour/contourf and reorganizes sections. |
docs/index.md |
Adds user guide entry for contouring and updates toctree structure. |
docs/conf.py |
Enables Matplotlib Sphinx roles and adjusts Napoleon settings. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @settings(deadline=None, max_examples=200) | ||
| @given(data=st.data()) | ||
| def test_node_degrees_are_2(self, data: st.DataObject) -> None: |
There was a problem hiding this comment.
The Hypothesis settings here (deadline=None, max_examples=200) are repeated across many tests in this file, which can make the test suite significantly slower and remove safeguards against pathological cases. Consider reducing max_examples and/or reinstating a reasonable deadline (or applying settings at a module/class level) to keep CI runtimes predictable.
|
I think this is ready for review! I will work on getting the documentation changes made as part of this PR publicly available. @proteanplanet I have attempted to test for all the edge cases you handle in @milenaveneziani please review in whatever detail you'd like, no pressure. But, if there are any plots or meshes you'd like me to tests with this before it gets merged that would be very helpful. (Particularly, an example where there is a remapped version to compare to would be great!). |
|
Documentation can be found here:
|
xylar
left a comment
xylar
left a comment
There was a problem hiding this comment.
I haven't gone through the code in detail but the results look very good to me. I did a variant of the notebook for contouring, using the EC30to60 bathymetry as the field. I added more contours and made the figures large enough to see the hexagons. It looks great to me!
|
@andrewdnolan As a simple visual evaluation, please can you post here a polar stereographic map of the Arctic coastline from Icoswisc30e3r5, akin to this: 
|
|
A public API for plotting coastlines is not included in this PR. I've got something working that builds off of this PR but there will be a separate follow on PR for coastlines. (Planning to open that early next week). Also planing on a quick follow on for contouring fields defined at vertices. Here's a quick pass at recreating the figure above. (Where I've used my WIP coastline feature, that will be in the follow on PR). I'm not sure that the projection extent and/or levels used to make the contours exactly match, but I think it's close enough for a visual evaluation. |
proteanplanet
left a comment
proteanplanet
left a comment
There was a problem hiding this comment.
Approved. The Arctic is full of edge cases, and all the ones I know of are not being triggered.
|
oh my goodness, I just noticed this! So sorry for not reviewing. But I will try this asap for sure. Thanks @andrewdnolan! |
|
@milenaveneziani No worries! I'll be releasing a new version of So, we've got time for follow on PR's if you run into any issues. It would be great to confirm that this works as expected for your applications. Please let me know if you hit any issues. Thanks! |
|
Sounds good @andrewdnolan. I will keep an eye our for the new e3sm-unified release. |
Native mesh contouring
This PR implements contouring on the native MPAS mesh, with the approach inspired by
ridgepack. The contours produced are "discrete", in that contours follow control volume boundaries (i.e. the contours of cell fields will lie along edges of mesh). This differs from a classical contouring (e.g. marching squares), which expects data defined at discrete points (c.f. defined over a control volume) and general employs linear interpolation to produce smoothed contours.Contours of the unstructured mesh are treated as graphs. Because contours follow mesh boundaries, they should only ever be path or cycle graphs, which has no self intersections. We use
hypothesisfor property based testing to confirm these invariants for randomly generated contour fields.For the end user, we follow the
matplotlibcontour/contourfAPI as closely as possible, wheremosaic.contour/mosaic.contourfrequire the matplotlib Axes andmosiac.Descriptoras positional arguments, but all keyword arguments are exactly the same. In theory this also includes contour labeling, though more thorough testing is requite to validate this. We achieve this by sub-classingmatplotlib.contour.ContourSet, which is the the data structure matplotlib uses to process/handle contours.To Do:
Add support for vertex fields (?)networkxdependency and implement custom walkCoastlines (should this be a separate PR ?)