From b21218e79f2b431b3311a0a07dfbcdb7ae48f1ab Mon Sep 17 00:00:00 2001 From: Robert Muir Date: Mon, 11 Apr 2016 10:36:10 -0400 Subject: [PATCH] LUCENE-7185: fix random number generation used for spatial tests. Note that GeoPoint tests are still on the old RNG as we haven't get made them happy. --- .../geopoint/search/GeoPointTestUtil.java | 287 ++++++++++++++++++ .../geopoint/search/TestGeoPointQuery.java | 42 +++ .../search/TestLegacyGeoPointQuery.java | 43 ++- .../spatial/util/BaseGeoPointTestCase.java | 79 +++-- .../org/apache/lucene/geo/GeoTestUtil.java | 46 ++- 5 files changed, 469 insertions(+), 28 deletions(-) create mode 100644 lucene/spatial/src/test/org/apache/lucene/spatial/geopoint/search/GeoPointTestUtil.java diff --git a/lucene/spatial/src/test/org/apache/lucene/spatial/geopoint/search/GeoPointTestUtil.java b/lucene/spatial/src/test/org/apache/lucene/spatial/geopoint/search/GeoPointTestUtil.java new file mode 100644 index 00000000000..a8c4271e42f --- /dev/null +++ b/lucene/spatial/src/test/org/apache/lucene/spatial/geopoint/search/GeoPointTestUtil.java @@ -0,0 +1,287 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.lucene.spatial.geopoint.search; + +import java.util.ArrayList; +import java.util.Random; + +import org.apache.lucene.geo.GeoUtils; +import org.apache.lucene.geo.Polygon; +import org.apache.lucene.geo.Rectangle; + +import com.carrotsearch.randomizedtesting.RandomizedContext; + +// HACK: this is a wimpier version of GeoTestUtil used for now until we can +// get all tests passing with new random number generator! + +final class GeoPointTestUtil { + + /** returns next pseudorandom latitude (anywhere) */ + public static double nextLatitude() { + return -90 + 180.0 * random().nextDouble(); + } + + /** returns next pseudorandom longitude (anywhere) */ + public static double nextLongitude() { + return -180 + 360.0 * random().nextDouble(); + } + + /** returns next pseudorandom latitude, kinda close to {@code otherLatitude} */ + public static double nextLatitudeNear(double otherLatitude) { + GeoUtils.checkLatitude(otherLatitude); + return normalizeLatitude(otherLatitude + random().nextDouble() - 0.5); + } + + /** returns next pseudorandom longitude, kinda close to {@code otherLongitude} */ + public static double nextLongitudeNear(double otherLongitude) { + GeoUtils.checkLongitude(otherLongitude); + return normalizeLongitude(otherLongitude + random().nextDouble() - 0.5); + } + + /** + * returns next pseudorandom latitude, kinda close to {@code minLatitude/maxLatitude} + * NOTE:minLatitude/maxLatitude are merely guidelines. the returned value is sometimes + * outside of that range! this is to facilitate edge testing. + */ + public static double nextLatitudeAround(double minLatitude, double maxLatitude) { + GeoUtils.checkLatitude(minLatitude); + GeoUtils.checkLatitude(maxLatitude); + return normalizeLatitude(randomRangeMaybeSlightlyOutside(minLatitude, maxLatitude)); + } + + /** + * returns next pseudorandom longitude, kinda close to {@code minLongitude/maxLongitude} + * NOTE:minLongitude/maxLongitude are merely guidelines. the returned value is sometimes + * outside of that range! this is to facilitate edge testing. + */ + public static double nextLongitudeAround(double minLongitude, double maxLongitude) { + GeoUtils.checkLongitude(minLongitude); + GeoUtils.checkLongitude(maxLongitude); + return normalizeLongitude(randomRangeMaybeSlightlyOutside(minLongitude, maxLongitude)); + } + + /** returns next pseudorandom box: can cross the 180th meridian */ + public static Rectangle nextBox() { + return nextBoxInternal(nextLatitude(), nextLatitude(), nextLongitude(), nextLongitude(), true); + } + + /** returns next pseudorandom box: will not cross the 180th meridian */ + public static Rectangle nextSimpleBox() { + return nextBoxInternal(nextLatitude(), nextLatitude(), nextLongitude(), nextLongitude(), false); + } + + /** returns next pseudorandom box, can cross the 180th meridian, kinda close to {@code otherLatitude} and {@code otherLongitude} */ + public static Rectangle nextBoxNear(double otherLatitude, double otherLongitude) { + GeoUtils.checkLongitude(otherLongitude); + GeoUtils.checkLongitude(otherLongitude); + return nextBoxInternal(nextLatitudeNear(otherLatitude), nextLatitudeNear(otherLatitude), + nextLongitudeNear(otherLongitude), nextLongitudeNear(otherLongitude), true); + } + + /** returns next pseudorandom box, will not cross the 180th meridian, kinda close to {@code otherLatitude} and {@code otherLongitude} */ + public static Rectangle nextSimpleBoxNear(double otherLatitude, double otherLongitude) { + GeoUtils.checkLongitude(otherLongitude); + GeoUtils.checkLongitude(otherLongitude); + return nextBoxInternal(nextLatitudeNear(otherLatitude), nextLatitudeNear(otherLatitude), + nextLongitudeNear(otherLongitude), nextLongitudeNear(otherLongitude), false); + } + + /** returns next pseudorandom polygon */ + public static Polygon nextPolygon() { + if (random().nextBoolean()) { + return surpriseMePolygon(null, null); + } + + Rectangle box = nextBoxInternal(nextLatitude(), nextLatitude(), nextLongitude(), nextLongitude(), false); + if (random().nextBoolean()) { + // box + return boxPolygon(box); + } else { + // triangle + return trianglePolygon(box); + } + } + + /** returns next pseudorandom polygon, kinda close to {@code otherLatitude} and {@code otherLongitude} */ + public static Polygon nextPolygonNear(double otherLatitude, double otherLongitude) { + if (random().nextBoolean()) { + return surpriseMePolygon(otherLatitude, otherLongitude); + } + + Rectangle box = nextBoxInternal(nextLatitudeNear(otherLatitude), nextLatitudeNear(otherLatitude), + nextLongitudeNear(otherLongitude), nextLongitudeNear(otherLongitude), false); + if (random().nextBoolean()) { + // box + return boxPolygon(box); + } else { + // triangle + return trianglePolygon(box); + } + } + + private static Rectangle nextBoxInternal(double lat0, double lat1, double lon0, double lon1, boolean canCrossDateLine) { + if (lat1 < lat0) { + double x = lat0; + lat0 = lat1; + lat1 = x; + } + + if (canCrossDateLine == false && lon1 < lon0) { + double x = lon0; + lon0 = lon1; + lon1 = x; + } + + return new Rectangle(lat0, lat1, lon0, lon1); + } + + private static Polygon boxPolygon(Rectangle box) { + assert box.crossesDateline() == false; + final double[] polyLats = new double[5]; + final double[] polyLons = new double[5]; + polyLats[0] = box.minLat; + polyLons[0] = box.minLon; + polyLats[1] = box.maxLat; + polyLons[1] = box.minLon; + polyLats[2] = box.maxLat; + polyLons[2] = box.maxLon; + polyLats[3] = box.minLat; + polyLons[3] = box.maxLon; + polyLats[4] = box.minLat; + polyLons[4] = box.minLon; + return new Polygon(polyLats, polyLons); + } + + private static Polygon trianglePolygon(Rectangle box) { + assert box.crossesDateline() == false; + final double[] polyLats = new double[4]; + final double[] polyLons = new double[4]; + polyLats[0] = box.minLat; + polyLons[0] = box.minLon; + polyLats[1] = box.maxLat; + polyLons[1] = box.minLon; + polyLats[2] = box.maxLat; + polyLons[2] = box.maxLon; + polyLats[3] = box.minLat; + polyLons[3] = box.minLon; + return new Polygon(polyLats, polyLons); + } + + private static Polygon surpriseMePolygon(Double otherLatitude, Double otherLongitude) { + // repeat until we get a poly that doesn't cross dateline: + newPoly: + while (true) { + //System.out.println("\nPOLY ITER"); + final double centerLat; + final double centerLon; + if (otherLatitude == null) { + centerLat = nextLatitude(); + centerLon = nextLongitude(); + } else { + GeoUtils.checkLatitude(otherLatitude); + GeoUtils.checkLongitude(otherLongitude); + centerLat = nextLatitudeNear(otherLatitude); + centerLon = nextLongitudeNear(otherLongitude); + } + + double radius = 0.1 + 20 * random().nextDouble(); + double radiusDelta = random().nextDouble(); + + ArrayList lats = new ArrayList<>(); + ArrayList lons = new ArrayList<>(); + double angle = 0.0; + while (true) { + angle += random().nextDouble()*40.0; + //System.out.println(" angle " + angle); + if (angle > 360) { + break; + } + double len = radius * (1.0 - radiusDelta + radiusDelta * random().nextDouble()); + //System.out.println(" len=" + len); + double lat = centerLat + len * Math.cos(Math.toRadians(angle)); + double lon = centerLon + len * Math.sin(Math.toRadians(angle)); + if (lon <= GeoUtils.MIN_LON_INCL || lon >= GeoUtils.MAX_LON_INCL) { + // cannot cross dateline: try again! + continue newPoly; + } + if (lat > 90) { + // cross the north pole + lat = 180 - lat; + lon = 180 - lon; + } else if (lat < -90) { + // cross the south pole + lat = -180 - lat; + lon = 180 - lon; + } + if (lon <= GeoUtils.MIN_LON_INCL || lon >= GeoUtils.MAX_LON_INCL) { + // cannot cross dateline: try again! + continue newPoly; + } + lats.add(lat); + lons.add(lon); + + //System.out.println(" lat=" + lats.get(lats.size()-1) + " lon=" + lons.get(lons.size()-1)); + } + + // close it + lats.add(lats.get(0)); + lons.add(lons.get(0)); + + double[] latsArray = new double[lats.size()]; + double[] lonsArray = new double[lons.size()]; + for(int i=0;i= -90 && latitude <= 90) { + return latitude; //common case, and avoids slight double precision shifting + } + double off = Math.abs((latitude + 90) % 360); + return (off <= 180 ? off : 360-off) - 90; + } + + /** Puts longitude in range of -180 to +180. */ + private static double normalizeLongitude(double longitude) { + if (longitude >= -180 && longitude <= 180) { + return longitude; //common case, and avoids slight double precision shifting + } + double off = (longitude + 180) % 360; + if (off < 0) { + return 180 + off; + } else if (off == 0 && longitude > 0) { + return 180; + } else { + return -180 + off; + } + } + + /** Keep it simple, we don't need to take arbitrary Random for geo tests */ + private static Random random() { + return RandomizedContext.current().getRandom(); + } +} diff --git a/lucene/spatial/src/test/org/apache/lucene/spatial/geopoint/search/TestGeoPointQuery.java b/lucene/spatial/src/test/org/apache/lucene/spatial/geopoint/search/TestGeoPointQuery.java index ff8baca95c8..7e64430be41 100644 --- a/lucene/spatial/src/test/org/apache/lucene/spatial/geopoint/search/TestGeoPointQuery.java +++ b/lucene/spatial/src/test/org/apache/lucene/spatial/geopoint/search/TestGeoPointQuery.java @@ -20,6 +20,7 @@ import org.apache.lucene.document.Document; import org.apache.lucene.search.Query; import org.apache.lucene.spatial.util.GeoEncodingUtils; import org.apache.lucene.geo.Polygon; +import org.apache.lucene.geo.Rectangle; import org.apache.lucene.spatial.geopoint.document.GeoPointField; import org.apache.lucene.spatial.geopoint.document.GeoPointField.TermEncoding; import org.apache.lucene.spatial.util.BaseGeoPointTestCase; @@ -61,4 +62,45 @@ public class TestGeoPointQuery extends BaseGeoPointTestCase { return new GeoPointInPolygonQuery(field, TermEncoding.PREFIX, polygons); } + // TODO: remove these once we get tests passing! + + @Override + protected double nextLongitude() { + return GeoPointTestUtil.nextLongitude(); + } + + @Override + protected double nextLongitudeNear(double other) { + return GeoPointTestUtil.nextLongitudeNear(other); + } + + @Override + protected double nextLatitude() { + return GeoPointTestUtil.nextLatitude(); + } + + @Override + protected double nextLatitudeNear(double other) { + return GeoPointTestUtil.nextLatitudeNear(other); + } + + @Override + protected Rectangle nextBox() { + return GeoPointTestUtil.nextBox(); + } + + @Override + protected Rectangle nextBoxNear(double latitude, double longitude) { + return GeoPointTestUtil.nextBoxNear(latitude, longitude); + } + + @Override + protected Polygon nextPolygon() { + return GeoPointTestUtil.nextPolygon(); + } + + @Override + protected Polygon nextPolygonNear(double latitude, double longitude) { + return GeoPointTestUtil.nextPolygonNear(latitude, longitude); + } } diff --git a/lucene/spatial/src/test/org/apache/lucene/spatial/geopoint/search/TestLegacyGeoPointQuery.java b/lucene/spatial/src/test/org/apache/lucene/spatial/geopoint/search/TestLegacyGeoPointQuery.java index 4c8e3e32de3..75cc377d0e5 100644 --- a/lucene/spatial/src/test/org/apache/lucene/spatial/geopoint/search/TestLegacyGeoPointQuery.java +++ b/lucene/spatial/src/test/org/apache/lucene/spatial/geopoint/search/TestLegacyGeoPointQuery.java @@ -20,6 +20,7 @@ import org.apache.lucene.document.Document; import org.apache.lucene.search.Query; import org.apache.lucene.spatial.util.GeoEncodingUtils; import org.apache.lucene.geo.Polygon; +import org.apache.lucene.geo.Rectangle; import org.apache.lucene.spatial.geopoint.document.GeoPointField; import org.apache.lucene.spatial.geopoint.document.GeoPointField.TermEncoding; import org.apache.lucene.spatial.util.BaseGeoPointTestCase; @@ -77,5 +78,45 @@ public class TestLegacyGeoPointQuery extends BaseGeoPointTestCase { assumeTrue("legacy encoding goes OOM on this test", false); } - + // TODO: remove these once we get tests passing! + + @Override + protected double nextLongitude() { + return GeoPointTestUtil.nextLongitude(); + } + + @Override + protected double nextLongitudeNear(double other) { + return GeoPointTestUtil.nextLongitudeNear(other); + } + + @Override + protected double nextLatitude() { + return GeoPointTestUtil.nextLatitude(); + } + + @Override + protected double nextLatitudeNear(double other) { + return GeoPointTestUtil.nextLatitudeNear(other); + } + + @Override + protected Rectangle nextBox() { + return GeoPointTestUtil.nextBox(); + } + + @Override + protected Rectangle nextBoxNear(double latitude, double longitude) { + return GeoPointTestUtil.nextBoxNear(latitude, longitude); + } + + @Override + protected Polygon nextPolygon() { + return GeoPointTestUtil.nextPolygon(); + } + + @Override + protected Polygon nextPolygonNear(double latitude, double longitude) { + return GeoPointTestUtil.nextPolygonNear(latitude, longitude); + } } 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 adca4666faf..c2536bd47a3 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 @@ -38,7 +38,6 @@ import org.apache.lucene.document.NumericDocValuesField; import org.apache.lucene.document.StoredField; import org.apache.lucene.document.StringField; import org.apache.lucene.geo.Rectangle; -import org.apache.lucene.geo.GeoTestUtil; import org.apache.lucene.geo.GeoUtils; import org.apache.lucene.geo.Polygon; import org.apache.lucene.index.DirectoryReader; @@ -68,7 +67,6 @@ import org.apache.lucene.util.LuceneTestCase; import org.apache.lucene.util.SloppyMath; import org.apache.lucene.util.TestUtil; import org.apache.lucene.util.bkd.BKDWriter; -import org.junit.BeforeClass; /** * Abstract class to do basic tests for a geospatial impl (high level @@ -85,13 +83,48 @@ public abstract class BaseGeoPointTestCase extends LuceneTestCase { protected static final String FIELD_NAME = "point"; - private static double originLat; - private static double originLon; + private double originLat; + private double originLon; - @BeforeClass - public static void beforeClassBase() throws Exception { - originLon = GeoTestUtil.nextLongitude(); - originLat = GeoTestUtil.nextLatitude(); + @Override + public void setUp() throws Exception { + super.setUp(); + originLon = nextLongitude(); + originLat = nextLatitude(); + } + + // TODO: remove these hooks once all subclasses can pass with new random! + + protected double nextLongitude() { + return org.apache.lucene.geo.GeoTestUtil.nextLongitude(); + } + + protected double nextLongitudeNear(double other) { + return org.apache.lucene.geo.GeoTestUtil.nextLongitudeNear(other); + } + + protected double nextLatitude() { + return org.apache.lucene.geo.GeoTestUtil.nextLatitude(); + } + + protected double nextLatitudeNear(double other) { + return org.apache.lucene.geo.GeoTestUtil.nextLatitudeNear(other); + } + + protected Rectangle nextBox() { + return org.apache.lucene.geo.GeoTestUtil.nextBox(); + } + + protected Rectangle nextBoxNear(double latitude, double longitude) { + return org.apache.lucene.geo.GeoTestUtil.nextBoxNear(latitude, longitude); + } + + protected Polygon nextPolygon() { + return org.apache.lucene.geo.GeoTestUtil.nextPolygon(); + } + + protected Polygon nextPolygonNear(double latitude, double longitude) { + return org.apache.lucene.geo.GeoTestUtil.nextPolygonNear(latitude, longitude); } /** Valid values that should not cause exception */ @@ -691,19 +724,19 @@ public abstract class BaseGeoPointTestCase extends LuceneTestCase { verify(small, lats, lons); } - public double randomLat(boolean small) { + public final double randomLat(boolean small) { if (small) { - return GeoTestUtil.nextLatitudeNear(originLat); + return nextLatitudeNear(originLat); } else { - return GeoTestUtil.nextLatitude(); + return nextLatitude(); } } - public double randomLon(boolean small) { + public final double randomLon(boolean small) { if (small) { - return GeoTestUtil.nextLongitudeNear(originLon); + return nextLongitudeNear(originLon); } else { - return GeoTestUtil.nextLongitude(); + return nextLongitude(); } } @@ -719,11 +752,11 @@ public abstract class BaseGeoPointTestCase extends LuceneTestCase { return lon; } - protected Rectangle randomRect(boolean small) { + protected final Rectangle randomRect(boolean small) { if (small) { - return GeoTestUtil.nextBoxNear(originLat, originLon); + return nextBoxNear(originLat, originLon); } else { - return GeoTestUtil.nextBox(); + return nextBox(); } } @@ -1102,9 +1135,9 @@ public abstract class BaseGeoPointTestCase extends LuceneTestCase { // Polygon final Polygon polygon; if (small) { - polygon = GeoTestUtil.nextPolygonNear(originLat, originLon); + polygon = nextPolygonNear(originLat, originLon); } else { - polygon = GeoTestUtil.nextPolygon(); + polygon = nextPolygon(); } Query query = newPolygonQuery(FIELD_NAME, polygon); @@ -1291,8 +1324,8 @@ public abstract class BaseGeoPointTestCase extends LuceneTestCase { RandomIndexWriter writer = new RandomIndexWriter(random(), dir, iwc); for (int i = 0; i < numDocs; i++) { - double latRaw = GeoTestUtil.nextLatitude(); - double lonRaw = GeoTestUtil.nextLongitude(); + double latRaw = nextLatitude(); + double lonRaw = nextLongitude(); // pre-normalize up front, so we can just use quantized value for testing and do simple exact comparisons double lat = quantizeLat(latRaw); double lon = quantizeLon(lonRaw); @@ -1306,8 +1339,8 @@ public abstract class BaseGeoPointTestCase extends LuceneTestCase { IndexSearcher searcher = newSearcher(reader); for (int i = 0; i < numQueries; i++) { - double lat = GeoTestUtil.nextLatitude(); - double lon = GeoTestUtil.nextLongitude(); + double lat = nextLatitude(); + double lon = nextLongitude(); double radius = 50000000D * random().nextDouble(); BitSet expected = new BitSet(); diff --git a/lucene/test-framework/src/java/org/apache/lucene/geo/GeoTestUtil.java b/lucene/test-framework/src/java/org/apache/lucene/geo/GeoTestUtil.java index 23e34164668..9e719c09bb7 100644 --- a/lucene/test-framework/src/java/org/apache/lucene/geo/GeoTestUtil.java +++ b/lucene/test-framework/src/java/org/apache/lucene/geo/GeoTestUtil.java @@ -17,24 +17,62 @@ package org.apache.lucene.geo; import java.util.ArrayList; -import java.util.List; import java.util.Random; -import org.apache.lucene.util.SloppyMath; +import org.apache.lucene.util.NumericUtils; +import org.apache.lucene.util.TestUtil; import com.carrotsearch.randomizedtesting.RandomizedContext; /** static methods for testing geo */ public class GeoTestUtil { + private static final long LATITUDE_MIN_SORTABLE = NumericUtils.doubleToSortableLong(-90); + private static final long LATITUDE_MAX_SORTABLE = NumericUtils.doubleToSortableLong(90); + /** returns next pseudorandom latitude (anywhere) */ public static double nextLatitude() { - return -90 + 180.0 * random().nextDouble(); + int surpriseMe = random().nextInt(17); + if (surpriseMe == 0) { + // random bitpattern in range + return NumericUtils.sortableLongToDouble(TestUtil.nextLong(random(), LATITUDE_MIN_SORTABLE, LATITUDE_MAX_SORTABLE)); + } else if (surpriseMe == 1) { + // edge case + return -90.0; + } else if (surpriseMe == 2) { + // edge case + return 90.0; + } else if (surpriseMe == 3) { + // may trigger divide by zero + return 0.0; + } else { + // distributed ~ evenly + return -90 + 180.0 * random().nextDouble(); + } } + private static final long LONGITUDE_MIN_SORTABLE = NumericUtils.doubleToSortableLong(-180); + private static final long LONGITUDE_MAX_SORTABLE = NumericUtils.doubleToSortableLong(180); + /** returns next pseudorandom longitude (anywhere) */ public static double nextLongitude() { - return -180 + 360.0 * random().nextDouble(); + int surpriseMe = random().nextInt(17); + if (surpriseMe == 0) { + // random bitpattern in range + return NumericUtils.sortableLongToDouble(TestUtil.nextLong(random(), LONGITUDE_MIN_SORTABLE, LONGITUDE_MAX_SORTABLE)); + } else if (surpriseMe == 1) { + // edge case + return -180.0; + } else if (surpriseMe == 2) { + // edge case + return 180.0; + } else if (surpriseMe == 3) { + // may trigger divide by 0 + return 0.0; + } else { + // distributed ~ evenly + return -180 + 360.0 * random().nextDouble(); + } } /** returns next pseudorandom latitude, kinda close to {@code otherLatitude} */