Skip to content
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

Add support for planar periodic meshes #23

Merged
merged 17 commits into from
Jan 13, 2025

Conversation

andrewdnolan
Copy link
Collaborator

@andrewdnolan andrewdnolan commented Dec 19, 2024

This PR adds support for planar periodic meshes, by streamlining and abstracting the spherical wrapping functionality so that it works to both planar and spherical meshes. For this to work, I've added getter/setter methods for the period attributes that handle the planar periodic / spherical meshes details appropriately. This has also dramatically simplified the spherical wrapping procedure, which was unduly complicated (and insufficient) previously. Testing of new the spherical wrapping for vertex patches has revealed a bug in #12, which produces invalid geometries for vertices along culled boundaries. When these invalid geometries intersect the map projection boundary, cartopy struggles with them resulting in the patches spanning the entire plot area. This bug is fixed as part of this PR. Finally, for testing purposes this PR also adds a small planar periodic mesh to the datasets database.

Fixes #15.

@andrewdnolan andrewdnolan added the enhancement New feature or request label Dec 19, 2024
@andrewdnolan andrewdnolan self-assigned this Dec 19, 2024
@andrewdnolan
Copy link
Collaborator Author

andrewdnolan commented Dec 19, 2024

Planar Periodic Testing

import mosaic
import matplotlib.pyplot as plt

def test_periodic(descriptor, ds, loc):
    fig, ax = plt.subplots()

    mosaic.polypcolor(ax, descriptor, ds.indexToCellID * 0.0, ec='k', cmap='binary')
    mosaic.polypcolor(ax, descriptor, ds[f"indexTo{loc}ID"], alpha=0.5, ec='tab:orange')

    ax.scatter(ds[f"x{loc}"], ds[f"y{loc}"], color="tab:blue")

    ax.set_title(f"{loc} Patches")


ds = mosaic.datasets.open_dataset("doubly_periodic_4x4")

descriptor = mosaic.Descriptor(ds)

for loc in ["Cell", "Edge", "Vertex"]:
    test_periodic(descriptor, ds, loc)

plt.show()

Cell patches:

Cell_periodic

Edges patches:

Edge_periodic

Vertex patches

Vertex_periodic

@andrewdnolan andrewdnolan force-pushed the periodic_planar branch 2 times, most recently from 9de3628 to b5bba02 Compare December 20, 2024 23:37
Further testing is need to move the spherical wrapping into the
new method used by planar periodic meshes.
Ensures vertex patches along culled mesh boundaries have proper
counter-clockwise ordering. Has not been an issue previsouly since
the incorrect ordering is only an issue for cartopy along projection
boundaries.
Also, update the period when the projection is set after instantation.
The previous fix to the bug introduced by E3SM-Project#12 resulted in invalid
patch geometries for single kite patches (i.e. along culled boundary).
This was fixed and now unit tests for validity test for vertices as
well. Also added helper function to quickly identify problematic
patches and added said function to the documentation.
Also chnaged the reference transfrom to geodetic, which is
tecincally more correct than PlateCarree
@andrewdnolan andrewdnolan marked this pull request as ready for review January 12, 2025 02:44
@andrewdnolan
Copy link
Collaborator Author

@xylar I think this is ready for review. I've worked out all the lingering issue in vertex patch creation and am now including vertex patches in the unit tests. I've also updated the documentation to include information about the periodic mesh support and list of the projection we currently support.

Unit tests cover a lot of functionality for spherical plotting. There's a test for making sure no figure rendering hangs and there's also a test which manually converts patches to shapely geometries and check that they are all valid. Both test loop over the supported projections and three patch location, with all tests passing.

I still need to remove the in development warning from the readme / docs, but I think I'll do that as part of the PR for releasing v1.0.0.

I also ran some local tests with the Humboldt mesh that had been run the mesh converter and everything still looks good for the vertex patches.

Copy link
Collaborator

@xylar xylar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just a review by inspection of the code. Everything looks excellent to me. The new tests seem quite thorough, and the treatment of periodic boundaries looks reasonable.

docs/user_guide/wrapping.md Outdated Show resolved Hide resolved
Co-authored-by: Xylar Asay-Davis <[email protected]>
@andrewdnolan
Copy link
Collaborator Author

@xylar Thanks for looking through! I'm going to merge and move forward with releasing v1.0.0.

@andrewdnolan andrewdnolan merged commit 7dfb174 into E3SM-Project:main Jan 13, 2025
5 checks passed
@andrewdnolan andrewdnolan deleted the periodic_planar branch January 13, 2025 15:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Planar periodic meshes are not supported
2 participants