fix(rust/sedona-raster): align RS_Rotation with Sedona Spark's formula - #1223
Merged
james-willis merged 5 commits intoSep 4, 2026
Conversation
SedonaDB computes atan2(-skewX, scaleX); Sedona Spark acos(scaleX / hypot(scaleX, skewY)), negated when skewY > 0. The formulas coincide exactly when |skewX| == |skewY| — axis-aligned grids and rigid rotations of square pixels — and separate everywhere else. Anchored passing coverage for the agreeing cases (zero-sign normalized with + 0.0, the 1-ulp libm-vs-JVM noise rounded to 12 digits), xfails for the divergences: IEEE -0 vs 0 on axis-aligned grids, a rigid rotation of non-square pixels misreported by SedonaDB (-0.7137 for a true -pi/6), and shear.
test_rs_scalar.py becomes test_rs_bandnodatavalue.py, and the RS_Rotation cases move to their own test_rs_rotation.py — the parity suite keeps one module per function.
RS_Rotation now computes acos(scaleX / hypot(scaleX, skewY)), negated when skewY > 0 — the angle of the grid's column-axis direction, Sedona Spark's formula. The previous atan2(-skewX, scaleX) mixed the row axis's skew into the answer: it agreed with Spark only while |skewX| == |skewY|, misreported a rigid 30-degree rotation of non-square 2x3 pixels as -0.7137 (true -pi/6, which the new formula recovers exactly), diverged under shear, and returned IEEE -0.0 for axis-aligned rasters where Spark returns +0.0 (acos also fixes that). Once RS_GeoTransform lands its thetaI agrees with RS_Rotation by construction. The parity xfails flip to anchored passing tests: axis-aligned compares exactly including the zero's sign; rotated and sheared grids round to 12 digits since the raw doubles can differ by one ulp across runtimes.
james-willis
commented
Sep 2, 2026
| @@ -14,7 +14,9 @@ | |||
| # KIND, either express or implied. See the License for the | |||
Contributor
Author
There was a problem hiding this comment.
drivey by: clean up the parity harness to more comprehensively model 1 file for 1 function
james-willis
marked this pull request as ready for review
September 2, 2026 21:50
james-willis
requested review from
paleolimbot
and removed request for
prantogg
September 2, 2026 21:50
Neither runtime promises a bit-exact acos (Java Math.acos is specified to 1 ulp of correctly rounded; Rust f64::acos is the platform libm with no contract). Measured on macOS vs Temurin 17: 2 of 72 swept angles differ by exactly 1 ulp, pi/6 among them, and which angles are unlucky is libm-dependent — a raw comparison would be cross-platform flaky, not strict. The anchored constants sit thousands of ulps from a 12-digit rounding boundary, so ROUND collapses the 1-ulp pairs safely.
james-willis
commented
Sep 2, 2026
james-willis
deleted the
james/db-573-rs_rotation-diverges-from-sedona-spark-under-shear
branch
September 4, 2026 20:29
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.
RS_Rotationparity coverage that turned into a kernel fix, in three commits:atan2(-skewX, scaleX)where Sedona Spark computesacos(scaleX / sqrt(scaleX² + skewY²)), negated whenskewY > 0. They coincide only while|skewX| == |skewY|.test_rs_scalar.py→test_rs_bandnodatavalue.py,RS_Rotationcases intotest_rs_rotation.py(the suite's layout convention going forward).GeoTransformEx::rotationnow uses Spark's formula — the angle of the grid's actual column-axis direction(scaleX, skewY).Why align rather than document the divergence
RS_GeoTransform(feat(rust/sedona-raster-functions): add RS_GeoTransform composite accessor #1221) ports Spark's decomposition, so after this change itsthetaIagrees withRS_Rotationby construction — no internal inconsistency.acosincidentally fixes a zero-sign nit: the oldatan2returned IEEE-0.0for axis-aligned rasters where Spark returns+0.0.Blast radius:
GeoTransformEx::rotationfeeds only theRS_RotationUDF; rust unit tests ingeo_transform.rs,affine_transformation.rs, andrs_geotransform.rsare rewritten around derived cases (axis-aligned asserts the positive zero's sign; sheared expectation matches Spark bit-for-bit in probing).The parity tests (all passing, anchored)
0.0, sign included.round(-theta, 12).round(-acos(scaleX/hypot(scaleX, skewY)), 12).Rotated/sheared queries compare through
ROUND(..., 12), with the evidence in a comment above the tests: neither runtime promises a bit-exactacos(JavaMath.acosis specified to within 1 ulp of correctly rounded; Rustf64::acosis the platform libm with no accuracy contract), and a 72-angle sweep on macOS vs Temurin 17 measured 2 angles differing by exactly 1 ulp — π/6 among them — with the unlucky set being libm-dependent, so a raw comparison would be cross-platform flaky rather than strict. The anchored constants sit thousands of ulps from a 12-digit rounding boundary, soROUNDcollapses 1-ulp pairs without ever splitting them. Axis-aligned cases compare raw and exact.Verification
cargo test -p sedona-rasterand-p sedona-raster-functionsgreen locally;integration/spark-parityat28 passed, 14 xfailed(RS_Rotation contributes 7 anchored passes, no xfails). This branch carries the #1217 lane so CI runs the parity suite.Aside, noted while probing and out of scope: SedonaDB evaluates
-0.0 = 0.0asfalsein SQL where Spark saystrue.