From 370f2d3488f11a70b06453b43ffd779e2844e1e2 Mon Sep 17 00:00:00 2001
From: Robert Muir <rmuir@apache.org>
Date: Wed, 6 Apr 2016 14:17:34 -0400
Subject: [PATCH] don't test LatLonPoint's quantization with epsilons: demand
 exact answers

---
 .../apache/lucene/document/LatLonPoint.java   |   9 +-
 .../lucene/document/TestLatLonPoint.java      | 267 +++++++-----------
 2 files changed, 107 insertions(+), 169 deletions(-)

diff --git a/lucene/sandbox/src/java/org/apache/lucene/document/LatLonPoint.java b/lucene/sandbox/src/java/org/apache/lucene/document/LatLonPoint.java
index 0d8a7b2b774..107d83cb877 100644
--- a/lucene/sandbox/src/java/org/apache/lucene/document/LatLonPoint.java
+++ b/lucene/sandbox/src/java/org/apache/lucene/document/LatLonPoint.java
@@ -46,8 +46,9 @@ import org.apache.lucene.geo.Polygon;
  *   <li>{@link #newPolygonQuery newPolygonQuery()} for matching points within an arbitrary polygon.
  * </ul>
  * <p>
- * <b>WARNING</b>: Values are indexed with some loss of precision, incurring up to 1E-7 error from the
- * original {@code double} values. 
+ * <b>WARNING</b>: Values are indexed with some loss of precision from the
+ * original {@code double} values (4.190951585769653E-8 for the latitude component
+ * and 8.381903171539307E-8 for longitude).
  * @see PointValues
  */
 // TODO ^^^ that is very sandy and hurts the API, usage, and tests tremendously, because what the user passes
@@ -98,9 +99,9 @@ public class LatLonPoint extends Field {
 
   private static final int BITS = 32;
   private static final double LONGITUDE_MUL = (0x1L<<BITS)/360.0D;
-  private static final double LONGITUDE_DECODE = 1/LONGITUDE_MUL;
+  static final double LONGITUDE_DECODE = 1/LONGITUDE_MUL;
   private static final double LATITUDE_MUL  = (0x1L<<BITS)/180.0D;
-  private static final double LATITUDE_DECODE  = 1/LATITUDE_MUL;
+  static final double LATITUDE_DECODE  = 1/LATITUDE_MUL;
   
   @Override
   public String toString() {
diff --git a/lucene/sandbox/src/test/org/apache/lucene/document/TestLatLonPoint.java b/lucene/sandbox/src/test/org/apache/lucene/document/TestLatLonPoint.java
index 6d50f068d32..fb8e0dc6589 100644
--- a/lucene/sandbox/src/test/org/apache/lucene/document/TestLatLonPoint.java
+++ b/lucene/sandbox/src/test/org/apache/lucene/document/TestLatLonPoint.java
@@ -16,8 +16,11 @@
  */
 package org.apache.lucene.document;
 
-import org.apache.lucene.geo.GeoTestUtil;
+import java.util.Random;
+
 import org.apache.lucene.util.LuceneTestCase;
+import org.apache.lucene.util.NumericUtils;
+import org.apache.lucene.util.TestUtil;
 
 /** Simple tests for {@link LatLonPoint} */
 public class TestLatLonPoint extends LuceneTestCase {
@@ -35,179 +38,113 @@ public class TestLatLonPoint extends LuceneTestCase {
     // sort field
     assertEquals("<distance:\"field\" latitude=18.0 longitude=19.0>", LatLonPoint.newDistanceSort("field", 18.0, 19.0).toString());
   }
-   
-  public void testEncodeDecode() throws Exception {
-    // just for testing quantization error
-    final double ENCODING_TOLERANCE = 1e-7;
 
-    int iters = atLeast(10000);
-    for(int iter=0;iter<iters;iter++) {
-      double lat = GeoTestUtil.nextLatitude();
-      double latEnc = LatLonPoint.decodeLatitude(LatLonPoint.encodeLatitude(lat));
-      assertEquals("lat=" + lat + " latEnc=" + latEnc + " diff=" + (lat - latEnc), lat, latEnc, ENCODING_TOLERANCE);
+  /**
+   * step through some integers, ensuring they decode to their expected double values.
+   * double values start at -90 and increase by LATITUDE_DECODE for each integer.
+   * check edge cases within the double range and random doubles within the range too.
+   */
+  public void testLatitudeQuantization() throws Exception {
+    Random random = random();
+    for (int i = 0; i < 10000; i++) {
+      int encoded = random.nextInt();
+      double min = -90.0 + (encoded - (long)Integer.MIN_VALUE) * LatLonPoint.LATITUDE_DECODE;
+      double decoded = LatLonPoint.decodeLatitude(encoded);
+      // should exactly equal expected value
+      assertEquals(min, decoded, 0.0D);
+      // should round-trip
+      assertEquals(encoded, LatLonPoint.encodeLatitude(decoded));
+      assertEquals(encoded, LatLonPoint.encodeLatitudeCeil(decoded));
+      // test within the range
+      if (i != Integer.MAX_VALUE) {
+        // this is the next representable value
+        // all double values between [min .. max) should encode to the current integer
+        // all double values between (min .. max] should encodeCeil to the next integer.
+        double max = min + LatLonPoint.LATITUDE_DECODE;
+        assertEquals(max, LatLonPoint.decodeLatitude(encoded+1), 0.0D);
+        assertEquals(encoded+1, LatLonPoint.encodeLatitude(max));
+        assertEquals(encoded+1, LatLonPoint.encodeLatitudeCeil(max));
 
-      double lon = GeoTestUtil.nextLongitude();
-      double lonEnc = LatLonPoint.decodeLongitude(LatLonPoint.encodeLongitude(lon));
-      assertEquals("lon=" + lon + " lonEnc=" + lonEnc + " diff=" + (lon - lonEnc), lon, lonEnc, ENCODING_TOLERANCE);
+        // first and last doubles in range that will be quantized
+        double minEdge = Math.nextUp(min);
+        double maxEdge = Math.nextDown(max);
+        assertEquals(encoded,   LatLonPoint.encodeLatitude(minEdge));
+        assertEquals(encoded+1, LatLonPoint.encodeLatitudeCeil(minEdge));
+        assertEquals(encoded,   LatLonPoint.encodeLatitude(maxEdge));
+        assertEquals(encoded+1, LatLonPoint.encodeLatitudeCeil(maxEdge));
+        
+        // check random values within the double range
+        long minBits = NumericUtils.doubleToSortableLong(minEdge);
+        long maxBits = NumericUtils.doubleToSortableLong(maxEdge);
+        for (int j = 0; j < 100; j++) {
+          double value = NumericUtils.sortableLongToDouble(TestUtil.nextLong(random, minBits, maxBits));
+          // round down
+          assertEquals(encoded,   LatLonPoint.encodeLatitude(value));
+          // round up
+          assertEquals(encoded+1, LatLonPoint.encodeLatitudeCeil(value));
+        }
+      }
     }
-
-    // check edge/interesting cases explicitly
-    assertEquals(0.0, LatLonPoint.decodeLatitude(LatLonPoint.encodeLatitude(0.0)), ENCODING_TOLERANCE);
-    assertEquals(90.0, LatLonPoint.decodeLatitude(LatLonPoint.encodeLatitude(90.0)), ENCODING_TOLERANCE);
-    assertEquals(-90.0, LatLonPoint.decodeLatitude(LatLonPoint.encodeLatitude(-90.0)), ENCODING_TOLERANCE);
-
-    assertEquals(0.0, LatLonPoint.decodeLongitude(LatLonPoint.encodeLongitude(0.0)), ENCODING_TOLERANCE);
-    assertEquals(180.0, LatLonPoint.decodeLongitude(LatLonPoint.encodeLongitude(180.0)), ENCODING_TOLERANCE);
-    assertEquals(-180.0, LatLonPoint.decodeLongitude(LatLonPoint.encodeLongitude(-180.0)), ENCODING_TOLERANCE);
-  }
-  
-  public void testEncodeDecodeCeil() throws Exception {
-    // just for testing quantization error
-    final double ENCODING_TOLERANCE = 1e-7;
-
-    int iters = atLeast(10000);
-    for(int iter=0;iter<iters;iter++) {
-      double lat = GeoTestUtil.nextLatitude();
-      double latEnc = LatLonPoint.decodeLatitude(LatLonPoint.encodeLatitudeCeil(lat));
-      assertEquals("lat=" + lat + " latEnc=" + latEnc + " diff=" + (lat - latEnc), lat, latEnc, ENCODING_TOLERANCE);
-
-      double lon = GeoTestUtil.nextLongitude();
-      double lonEnc = LatLonPoint.decodeLongitude(LatLonPoint.encodeLongitudeCeil(lon));
-      assertEquals("lon=" + lon + " lonEnc=" + lonEnc + " diff=" + (lon - lonEnc), lon, lonEnc, ENCODING_TOLERANCE);
-    }
-
-    // check edge/interesting cases explicitly
-    assertEquals(0.0, LatLonPoint.decodeLatitude(LatLonPoint.encodeLatitudeCeil(0.0)), ENCODING_TOLERANCE);
-    assertEquals(90.0, LatLonPoint.decodeLatitude(LatLonPoint.encodeLatitudeCeil(90.0)), ENCODING_TOLERANCE);
-    assertEquals(-90.0, LatLonPoint.decodeLatitude(LatLonPoint.encodeLatitudeCeil(-90.0)), ENCODING_TOLERANCE);
-
-    assertEquals(0.0, LatLonPoint.decodeLongitude(LatLonPoint.encodeLongitudeCeil(0.0)), ENCODING_TOLERANCE);
-    assertEquals(180.0, LatLonPoint.decodeLongitude(LatLonPoint.encodeLongitudeCeil(180.0)), ENCODING_TOLERANCE);
-    assertEquals(-180.0, LatLonPoint.decodeLongitude(LatLonPoint.encodeLongitudeCeil(-180.0)), ENCODING_TOLERANCE);
   }
 
-  public void testEncodeDecodeExtremeValues() throws Exception {
+  /** 
+   * step through some integers, ensuring they decode to their expected double values.
+   * double values start at -180 and increase by LONGITUDE_DECODE for each integer.
+   * check edge cases within the double range and a random doubles within the range too.
+   */
+  public void testLongitudeQuantization() throws Exception {
+    Random random = random();
+    for (int i = 0; i < 10000; i++) {
+      int encoded = random.nextInt();
+      double min = -180.0 + (encoded - (long)Integer.MIN_VALUE) * LatLonPoint.LONGITUDE_DECODE;
+      double decoded = LatLonPoint.decodeLongitude(encoded);
+      // should exactly equal expected value
+      assertEquals(min, decoded, 0.0D);
+      // should round-trip
+      assertEquals(encoded, LatLonPoint.encodeLongitude(decoded));
+      assertEquals(encoded, LatLonPoint.encodeLongitudeCeil(decoded));
+      // test within the range
+      if (i != Integer.MAX_VALUE) {
+        // this is the next representable value
+        // all double values between [min .. max) should encode to the current integer
+        // all double values between (min .. max] should encodeCeil to the next integer.
+        double max = min + LatLonPoint.LONGITUDE_DECODE;
+        assertEquals(max, LatLonPoint.decodeLongitude(encoded+1), 0.0D);
+        assertEquals(encoded+1, LatLonPoint.encodeLongitude(max));
+        assertEquals(encoded+1, LatLonPoint.encodeLongitudeCeil(max));
+
+        // first and last doubles in range that will be quantized
+        double minEdge = Math.nextUp(min);
+        double maxEdge = Math.nextDown(max);
+        assertEquals(encoded,   LatLonPoint.encodeLongitude(minEdge));
+        assertEquals(encoded+1, LatLonPoint.encodeLongitudeCeil(minEdge));
+        assertEquals(encoded,   LatLonPoint.encodeLongitude(maxEdge));
+        assertEquals(encoded+1, LatLonPoint.encodeLongitudeCeil(maxEdge));
+        
+        // check random values within the double range
+        long minBits = NumericUtils.doubleToSortableLong(minEdge);
+        long maxBits = NumericUtils.doubleToSortableLong(maxEdge);
+        for (int j = 0; j < 100; j++) {
+          double value = NumericUtils.sortableLongToDouble(TestUtil.nextLong(random, minBits, maxBits));
+          // round down
+          assertEquals(encoded,   LatLonPoint.encodeLongitude(value));
+          // round up
+          assertEquals(encoded+1, LatLonPoint.encodeLongitudeCeil(value));
+        }
+      }
+    }
+  }
+
+  // check edge/interesting cases explicitly
+  public void testEncodeEdgeCases() {
     assertEquals(Integer.MIN_VALUE, LatLonPoint.encodeLatitude(-90.0));
-    assertEquals(0, LatLonPoint.encodeLatitude(0.0));
-    assertEquals(Integer.MAX_VALUE, LatLonPoint.encodeLatitude(90.0));
-
-    assertEquals(Integer.MIN_VALUE, LatLonPoint.encodeLongitude(-180.0));
-    assertEquals(0, LatLonPoint.encodeLatitude(0.0));
-    assertEquals(Integer.MAX_VALUE, LatLonPoint.encodeLongitude(180.0));
-  }
-  
-  public void testEncodeDecodeExtremeValuesCeil() throws Exception {
     assertEquals(Integer.MIN_VALUE, LatLonPoint.encodeLatitudeCeil(-90.0));
-    assertEquals(0, LatLonPoint.encodeLatitudeCeil(0.0));
+    assertEquals(Integer.MAX_VALUE, LatLonPoint.encodeLatitude(90.0));
     assertEquals(Integer.MAX_VALUE, LatLonPoint.encodeLatitudeCeil(90.0));
-
+    
+    assertEquals(Integer.MIN_VALUE, LatLonPoint.encodeLongitude(-180.0));
     assertEquals(Integer.MIN_VALUE, LatLonPoint.encodeLongitudeCeil(-180.0));
-    assertEquals(0, LatLonPoint.encodeLatitudeCeil(0.0));
+    assertEquals(Integer.MAX_VALUE, LatLonPoint.encodeLongitude(180.0));
     assertEquals(Integer.MAX_VALUE, LatLonPoint.encodeLongitudeCeil(180.0));
   }
-
-  public void testEncodeDecodeIsStable() throws Exception {
-    int iters = atLeast(1000);
-    for(int iter=0;iter<iters;iter++) {
-      double lat = GeoTestUtil.nextLatitude();
-      double lon = GeoTestUtil.nextLongitude();
-
-      double latEnc = LatLonPoint.decodeLatitude(LatLonPoint.encodeLatitude(lat));
-      double lonEnc = LatLonPoint.decodeLongitude(LatLonPoint.encodeLongitude(lon));
-
-      double latEnc2 = LatLonPoint.decodeLatitude(LatLonPoint.encodeLatitude(latEnc));
-      double lonEnc2 = LatLonPoint.decodeLongitude(LatLonPoint.encodeLongitude(lonEnc));
-      assertEquals(latEnc, latEnc2, 0.0);
-      assertEquals(lonEnc, lonEnc2, 0.0);
-    }
-  }
-  
-  public void testEncodeDecodeCeilIsStable() throws Exception {
-    int iters = atLeast(1000);
-    for(int iter=0;iter<iters;iter++) {
-      double lat = GeoTestUtil.nextLatitude();
-      double lon = GeoTestUtil.nextLongitude();
-
-      double latEnc = LatLonPoint.decodeLatitude(LatLonPoint.encodeLatitudeCeil(lat));
-      double lonEnc = LatLonPoint.decodeLongitude(LatLonPoint.encodeLongitudeCeil(lon));
-
-      double latEnc2 = LatLonPoint.decodeLatitude(LatLonPoint.encodeLatitudeCeil(latEnc));
-      double lonEnc2 = LatLonPoint.decodeLongitude(LatLonPoint.encodeLongitudeCeil(lonEnc));
-      assertEquals(latEnc, latEnc2, 0.0);
-      assertEquals(lonEnc, lonEnc2, 0.0);
-    }
-  }
-  
-  /** make sure values always go down: this is important for edge case consistency */
-  public void testEncodeDecodeRoundsDown() throws Exception {
-    int iters = atLeast(10000);
-    for(int iter=0;iter<iters;iter++) {
-      final double latBase = GeoTestUtil.nextLatitude();
-      final double lonBase = GeoTestUtil.nextLongitude();
-
-      // test above the value
-      double lat = latBase;
-      double lon = lonBase;
-      for (int i = 0; i < 1000; i++) {
-        lat = Math.min(90, Math.nextUp(lat));
-        lon = Math.min(180, Math.nextUp(lon));
-        double latEnc = LatLonPoint.decodeLatitude(LatLonPoint.encodeLatitude(lat));
-        double lonEnc = LatLonPoint.decodeLongitude(LatLonPoint.encodeLongitude(lon));
-        assertTrue(latEnc <= lat);
-        assertTrue(lonEnc <= lon);
-      }
-
-      // test below the value
-      lat = latBase;
-      lon = lonBase;
-      for (int i = 0; i < 1000; i++) {
-        lat = Math.max(-90, Math.nextDown(lat));
-        lon = Math.max(-180, Math.nextDown(lon));
-        double latEnc = LatLonPoint.decodeLatitude(LatLonPoint.encodeLatitude(lat));
-        double lonEnc = LatLonPoint.decodeLongitude(LatLonPoint.encodeLongitude(lon));
-        assertTrue(latEnc <= lat);
-        assertTrue(lonEnc <= lon);
-      }
-    }
-  }
-
-  /** bug in previous encoding! */
-  public void testSpecialBuggyValue() throws Exception {
-    double special = 124.40717171877621;
-    double lonEnc = LatLonPoint.decodeLongitude(LatLonPoint.encodeLongitude(special));
-    assertTrue(lonEnc <= special);
-  }
-  
-  /** make sure values can go up if we need */
-  public void testEncodeDecodeCeilRoundsUp() throws Exception {
-    int iters = atLeast(10000);
-    for(int iter=0;iter<iters;iter++) {
-      final double latBase = GeoTestUtil.nextLatitude();
-      final double lonBase = GeoTestUtil.nextLongitude();
-
-      // test above the value
-      double lat = latBase;
-      double lon = lonBase;
-      for (int i = 0; i < 1000; i++) {
-        lat = Math.min(90, Math.nextUp(lat));
-        lon = Math.min(180, Math.nextUp(lon));
-        double latEnc = LatLonPoint.decodeLatitude(LatLonPoint.encodeLatitudeCeil(lat));
-        double lonEnc = LatLonPoint.decodeLongitude(LatLonPoint.encodeLongitudeCeil(lon));
-        assertTrue(latEnc >= lat);
-        assertTrue(lonEnc >= lon);
-      }
-
-      // test below the value
-      lat = latBase;
-      lon = lonBase;
-      for (int i = 0; i < 1000; i++) {
-        lat = Math.max(-90, Math.nextDown(lat));
-        lon = Math.max(-180, Math.nextDown(lon));
-        double latEnc = LatLonPoint.decodeLatitude(LatLonPoint.encodeLatitudeCeil(lat));
-        double lonEnc = LatLonPoint.decodeLongitude(LatLonPoint.encodeLongitudeCeil(lon));
-        assertTrue(latEnc >= lat);
-        assertTrue(lonEnc >= lon);
-      }
-    }
-  }
 }