Fix LAEA inverse rq normalization and authalic inverse series - #3280
Merged
Conversation
The oblique/equatorial LAEA inverse divided out the rq factor twice (once in the input normalization, again in the angular distance), inflating distances by ~0.11% -- up to 2.6 km for EPSG:3035 over Europe. Keep rq in rho so sce divides by it exactly once, matching PROJ's laea.cpp. Same fix in the CUDA kernel. The authalic latitude inverse series coefficients did not invert _authalic_q (4.8 m max error). Replace them with PROJ's pj_authset / Snyder eq. 3-18 coefficients (1.6 mm max error), which feed the AEA, CEA, and LAEA inverses on both CPU and CUDA paths. Measured parity vs pyproj after the fix: 3035 inverse 1.0 mm (was 2.6 km), 6933 inverse 1.6 mm (was 4.8 m), WGS84 AEA inverse 1.5 mm (was ~4.9 m). Forward kernels unchanged.
brendancol
commented
Jun 12, 2026
brendancol
left a comment
Contributor
Author
There was a problem hiding this comment.
PR Review: Fix LAEA inverse rq normalization and authalic inverse series
Blockers (must fix before merge)
None.
Suggestions (should fix, not blocking)
-
_projections.py_laea_inv_point:sce = 2.0 * math.asin(0.5 * rho / rq)has no clamp on the asin argument, unlike theratioclamps elsewhere in the same function. With the corrected (larger by 1/rq)rho, inputs beyond the projection disc reach the asin domain edge slightly sooner and produce NaN instead of a clamped pole value. NaN for out-of-disc input is defensible (PROJ errors there) and the downstream finite-filters handle it, but a one-line clamp would match the file's own conventions. Same spot in_d_laea_inv(_projections_cuda.py). -
test_reproject_inverse_kernels_3274.pycovers OBLIQ (3035) and N-polar LAEA but not the EQUIT branch (lat_0=0), which shares the fixed code path. I ran an EQUIT parity check locally (+proj=laea +lat_0=0 +lon_0=20, 500 points): max error 1.6 mm, so the fix holds there too; a pinned test would keep it that way.
Nits (optional improvements)
-
_authalic_invstill evaluates five series terms although terms 4-5 are now zero. The docstring explains the signature-stability tradeoff, so this is fine to leave; mentioning the dead terms could alternatively be dropped from the loop in a later cleanup.
What looks good
- The rq fix matches PROJ's laea.cpp normalization exactly (
x /= dd; y *= ddwith rho keeping rq), and the comment documents why the factor must stay in rho. - The replacement coefficients are pinned against the exact
_authalic_qnumeric inverse (round-trip test at <5e-9 rad) rather than against pyproj alone, so the test distinguishes a series bug from a datum-step difference. - The AEA test deliberately uses a WGS84 proj-string CRS instead of EPSG:5070, with a comment explaining the ~1-2 m NAD83 step that pyproj inserts and the fast path skips. That avoids a flaky tolerance.
- CUDA kernels updated in lockstep and verified two ways: against pyproj per pixel and against the CPU fast path at 1e-9.
- Kernel signatures and the
_APAarray shape are unchanged, so no call-site churn on either backend.
Checklist
- Algorithm matches reference (PROJ laea.cpp, pj_authset/Snyder 3-18)
- All implemented backends produce consistent results (CPU/CUDA parity test)
- NaN handling unchanged; finite-filtering downstream unaffected
- Edge cases: polar branch pinned, forward path pinned, end-to-end vs exact path
- EQUIT LAEA branch untested (verified manually, see suggestion)
- No materialization or copy concerns (scalar kernel math only)
- Benchmark not needed (no perf-relevant change)
- README matrix not applicable (no new function)
- Docstrings updated with measured error bounds and issue reference
brendancol
commented
Jun 12, 2026
brendancol
left a comment
Contributor
Author
There was a problem hiding this comment.
Follow-up review after the suggestion commit: both suggestions are addressed. The asin argument is now clamped in both the CPU kernel and the CUDA twin with a comment tying it to the existing ratio clamps, and the EQUIT-mode parity test pins the lat_0=0 branch at the same 5 cm tolerance (measured 1.6 mm). The nit about the two zero series terms stays as documented signature-stability. 11 regression tests and the full 460-test reproject suite pass. No remaining findings.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #3274
_laea_inv_point(oblique/equatorial) stripped therqfactor in its input normalization and then divided byrqagain in the angular distance, inflating distances by ~0.11% of the distance from the projection origin. Keeprqinrhososce = 2*asin(0.5*rho/rq)divides by it exactly once, matching PROJ'slaea.cpp. Same change in the CUDA kernel_d_laea_inv._authalic_apacoefficients did not invert_authalic_q(wrong at leading order in the second term, 17/360 vs 23/360). Replaced with PROJ'spj_authset/ Snyder eq. 3-18 coefficients. This feeds the AEA, CEA, and LAEA inverses on CPU and CUDA; array shape and kernel signatures are unchanged (terms 4-6 are now zero).Parity vs pyproj after the fix: EPSG:3035 inverse 1.0 mm (was 2.6 km), EPSG:6933 inverse 1.6 mm (was 4.8 m), WGS84 AEA inverse 1.5 mm (was ~4.9 m). Forward kernels were already at the 1e-5 m level and are unchanged. The residual ~2 m for EPSG:5070 is pyproj's WGS84->NAD83 step, which the fast path skips by design.
Backends: numpy and dask+numpy share the CPU kernels; cupy and dask+cupy use the fixed CUDA kernel. CUDA tests ran on a real GPU.
Test plan:
test_reproject_inverse_kernels_3274.py: series round-trip vs_authalic_q, coefficient pin, inverse parity vs pyproj for LAEA/CEA/AEA (oblique and polar), forward regression pin, end-to-end fast path vstransform_precision=0, CUDA-vs-pyproj and CUDA-vs-CPU parity