Revamp approach to projecting spherical meshes#51
Revamp approach to projecting spherical meshes#51andrewdnolan merged 18 commits intoE3SM-Project:mainfrom
Conversation
used by descriptor. Will be used for culling cells that lie outside or cross the projection boundary. As part of the culling, the connectivity information will be altered so that mesh is not periodic across projection boundary
b7d87ad to
21de19e
Compare
|
The new method of determining invalid cells is also fast! And again this a onetime cost that will support all plotting methods. For the v3 hi-res, it takes me import mosaic
import xarray as xr
ds = xr.open_dataset("mpaso.RRSwISC6to18E3r5.20250205.nc")
%%timeit
descriptor = mosaic.Descriptor(
ds, use_latlon=True, projection=ccrs.Mollweide(), transform=ccrs.Geodetic()
)
# 5.69 s ± 35.3 ms per loop (mean ± std. dev. of 7 runs, 1 loop each)The |
|
@xylar @cbegeman There's still some more to do for this PR with regards to testing and documentation. But, I think this is ready for a review of the algorithmic approach: culling invalid cells. This allows us to support (basically) all cartopy projections and is indiscernible for any scientifically useful mesh. But, this will cause some artifacts for very coarse meshes which could causes issues in polaris. (I have some ideas of hacky ways we can keep things working in polaris, more to come on that). I'm curious to hear what you think about this. Any comments and/or concerns would be greatly appreciated! (Happy to make any test plots for a given mesh/projections that could be helpful in your evaluations). |
|
@andrewdnolan This seems like a great alternative! Are there any implications for plotting planar meshes? |
Nope! Planar non-periodic and planar periodic are unaltered by these changes. With regards to very coarse spherical meshes in |
xylar
left a comment
There was a problem hiding this comment.
@andrewdnolan, thanks for walking me through this! I think it looks great and I'm happy to approve. Let me know if you'd like me to either do some more testing or look at specific code.
6f4a9c8 to
7495d90
Compare
cbegeman
left a comment
There was a problem hiding this comment.
Approving on the basis of @andrewdnolan's explanation and testing and brief visual inspection of code changes. Thanks for giving this careful thought @andrewdnolan!
There was a problem hiding this comment.
Pull request overview
This PR revamps how spherical MPAS meshes are handled under Cartopy projections by switching from “periodic wrapping/correction” of patches to proactively culling cells (and dependent edges/vertices) that become invalid or cross projection boundaries after reprojection. This aims to unlock support for most Cartopy projections (including azimuthal ones) and reduce redundant per-plot correction work.
Changes:
- Add mesh-culling utilities and centroid computation to support projection-aware culling.
- Update
Descriptorto transform coordinates once, compute a cull mask, and store a culled internal mesh for plotting. - Expand/adjust tests and docs to reflect the new projection/culling behavior and broader projection coverage.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
mosaic/descriptor.py |
Transforms mesh coords, computes cull masks for projections, and culls spherical meshes instead of wrapping. |
mosaic/utils.py |
Adds cull_mesh, centroid utilities, and updates invalid-patch detection API to work on raw patch arrays. |
mosaic/polypcolor.py |
Updates array-location parsing to support culled-vs-unculled inputs via lookup tables. |
tests/test_spherical.py |
Expands projection coverage by iterating over Cartopy projections (excluding explicitly unsupported). |
tests/test_planar_periodic.py |
Switches patch validity checks to the new get_invalid_patches(patches) API. |
tests/test_polypcolor.py |
Adds duck-typing and dimension/length compatibility tests for culled vs unculled arrays. |
tests/test_utils.py |
Adds tests for compute_cell_centroid correctness and padding validation. |
docs/user_guide/* + docs/index.md |
Reorganizes/updates docs around spherical vs planar periodic behavior and adds new pages. |
pyproject.toml / dev-environment.txt / .pre-commit-config.yaml |
Updates tooling/deps (docs extras, dev env, mypy hook rev). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
handles default transfrom based on `is_spherical` and `latlon` attributes
Avoids possible issues with `-1` meaning no connecivity
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 17 out of 17 changed files in this pull request and generated 6 comments.
Comments suppressed due to low confidence (1)
mosaic/descriptor.py:242
- The
projectionproperty is annotated as always returning aCRS, but this setter acceptsNoneand the constructor defaultsprojectiontoNone. This makes the type contract inconsistent and can cause mypy issues. Consider annotating the getter/setter asCRS | Noneto match actual behavior.
@property
def projection(self) -> CRS:
"""The target projection for plotting."""
return self._projection
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>





Projection Support as it stands:
As of this PR, we only support a subset of cartopy projections, which are mostly rectangular projections. Because these projections are rectangular, we can assume they have a constant period across projection boundaries. Using this period we treat spherical meshes the same way we treat planar periodic meshes; manually correcting the coordinates of "nodes" that wrap across a periodic boundary.
Limitations of the current approach:
ccrs.AzimuthalEquidistant). Therefore, we can not support certain desired projections (e.g.ccrs.Orthographic) with out current approach.mosaic.polypcolorand will require doing totally redundant work for contours (Implement cell contouring on the native mesh #52) and any other additional plotting methods added in the futureProposed Revision:
In this PR I propose that instead of worrying about correcting cell coordinates that wrap across the projection boundary, we instead cull those cells along with other invalid cells and cells that lie outside of the projection boundary from the mesh dataset stored by
mosaic.Descriptor. Because cells that cross the projection boundary will be culled, there will not be any need for special handling or correcting, and we should be able to support nearly all cartopy projections.Benefits of the proposed solution:
Limitations of the current approach:
Closes #50, #21