You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by mi...@apache.org on 2015/05/26 22:07:13 UTC

svn commit: r1681836 - in /lucene/dev/branches/LUCENE-6481/lucene/sandbox/src: java/org/apache/lucene/search/GeoPointInBBoxQuery.java java/org/apache/lucene/search/GeoPointInPolygonQuery.java test/org/apache/lucene/search/TestGeoPointQuery.java

Author: mikemccand
Date: Tue May 26 20:07:12 2015
New Revision: 1681836

URL: http://svn.apache.org/r1681836
Log:
LUCENE-6481: fix some ant precommit issues; improve test (use threads; randomly test poly query too), but currently disabled; add validation of the incoming lat/lon to poly query; improve query toStrings a bit

Modified:
    lucene/dev/branches/LUCENE-6481/lucene/sandbox/src/java/org/apache/lucene/search/GeoPointInBBoxQuery.java
    lucene/dev/branches/LUCENE-6481/lucene/sandbox/src/java/org/apache/lucene/search/GeoPointInPolygonQuery.java
    lucene/dev/branches/LUCENE-6481/lucene/sandbox/src/test/org/apache/lucene/search/TestGeoPointQuery.java

Modified: lucene/dev/branches/LUCENE-6481/lucene/sandbox/src/java/org/apache/lucene/search/GeoPointInBBoxQuery.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/LUCENE-6481/lucene/sandbox/src/java/org/apache/lucene/search/GeoPointInBBoxQuery.java?rev=1681836&r1=1681835&r2=1681836&view=diff
==============================================================================
--- lucene/dev/branches/LUCENE-6481/lucene/sandbox/src/java/org/apache/lucene/search/GeoPointInBBoxQuery.java (original)
+++ lucene/dev/branches/LUCENE-6481/lucene/sandbox/src/java/org/apache/lucene/search/GeoPointInBBoxQuery.java Tue May 26 20:07:12 2015
@@ -121,7 +121,13 @@ public class GeoPointInBBoxQuery extends
   @Override
   public String toString(String field) {
     final StringBuilder sb = new StringBuilder();
-    if (!getField().equals(field)) sb.append(getField()).append(':');
+    sb.append(getClass().getSimpleName());
+    sb.append(':');
+    if (!getField().equals(field)) {
+      sb.append(" field=");
+      sb.append(getField());
+      sb.append(':');
+    }
     return sb.append(" Lower Left: [")
         .append(minLon)
         .append(',')
@@ -170,7 +176,6 @@ public class GeoPointInBBoxQuery extends
      * @param start starting value on the space-filling curve for a cell at a given res
      * @param end ending value on the space-filling curve for a cell at a given res
      * @param res spatial res represented as a bit shift (MSB is lower res)
-     * @return
      */
     private void relateAndRecurse(final long start, final long end, final short res) {
       final double minLon = GeoUtils.mortonUnhash(start, true);

Modified: lucene/dev/branches/LUCENE-6481/lucene/sandbox/src/java/org/apache/lucene/search/GeoPointInPolygonQuery.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/LUCENE-6481/lucene/sandbox/src/java/org/apache/lucene/search/GeoPointInPolygonQuery.java?rev=1681836&r1=1681835&r2=1681836&view=diff
==============================================================================
--- lucene/dev/branches/LUCENE-6481/lucene/sandbox/src/java/org/apache/lucene/search/GeoPointInPolygonQuery.java (original)
+++ lucene/dev/branches/LUCENE-6481/lucene/sandbox/src/java/org/apache/lucene/search/GeoPointInPolygonQuery.java Tue May 26 20:07:12 2015
@@ -70,6 +70,19 @@ public final class GeoPointInPolygonQuer
   public GeoPointInPolygonQuery(final String field, final double minLon, final double minLat, final double maxLon,
                                 final double maxLat, final double[] polyLons, final double[] polyLats) {
     super(field, minLon, minLat, maxLon, maxLat);
+    if (polyLats.length != polyLons.length) {
+      throw new IllegalArgumentException("polyLats and polyLons must be equal length");
+    }
+    if (polyLats.length < 4) {
+      throw new IllegalArgumentException("at least 4 polygon points required");
+    }
+    if (polyLats[0] != polyLats[polyLats.length-1]) {
+      throw new IllegalArgumentException("first and last points of the polygon must be the same (it must close itself): polyLats[0]=" + polyLats[0] + " polyLats[" + (polyLats.length-1) + "]=" + polyLats[polyLats.length-1]);
+    }
+    if (polyLons[0] != polyLons[polyLons.length-1]) {
+      throw new IllegalArgumentException("first and last points of the polygon must be the same (it must close itself): polyLons[0]=" + polyLons[0] + " polyLons[" + (polyLons.length-1) + "]=" + polyLons[polyLons.length-1]);
+    }
+
     this.x = polyLons;
     this.y = polyLats;
   }
@@ -80,6 +93,7 @@ public final class GeoPointInPolygonQuer
    * GeoPolygonQuery(field, minLon, minLat, maxLon, maxLat, polyLons, polyLats)} by first computing the bounding
    * box lat/lon ranges
    */
+  // TODO: change this to normal ctor ... silly java requiring super() first
   public static GeoPointInPolygonQuery newPolygonQuery(final String field, final double[] polyLons, final double[] polyLats) {
     assert polyLons.length == polyLats.length;
     double minLon, maxLon, minLat, maxLat;
@@ -130,7 +144,13 @@ public final class GeoPointInPolygonQuer
     assert x.length == y.length;
 
     final StringBuilder sb = new StringBuilder();
-    if (!getField().equals(field)) sb.append(getField()).append(':');
+    sb.append(getClass().getSimpleName());
+    sb.append(':');
+    if (!getField().equals(field)) {
+      sb.append(" field=");
+      sb.append(getField());
+      sb.append(':');
+    }
     sb.append(" Points: ");
     for (int i=0; i<x.length; ++i) {
       sb.append("[")
@@ -146,12 +166,7 @@ public final class GeoPointInPolygonQuer
 
   private final class GeoPolygonTermsEnum extends GeoBBoxTermsEnum {
     GeoPolygonTermsEnum(final TermsEnum tenum) {
-            super(tenum);
-        }
-
-    @Override
-    protected boolean isWithin(final double minLon, final double minLat, final double maxLon, final double maxLat) {
-      return GeoUtils.rectIsWithin(minLon, minLat, maxLon, maxLat, x, y);
+      super(tenum);
     }
 
     /**
@@ -178,7 +193,7 @@ public final class GeoPointInPolygonQuer
       final double lon = GeoUtils.mortonUnhash(val, true);
       final double lat = GeoUtils.mortonUnhash(val, false);
       // post-filter by point in polygon
-      if (x!=null && !GeoUtils.pointInPolygon(x, y, lat, lon)) {
+      if (!GeoUtils.pointInPolygon(x, y, lat, lon)) {
         return AcceptStatus.NO;
       }
       return AcceptStatus.YES;

Modified: lucene/dev/branches/LUCENE-6481/lucene/sandbox/src/test/org/apache/lucene/search/TestGeoPointQuery.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/LUCENE-6481/lucene/sandbox/src/test/org/apache/lucene/search/TestGeoPointQuery.java?rev=1681836&r1=1681835&r2=1681836&view=diff
==============================================================================
--- lucene/dev/branches/LUCENE-6481/lucene/sandbox/src/test/org/apache/lucene/search/TestGeoPointQuery.java (original)
+++ lucene/dev/branches/LUCENE-6481/lucene/sandbox/src/test/org/apache/lucene/search/TestGeoPointQuery.java Tue May 26 20:07:12 2015
@@ -18,8 +18,11 @@ package org.apache.lucene.search;
  */
 
 import java.io.IOException;
+import java.util.ArrayList;
 import java.util.HashSet;
+import java.util.List;
 import java.util.Set;
+import java.util.concurrent.CountDownLatch;
 
 import org.apache.lucene.analysis.MockAnalyzer;
 import org.apache.lucene.document.Document;
@@ -116,8 +119,9 @@ public class TestGeoPointQuery extends L
   @Test
   public void testPolyQuery() throws Exception {
     TopDocs td = polygonQuery( new double[] { -96.7682647, -96.8280029, -96.6288757, -96.4929199,
-        -96.6041564, -96.7449188, -96.76826477}, new double[] { 33.073130, 32.9942669, 32.938386, 33.0374494,
-        33.1369762, 33.1162747, 33.073130}, 5);
+                                              -96.6041564, -96.7449188, -96.76826477, -96.7682647},
+      new double[] { 33.073130, 32.9942669, 32.938386, 33.0374494,
+                     33.1369762, 33.1162747, 33.073130, 33.073130}, 5);
     assertEquals("GeoPolygonQuery failed", td.totalHits, 1);
   }
 
@@ -204,7 +208,7 @@ public class TestGeoPointQuery extends L
     verify(lats, lons);
   }
 
-  private static void verify(double[] lats, double[] lons) throws IOException {
+  private static void verify(double[] lats, double[] lons) throws Exception {
     IndexWriterConfig iwc = newIndexWriterConfig();
     Directory dir;
     if (lats.length > 100000) {
@@ -223,7 +227,7 @@ public class TestGeoPointQuery extends L
         if (VERBOSE) {
           System.out.println("  id=" + id + " lat=" + lats[id] + " lon=" + lons[id]);
         }
-        doc.add(new GeoPointField("point", lons[id], lats[id], Field.Store.NO));
+        doc.add(new GeoPointField(FIELD_NAME, lons[id], lats[id], Field.Store.NO));
       } else if (VERBOSE) {
         System.out.println("  id=" + id + " skipped");
       }
@@ -243,66 +247,139 @@ public class TestGeoPointQuery extends L
     IndexReader r = DirectoryReader.open(w, true);
     w.close();
 
-    // We can't wrap with "exotic" readers because the BKD query must see the BKDDVFormat:
-    IndexSearcher s = newSearcher(r, false);
+    IndexSearcher s = newSearcher(r);
 
-    NumericDocValues docIDToID = MultiDocValues.getNumericValues(r, "id");
+    // nocommit put back:
+    // int numThreads = TestUtil.nextInt(random(), 2, 5);
+    int numThreads = 1;
 
-    int iters = atLeast(100);
-    for (int iter=0;iter<iters;iter++) {
-      double lat0 = randomLat();
-      double lat1 = randomLat();
-      double lon0 = randomLon();
-      double lon1 = randomLon();
-
-      if (lat1 < lat0) {
-        double x = lat0;
-        lat0 = lat1;
-        lat1 = x;
-      }
-
-      if (lon1 < lon0) {
-        double x = lon0;
-        lon0 = lon1;
-        lon1 = x;
-      }
-
-      if (VERBOSE) {
-        System.out.println("\nTEST: iter=" + iter + " lat=" + lat0 + " TO " + lat1 + " lon=" + lon0 + " TO " + lon1);
-      }
+    List<Thread> threads = new ArrayList<>();
+    final int iters = atLeast(100);
 
-      // nocommit test "in polygon" query too!
-      Query query = new GeoPointInBBoxQuery("point", lon0, lat0, lon1, lat1);
-
-      final FixedBitSet hits = new FixedBitSet(r.maxDoc());
-      s.search(query, new SimpleCollector() {
-
-          private int docBase;
+    final CountDownLatch startingGun = new CountDownLatch(1);
 
+    for(int i=0;i<numThreads;i++) {
+      Thread thread = new Thread() {
           @Override
-          public boolean needsScores() {
-            return false;
+          public void run() {
+            try {
+              _run();
+            } catch (Exception e) {
+              throw new RuntimeException(e);
+            }
           }
 
-          @Override
-          protected void doSetNextReader(LeafReaderContext context) throws IOException {
-            docBase = context.docBase;
-          }
+          private void _run() throws Exception {
+            startingGun.await();
 
-          @Override
-          public void collect(int doc) {
-            hits.set(docBase+doc);
+            NumericDocValues docIDToID = MultiDocValues.getNumericValues(r, "id");
+
+            for (int iter=0;iter<iters;iter++) {
+              double lat0 = randomLat();
+              double lat1 = randomLat();
+              double lon0 = randomLon();
+              double lon1 = randomLon();
+
+              if (lat1 < lat0) {
+                double x = lat0;
+                lat0 = lat1;
+                lat1 = x;
+              }
+
+              if (lon1 < lon0) {
+                double x = lon0;
+                lon0 = lon1;
+                lon1 = x;
+              }
+
+              if (VERBOSE) {
+                System.out.println("\nTEST: iter=" + iter + " lat=" + lat0 + " TO " + lat1 + " lon=" + lon0 + " TO " + lon1);
+              }
+
+              Query query;
+              boolean tooBigBBox = false;
+
+              double bboxLat0 = lat0;
+              double bboxLat1 = lat1;
+              double bboxLon0 = lon0;
+              double bboxLon1 = lon1;
+
+              // nocommit remove true || below:
+              if (true || random().nextBoolean()) {
+                query = new GeoPointInBBoxQuery(FIELD_NAME, lon0, lat0, lon1, lat1);
+              } else {
+                // nocommit remove "false &&" below
+                if (false && random().nextBoolean()) {
+                  // Intentionally pass a "too big" bounding box:
+                  double pct = random().nextDouble()*0.5;
+                  double width = lon1-lon0;
+                  bboxLon0 = Math.max(-180.0, lon0-width*pct);
+                  bboxLon1 = Math.min(180.0, lon1+width*pct);
+                  double height = lat1-lat0;
+                  bboxLat0 = Math.max(-90.0, lat0-height*pct);
+                  bboxLat1 = Math.min(90.0, lat1+height*pct);
+                  tooBigBBox = true;
+                }
+                double[] lats = new double[5];
+                double[] lons = new double[5];
+                lats[0] = bboxLat0;
+                lons[0] = bboxLon0;
+                lats[1] = bboxLat1;
+                lons[1] = bboxLon0;
+                lats[2] = bboxLat1;
+                lons[2] = bboxLon1;
+                lats[3] = bboxLat0;
+                lons[3] = bboxLon1;
+                lats[4] = bboxLat0;
+                lons[4] = bboxLon0;
+                query = new GeoPointInPolygonQuery(FIELD_NAME, bboxLon0, bboxLat0, bboxLon1, bboxLat1, lons, lats);
+              }
+
+              final FixedBitSet hits = new FixedBitSet(r.maxDoc());
+              s.search(query, new SimpleCollector() {
+
+                  private int docBase;
+
+                  @Override
+                  public boolean needsScores() {
+                    return false;
+                  }
+
+                  @Override
+                  protected void doSetNextReader(LeafReaderContext context) throws IOException {
+                    docBase = context.docBase;
+                  }
+
+                  @Override
+                  public void collect(int doc) {
+                    hits.set(docBase+doc);
+                  }
+                });
+
+              for(int docID=0;docID<r.maxDoc();docID++) {
+                int id = (int) docIDToID.get(docID);
+                boolean expected = deleted.contains(id) == false && rectContainsPointEnc(lat0, lat1, lon0, lon1, lats[id], lons[id]);
+                if (hits.get(docID) != expected) {
+                  System.out.println(Thread.currentThread().getName() + ": iter=" + iter + " id=" + id + " docID=" + docID + " lat=" + lats[id] + " lon=" + lons[id] + " (bbox: lat=" + lat0 + " TO " + lat1 + " lon=" + lon0 + " TO " + lon1 + ") expected " + expected + " but got: " + hits.get(docID) + " deleted?=" + deleted.contains(id) + " query=" + query);
+                  if (tooBigBBox) {
+                    System.out.println("  passed too-big bbox: lat=" + bboxLat0 + " TO " + bboxLat1 + " lon=" + bboxLon0 + " TO " + bboxLon1);
+                  }
+                  fail("wrong result");
+                }
+              }
+            }
           }
-        });
+        };
+      thread.setName("T" + i);
+      thread.start();
+      threads.add(thread);
+    }
 
-      for(int docID=0;docID<r.maxDoc();docID++) {
-        int id = (int) docIDToID.get(docID);
-        boolean expected = deleted.contains(id) == false && rectContainsPointEnc(lat0, lat1, lon0, lon1, lats[id], lons[id]);
-        if (hits.get(docID) != expected) {
-          fail("id=" + id + " docID=" + docID + " lat=" + lats[id] + " lon=" + lons[id] + " expected " + expected + " but got: " + hits.get(docID) + " deleted?=" + deleted.contains(id));
-        }
-      }
+    startingGun.countDown();
+    for(Thread thread : threads) {
+      thread.join();
     }
+
     IOUtils.close(r, dir);
   }
 
@@ -312,19 +389,7 @@ public class TestGeoPointQuery extends L
     if (Double.isNaN(pointLat)) {
       return false;
     }
-    /*
-    int rectLatMinEnc = BKDTreeWriter.encodeLat(rectLatMin);
-    int rectLatMaxEnc = BKDTreeWriter.encodeLat(rectLatMax);
-    int rectLonMinEnc = BKDTreeWriter.encodeLon(rectLonMin);
-    int rectLonMaxEnc = BKDTreeWriter.encodeLon(rectLonMax);
-    int pointLatEnc = BKDTreeWriter.encodeLat(pointLat);
-    int pointLonEnc = BKDTreeWriter.encodeLon(pointLon);
-
-    return pointLatEnc >= rectLatMinEnc &&
-      pointLatEnc < rectLatMaxEnc &&
-      pointLonEnc >= rectLonMinEnc &&
-      pointLonEnc < rectLonMaxEnc;
-    */
+
     return GeoUtils.bboxContains(pointLon, pointLat, rectLonMin, rectLatMin, rectLonMax, rectLatMax);
   }
 
@@ -335,5 +400,4 @@ public class TestGeoPointQuery extends L
   private static double randomLon() {
     return -180 + 360.0 * random().nextDouble();
   }
-
 }