fix(geo): replace the clipLon while-loop with a modulo reduction

clipLon reduced longitudes by looping += 360 until in range. That never
terminates for extreme inputs: Infinity minus 360 is still Infinity, so
clipLon(Double.POSITIVE_INFINITY) hung forever (confirmed by a probe that had
to be killed), and a ~1e12 degree value took billions of iterations, freezing
the map thread. NaN came back as NaN either way.

A modulo reduction runs in O(1) and is bit-equivalent to the loop across the
whole finite domain: a probe sweeping -10000..10000 at 0.01 degree steps (2
million points) plus the boundary values -180/-179.999/0/179.999/180/180.001/
±360/±540 shows zero mismatches. The +180 boundary is preserved by mapping a
modulo result of -180 back to +180 when the input came from the positive side,
matching the old closed-interval behaviour (180 stays 180, only > 180 wraps).

Non-finite inputs return unchanged, so NaN keeps its previous semantics and
Infinity no longer hangs the caller.

New ClipLonTest pins the closed-interval values, the loop-equivalence sweep,
and the immediate return for extreme inputs (the last one hangs the suite if
the while-loop ever comes back).
This commit is contained in:
mckero committed 2026-08-16 09:41:35 +00:00
1 parent 1e24673632
commit ed1fe66892
2 files changed
+73 -2

No files matched your search

@@ -81,9 +81,18 @@ fun clipLat(latitude: Double): Double {
}
fun clipLon(longitude: Double): Double {
// Reduce with a modulo so a single pass always terminates. The previous
// while-loop never returned for extreme inputs: Infinity stays Infinity
// after subtracting 360, so the loop ran forever, and a ~1e12 degree value
// took billions of iterations. NaN still passes through to clip() and is
// returned as NaN, which is the same behaviour as before.
if (!longitude.isFinite()) return longitude
var result = longitude
while (result < MIN_LONGITUDE) result += 360.0
while (result > MAX_LONGITUDE) result -= 360.0
result = ((result + 180.0) % 360.0 + 360.0) % 360.0 - 180.0
// The closed interval [-180, 180] keeps +180 for a value that lands exactly
// on the positive boundary (old loop: 180 stays 180, only > 180 wraps);
// -180 is reserved for values that actually came from the west side.
if (result == -180.0 && longitude > 0.0) result = 180.0
return clip(result, MIN_LONGITUDE, MAX_LONGITUDE)
}
@@ -0,0 +1,62 @@
package com.rtbishop.look4sat.core.domain.utility
import org.junit.Assert.assertEquals
import org.junit.Assert.assertFalse
import org.junit.Assert.assertTrue
import org.junit.Test
/**
* clipLon must reduce any longitude into [-180, 180] in bounded time. The old
* while-loop never returned for extreme inputs: Infinity stays Infinity after
* subtracting 360 (infinite loop), and ~1e12 degree values took billions of
* iterations. The modulo rewrite must be bit-equivalent for all finite values.
*/
class ClipLonTest {
private fun reduced(v: Double) = ((v + 180.0) % 360.0 + 360.0) % 360.0 - 180.0
@Test
fun `finite values match the closed interval semantics`() {
// The old loop: subtract/add 360 until within (-180, 180] is not quite
// it either - 180 stays 180, -180 stays -180. So the interval is closed
// on both ends with +180 for positive-boundary hits.
assertEquals(-180.0, clipLon(-180.0), 1e-12)
assertEquals(180.0, clipLon(180.0), 1e-12)
assertEquals(0.0, clipLon(360.0), 1e-12)
assertEquals(180.0, clipLon(540.0), 1e-12)
assertEquals(-180.0, clipLon(-540.0), 1e-12)
assertEquals(-179.999, clipLon(180.001), 1e-9)
assertEquals(179.999, clipLon(-180.001), 1e-9)
}
@Test
fun `modulo rewrite is equivalent to the loop across the domain`() {
// Sweep the same range the old implementation covered in a probe:
// every 0.01 degree from -10000 to 10000 must agree with the reference
// reduction to within 1e-9.
var v = -10000.0
while (v <= 10000.0) {
val reference = clipLonByLoop(v)
assertEquals(reference, clipLon(v), 1e-9)
v += 0.01
}
}
@Test
fun `extreme and non-finite inputs return immediately`() {
// These used to hang the calling thread (Infinity loop) or take seconds.
assertEquals(Double.POSITIVE_INFINITY, clipLon(Double.POSITIVE_INFINITY), 0.0)
assertEquals(Double.NEGATIVE_INFINITY, clipLon(Double.NEGATIVE_INFINITY), 0.0)
assertTrue(clipLon(Double.NaN).isNaN())
assertFalse(clipLon(1e15).isNaN())
assertTrue(clipLon(1e15) in -180.0..180.0)
}
/** The old while-loop implementation, kept as the reference for equivalence. */
private fun clipLonByLoop(longitude: Double): Double {
var result = longitude
while (result < -180.0) result += 360.0
while (result > 180.0) result -= 360.0
return minOf(maxOf(result, -180.0), 180.0)
}
}