Skip to content

Gate CUDA reproject fast path on WGS84-compatible datums (#3094) - #3119

Merged
brendancol merged 4 commits into
mainfrom
deep-sweep-accuracy-reproject-2026-06-09-01
Jun 10, 2026
Merged

Gate CUDA reproject fast path on WGS84-compatible datums (#3094)#3119
brendancol merged 4 commits into
mainfrom
deep-sweep-accuracy-reproject-2026-06-09-01

Conversation

@brendancol

Copy link
Copy Markdown
Contributor

Closes #3094

  • Adds the non-WGS84 datum guard from reproject: numba fast path corrupts coordinates for non-WGS84 datums #2651 to try_cuda_transform. The CUDA dispatcher matched projected CRSs whose datum sits in the Helmert table (OSGB36, DHDN, MGI, ED50, NAD27), then projected them with WGS84 constants and no datum shift, putting cupy and dask+cupy results ~100 m off while numpy went through pyproj. With the guard, those pairs return None and the chunk worker falls back to the CPU path.
  • Adds TestNonWgsDatumCudaFastPath: bail-out checks for EPSG:27700 (both directions) and NAD27, a check that the WGS84 UTM fast path stays active, and an end-to-end numpy-vs-cupy parity test for a 4326 -> 27700 reproject (NaN masks equal, values within 1e-4).
  • Also carries the accuracy-sweep state CSV update for the reproject module.

Backend coverage: numpy and dask+numpy were already correct; this fixes cupy and dask+cupy.

Test plan:

  • pytest xrspatial/tests/test_reproject.py -k NonWgsDatum (10 passed, CPU + CUDA classes, run on a GPU host)
  • pytest xrspatial/tests/test_reproject.py -k "cupy or cuda or CuPy" (48 passed on a GPU host)
  • Verified pre-fix divergence: try_cuda_transform coordinates off by ~96 m vs pyproj; end-to-end cupy vs numpy max diff 0.094 on [0,1] data

try_cuda_transform never got the non-WGS84 datum guard that #2651 added
to the CPU fast paths. The projected CRS matchers accept any datum in
the Helmert table, so cupy and dask+cupy reprojects between WGS84 and
e.g. EPSG:27700 ran the WGS84 Krueger series with no datum shift and
landed ~100 m off while numpy went through pyproj. Bail to the CPU
path (which defers these pairs to pyproj) and add CUDA-side tests
mirroring TestNonWgsDatumNumbaFastPath plus an end-to-end
numpy-vs-cupy parity check.
@github-actions github-actions Bot added the performance PR touches performance-sensitive code label Jun 9, 2026

@brendancol brendancol left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

PR Review: Gate CUDA reproject fast path on WGS84-compatible datums (#3094)

Blockers (must fix before merge)

None found.

Suggestions (should fix, not blocking)

None.

Nits (optional improvements)

  • _get_datum_params runs crs.to_dict() (which goes through pyproj's to_proj4) twice per chunk now that the guard sits in try_cuda_transform (xrspatial/reproject/_projections_cuda.py:751). The CPU fast path pays the same per-chunk cost in try_numba_transform, so this is parity rather than a regression. If chunk-loop profiling ever shows it, a small memo cache keyed on the CRS would fix both paths at once. Not needed for this PR.

What looks good

  • The guard is a faithful mirror of the #2651 CPU fix and sits before the cupy array allocation, so bailed pairs don't pay for device buffers they never use.
  • Root cause is correctly identified: the projected-CRS matchers (_tmerc_params, _lcc_params, ...) accept any datum in the Helmert table via _is_wgs84_compatible_ellipsoid, so EPSG:27700 reached the WGS84 Krueger kernels. The guard cuts that off for every affected datum (OSGB36, DHDN, MGI, ED50, NAD27, ...) rather than special-casing one code.
  • Test coverage is the right shape: both directions for 27700, the NAD27 geographic case, a WGS84-UTM check that proves the fast path is not over-blocked, and an end-to-end numpy-vs-cupy parity test with exact NaN-mask agreement. The parity test would have caught the original bug (~0.094 value error on [0,1] data).
  • Post-guard, the cupy worker falls back to the CPU coordinate path, which already defers these pairs to pyproj, so all four backends now agree by construction.

Checklist

  • Algorithm matches reference (#2651 CPU guard)
  • All implemented backends produce consistent results (numpy-vs-cupy parity test, run on a GPU host)
  • NaN handling is correct (mask equality asserted)
  • Edge cases are covered by tests (both directions, geographic + projected non-WGS84 sides, fast-path-still-active)
  • Dask chunk boundaries handled correctly (guard is per-chunk, no shape changes)
  • No premature materialization or unnecessary copies (guard placed before allocation)
  • Benchmark not needed (correctness fix; affected pairs previously returned wrong data)
  • README feature matrix unchanged (no API or backend-support change)
  • Docstrings/comments explain the why, with issue references

Resolve sweep-accuracy-state.csv conflict: keep the rasterize row added
on main and the updated reproject row (#3094) from this branch.
@brendancol
brendancol merged commit 8008e9d into main Jun 10, 2026
6 of 7 checks passed
@brendancol
brendancol deleted the deep-sweep-accuracy-reproject-2026-06-09-01 branch June 19, 2026 02:34
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.

reproject: cupy backend skips datum handling for non-WGS84 datums (try_cuda_transform missing the #2651 guard)

1 participant