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

Face Area Fix, Face Dimension Variable, & Data Type Fix #283

Merged
merged 7 commits into from Apr 19, 2023

Conversation

philipc2
Copy link
Member

@philipc2 philipc2 commented Apr 18, 2023

Overview

  • Switches to np.intp instead of np.uintp due to numpy indexing being written for intp
  • Added test case to test for invalid indexing
  • Introduces the Mesh2_face_dimension variable, used for storing the maximum number of non-fill-value vertices/nodes used for constructing a face
  • Modified Face Area functions to use Mesh2_face_dimension
  • Added test case for Unit Sphere Area from the ESDS meeting

test/test_mpas.py Outdated Show resolved Hide resolved
@philipc2 philipc2 changed the title Face Area Fix & Addition of Face Geometry Variable Face Area Fix, Face Dimension Variable, & Data Type Fix Apr 18, 2023
@philipc2
Copy link
Member Author

One concern is that we are bending the interpretation of face_dimension. Below is how it is defined in the UGRID conventions:

The face_dimension attribute specifies which netcdf dimension is used to indicate the index of the face in the connectivity arrays. This is needed because some applications store the data with the fastest varying index first, and some with that index last. The default is to use the num_faces as fastest dimension; e.g. a (num_faces, 3) array for triangles, but some applications might use a (3, num_faces) order, in which case the face_dimension attribute is required to help the client code disambiguate. The edge_dimension attribute is similar for the edge connectivity arrays.

However, @rajeeja and I believe that this name is appropriate for this use case. Any other suggestions for a different name, other than face_geometry or face_dimension?

I was considering nMesh2_face_nodes, since it would give us the number of face nodes per face, but the name is confusing, since it's not a singular "dimension"

uxarray/grid.py Show resolved Hide resolved
@philipc2 philipc2 linked an issue Apr 18, 2023 that may be closed by this pull request
@philipc2 philipc2 requested a review from anissa111 April 18, 2023 20:05
@rajeeja
Copy link
Contributor

rajeeja commented Apr 18, 2023

@erogluorhan please review, this should fix the primal and dual mesh area calc.

Copy link
Member

@erogluorhan erogluorhan left a comment

Choose a reason for hiding this comment

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

Thanks for addressing this really fast!

@erogluorhan erogluorhan merged commit 84dd54d into main Apr 19, 2023
14 checks passed
@erogluorhan erogluorhan deleted the philipc2/face-geometry-face-area-fix branch June 9, 2023 21:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants