Skip to content

Guard _apply_vertical_shift against non-finite coords and add vertical CRS tests#1453

Merged
brendancol merged 2 commits intomainfrom
vertical-shift-finite-coords-and-tests
May 4, 2026
Merged

Guard _apply_vertical_shift against non-finite coords and add vertical CRS tests#1453
brendancol merged 2 commits intomainfrom
vertical-shift-finite-coords-and-tests

Conversation

@brendancol
Copy link
Copy Markdown
Contributor

Summary

  • Mask non-finite lon/lat coords before handing them to _interp_geoid_2d. Inverse-projecting output grid coords for the geoid lookup can emit inf or NaN at projection singularities. Those values reach _interp_geoid_point, where the longitude wrap loop spins forever on inf. The fix replaces non-finite coords with NaN and drops the matching pixels from the valid-shift mask so NaN from the geoid does not contaminate the output.
  • Add TestVerticalShift covering EGM96 to ellipsoidal and back, EGM96 to EGM2008 (skipped if the grid is not available), no-op when both vertical CRSes match or only one side is set, the projected-CRS inverse-projection branch, and a polar-stereographic regression test for the hang.

Closes #1451

Test plan

  • pytest xrspatial/tests/test_reproject.py::TestVerticalShift -v passes (7/7)
  • pytest xrspatial/tests/test_reproject.py -x passes (143/143)
  • Polar-stereographic regression test returns finite output and no inf

…RS tests

Inverse-projecting output coords for the geoid lookup can produce inf or
NaN at projection singularities (poles, antimeridian). Those values flow
into _interp_geoid_point, where the longitude wrap loop spins forever on
inf. Mask non-finite coords with NaN before the JIT batch and drop those
pixels from the valid-shift mask so NaN from the geoid does not contaminate
the output.

Add a TestVerticalShift class covering EGM96<->ellipsoidal round-trip,
EGM96 to EGM2008 (skipped if grid not available), no-op when both vertical
CRSes match or only one side is set, the projected-CRS inverse-projection
branch, and a polar-stereographic regression test for the hang.

Closes #1451
@github-actions github-actions Bot added the performance PR touches performance-sensitive code label May 4, 2026
@brendancol
Copy link
Copy Markdown
Contributor Author

@copilot resolve the merge conflicts in this pull request

…e test conflicts

Co-authored-by: brendancol <433221+brendancol@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

Copilot AI commented May 4, 2026

@copilot resolve the merge conflicts in this pull request

Merged main into this branch and resolved the conflict in test_reproject.py (commit c10e1d1). Both the TestVerticalShift class from this PR and the new test classes added to main (TestMetadataPreservation, TestDaskDtypeParity, TestMergeDaskParity, TestCupyReprojectParity) are preserved.

@brendancol brendancol merged commit d3948f3 into main May 4, 2026
11 checks passed
@brendancol brendancol deleted the vertical-shift-finite-coords-and-tests branch May 5, 2026 03:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

performance PR touches performance-sensitive code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

_apply_vertical_shift can hang on non-finite coords; vertical CRS path is untested

2 participants