From 3cc8b6f8fd3c36cbd662540c890ec815bd312b6d Mon Sep 17 00:00:00 2001 From: Mike McCandless Date: Mon, 29 Feb 2016 15:45:09 -0500 Subject: [PATCH] also test LatLonPoint.newDistanceQuery from TestLatLonPointQueries, and remove some test leniency --- .../lucene/search/TestLatLonPointQueries.java | 63 ++++++++++++++----- .../spatial/util/BaseGeoPointTestCase.java | 20 ++++-- 2 files changed, 64 insertions(+), 19 deletions(-) diff --git a/lucene/sandbox/src/test/org/apache/lucene/search/TestLatLonPointQueries.java b/lucene/sandbox/src/test/org/apache/lucene/search/TestLatLonPointQueries.java index 0916b15ba8d..7798b9769cb 100644 --- a/lucene/sandbox/src/test/org/apache/lucene/search/TestLatLonPointQueries.java +++ b/lucene/sandbox/src/test/org/apache/lucene/search/TestLatLonPointQueries.java @@ -39,9 +39,7 @@ public class TestLatLonPointQueries extends BaseGeoPointTestCase { @Override protected Query newDistanceQuery(String field, double centerLat, double centerLon, double radiusMeters) { - // TODO: fix this to be debuggable before enabling! - // return LatLonPoint.newDistanceQuery(field, centerLat, centerLon, radiusMeters); - return null; + return LatLonPoint.newDistanceQuery(field, centerLat, centerLon, radiusMeters); } @Override @@ -59,6 +57,53 @@ public class TestLatLonPointQueries extends BaseGeoPointTestCase { assert Double.isNaN(pointLat) == false; + int rectLatMinEnc = LatLonPoint.encodeLat(rect.minLat); + int rectLatMaxEnc = LatLonPoint.encodeLat(rect.maxLat); + int rectLonMinEnc = LatLonPoint.encodeLon(rect.minLon); + int rectLonMaxEnc = LatLonPoint.encodeLon(rect.maxLon); + + int pointLatEnc = LatLonPoint.encodeLat(pointLat); + int pointLonEnc = LatLonPoint.encodeLon(pointLon); + + if (rect.minLon < rect.maxLon) { + return pointLatEnc >= rectLatMinEnc && + pointLatEnc < rectLatMaxEnc && + pointLonEnc >= rectLonMinEnc && + pointLonEnc < rectLonMaxEnc; + } else { + // Rect crosses dateline: + return pointLatEnc >= rectLatMinEnc && + pointLatEnc < rectLatMaxEnc && + (pointLonEnc >= rectLonMinEnc || + pointLonEnc < rectLonMaxEnc); + } + } + + @Override + protected double quantizeLat(double latRaw) { + return LatLonPoint.decodeLat(LatLonPoint.encodeLat(latRaw)); + } + + @Override + protected double quantizeLon(double lonRaw) { + return LatLonPoint.decodeLon(LatLonPoint.encodeLon(lonRaw)); + } + + // todo reconcile with GeoUtils (see LUCENE-6996) + public static double compare(final double v1, final double v2) { + final double delta = v1-v2; + return Math.abs(delta) <= BKD_TOLERANCE ? 0 : delta; + } + + @Override + protected Boolean polyRectContainsPoint(GeoRect rect, double pointLat, double pointLon) { + // TODO write better random polygon tests + + assert Double.isNaN(pointLat) == false; + + // TODO: this comment is wrong! we have fixed the quantization error (we now pre-quantize all randomly generated test points) yet the test + // still fails if we remove this evil "return null": + // false positive/negatives due to quantization error exist for both rectangles and polygons if (compare(pointLat, rect.minLat) == 0 || compare(pointLat, rect.maxLat) == 0 @@ -89,18 +134,6 @@ public class TestLatLonPointQueries extends BaseGeoPointTestCase { } } - // todo reconcile with GeoUtils (see LUCENE-6996) - public static double compare(final double v1, final double v2) { - final double delta = v1-v2; - return Math.abs(delta) <= BKD_TOLERANCE ? 0 : delta; - } - - @Override - protected Boolean polyRectContainsPoint(GeoRect rect, double pointLat, double pointLon) { - // TODO write better random polygon tests - return rectContainsPoint(rect, pointLat, pointLon); - } - @Override protected Boolean circleContainsPoint(double centerLat, double centerLon, double radiusMeters, double pointLat, double pointLon) { double distanceMeters = GeoDistanceUtils.haversin(centerLat, centerLon, pointLat, pointLon); diff --git a/lucene/spatial/src/test/org/apache/lucene/spatial/util/BaseGeoPointTestCase.java b/lucene/spatial/src/test/org/apache/lucene/spatial/util/BaseGeoPointTestCase.java index 1980e9a28c2..8313616f3d2 100644 --- a/lucene/spatial/src/test/org/apache/lucene/spatial/util/BaseGeoPointTestCase.java +++ b/lucene/spatial/src/test/org/apache/lucene/spatial/util/BaseGeoPointTestCase.java @@ -422,7 +422,7 @@ public abstract class BaseGeoPointTestCase extends LuceneTestCase { } else { result = -90 + 180.0 * random().nextDouble(); } - return result; + return quantizeLat(result); } public double randomLon(boolean small) { @@ -432,7 +432,19 @@ public abstract class BaseGeoPointTestCase extends LuceneTestCase { } else { result = -180 + 360.0 * random().nextDouble(); } - return result; + return quantizeLon(result); + } + + /** Override this to quantize randomly generated lat, so the test won't fail due to quantization errors, which are 1) annoying to debug, + * and 2) should never affect "real" usage terribly. */ + protected double quantizeLat(double lat) { + return lat; + } + + /** Override this to quantize randomly generated lon, so the test won't fail due to quantization errors, which are 1) annoying to debug, + * and 2) should never affect "real" usage terribly. */ + protected double quantizeLon(double lon) { + return lon; } protected GeoRect randomRect(boolean small, boolean canCrossDateLine) { @@ -694,9 +706,9 @@ public abstract class BaseGeoPointTestCase extends LuceneTestCase { @Override protected void describe(int docID, double pointLat, double pointLon) { - double distanceKM = SloppyMath.haversin(centerLat, centerLon, pointLat, pointLon); + double distanceMeters = GeoDistanceUtils.haversin(centerLat, centerLon, pointLat, pointLon); System.out.println(" docID=" + docID + " centerLon=" + centerLon + " centerLat=" + centerLat - + " pointLon=" + pointLon + " pointLat=" + pointLat + " distanceMeters=" + (distanceKM * 1000) + + " pointLon=" + pointLon + " pointLat=" + pointLat + " distanceMeters=" + distanceMeters + " vs" + ((rangeQuery == true) ? " minRadiusMeters=" + minRadiusMeters : "") + " radiusMeters=" + radiusMeters); } };