Skip to content

Merge coincident pole and antimeridian nodes in structured grids - #1690

Open
rajeeja wants to merge 7 commits into
mainfrom
rajeeja/structured-coincident-nodes
Open

Merge coincident pole and antimeridian nodes in structured grids#1690
rajeeja wants to merge 7 commits into
mainfrom
rajeeja/structured-coincident-nodes

Conversation

@rajeeja

@rajeeja rajeeja commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Closes #1689

  • _read_structured_grid matched nodes in the lon/lat plane, so the nlon+1 nodes at each pole and the two sides of the antimeridian were never merged.
  • Now matches in Cartesian space, so nodes that are the same point on the sphere collapse: 899 nodes at 1 deg, 3599 at 0.25 deg.
  • Merging leaves each polar quad with a repeated corner, so those faces are now stored as triangles padded with the fill value.
  • Pole valence is nlon as expected (36 at 10 deg, 360 at 1 deg) and V - E + F == 2 confirms a closed sphere.
  • Regional grids that touch neither pole nor the seam are unchanged.
  • Two regression tests added; the global one fails on main.

Match nodes in Cartesian space rather than the lon/lat plane so pole and
antimeridian nodes are recognized as the same point, and store the resulting
polar faces as triangles instead of quads with a repeated corner.
@rajeeja rajeeja self-assigned this Aug 19, 2026
@rajeeja

rajeeja commented Aug 19, 2026

Copy link
Copy Markdown
Contributor Author

Checked the docs and user guide for behavior changes: both datasets in user-guide/structured.ipynb are unaffected (air_temperature is regional, ersstv5 spans 0-358 deg so the seam never closes), and neither collapses a single node. Also fixed tol to stay in degrees rather than silently becoming a chord length, so the documented parameter keeps its meaning. Only grids that actually touch a pole or the antimeridian change.

@Sevans711 Sevans711 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thank you for your work on this! Requesting minor changes and clarifications. I will take a closer look at the tests after understanding things a bit better.

Can you clarify, what does this mean from your original comment?

  • Now matches in Cartesian space, so nodes that are the same point on the sphere collapse: 899 nodes at 1 deg, 3599 at 0.25 deg.

I was also confused about the following, but I left an inline comment about it, feel free to respond there if that is easier:

  • Merging leaves each polar quad with a repeated corner, so those faces are now stored as triangles padded with the fill value.

Comment thread uxarray/io/_structured.py Outdated
Comment thread uxarray/io/_structured.py Outdated
Comment thread uxarray/io/_structured.py Outdated
@Sevans711 Sevans711 added the bug Something isn't working label Aug 21, 2026

@erogluorhan erogluorhan left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Wow, it is great to see this fix! It's always been unsettling to see those lons unmerged at the pole with UXarray's structured grids.

float32 input (e.g. real climate datasets) silently ran the whole
xyz/tolerance pipeline at float32 precision, causing pole/antimeridian
merges to fail or merge only partially.
@rajeeja
rajeeja requested a review from Sevans711 August 21, 2026 16:01

@Sevans711 Sevans711 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks almost ready to merge now, thank you for the changes! Left inline comments with minor style request and noting one typo. Other than that I think it looks good!

Comment thread uxarray/io/_structured.py Outdated
Comment thread test/io/test_structured.py Outdated
Comment thread uxarray/io/_structured.py Outdated
Comment thread uxarray/io/_structured.py Outdated
Comment thread uxarray/io/_structured.py Outdated

@Sevans711 Sevans711 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks ready to merge, thank you for the implementation here and for the back and forth with addressing all of my comments!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Grid.from_structured does not merge coincident pole and antimeridian nodes

3 participants