You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by kw...@apache.org on 2016/04/09 22:12:58 UTC
lucene-solr:master: LUCENE-7197: Fix two test failures and add more
forensics that helped resolve the issue.
Repository: lucene-solr
Updated Branches:
refs/heads/master 4d3a633bf -> d377e7fd3
LUCENE-7197: Fix two test failures and add more forensics that helped resolve the issue.
Project: http://git-wip-us.apache.org/repos/asf/lucene-solr/repo
Commit: http://git-wip-us.apache.org/repos/asf/lucene-solr/commit/d377e7fd
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/d377e7fd
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/d377e7fd
Branch: refs/heads/master
Commit: d377e7fd34b4ace829ee6d4ba0486500aaef506b
Parents: 4d3a633
Author: Karl Wright <Da...@gmail.com>
Authored: Sat Apr 9 16:12:39 2016 -0400
Committer: Karl Wright <Da...@gmail.com>
Committed: Sat Apr 9 16:12:39 2016 -0400
----------------------------------------------------------------------
.../spatial3d/geom/GeoConcavePolygon.java | 10 ++++----
.../lucene/spatial3d/geom/GeoConvexPolygon.java | 1 +
.../apache/lucene/spatial3d/geom/GeoPoint.java | 2 +-
.../spatial3d/geom/GeoPolygonFactory.java | 12 ++++++++--
.../lucene/spatial3d/geom/PlanetModel.java | 25 ++++++++++++++++++++
.../lucene/spatial3d/geom/StandardXYZSolid.java | 18 +++++++++++++-
.../apache/lucene/spatial3d/TestGeo3DPoint.java | 25 +++++++++++++++-----
7 files changed, 78 insertions(+), 15 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/d377e7fd/lucene/spatial3d/src/java/org/apache/lucene/spatial3d/geom/GeoConcavePolygon.java
----------------------------------------------------------------------
diff --git a/lucene/spatial3d/src/java/org/apache/lucene/spatial3d/geom/GeoConcavePolygon.java b/lucene/spatial3d/src/java/org/apache/lucene/spatial3d/geom/GeoConcavePolygon.java
index 05e58ee..8c6f757 100644
--- a/lucene/spatial3d/src/java/org/apache/lucene/spatial3d/geom/GeoConcavePolygon.java
+++ b/lucene/spatial3d/src/java/org/apache/lucene/spatial3d/geom/GeoConcavePolygon.java
@@ -202,11 +202,9 @@ class GeoConcavePolygon extends GeoBasePolygon {
throw new IllegalArgumentException("Polygon points are all coplanar");
}
final GeoPoint check = points.get(endPointIndex);
- // Here note the flip of the sense of the sided plane!!
- final SidedPlane sp = new SidedPlane(check, false, start, end);
//System.out.println("Created edge "+sp+" using start="+start+" end="+end+" check="+check);
- edges[i] = sp;
- invertedEdges[i] = new SidedPlane(sp);
+ edges[i] = new SidedPlane(check, false, start, end);
+ invertedEdges[i] = new SidedPlane(edges[i]);
notableEdgePoints[i] = new GeoPoint[]{start, end};
}
// In order to naively confirm that the polygon is concave, I would need to
@@ -277,9 +275,11 @@ class GeoConcavePolygon extends GeoBasePolygon {
// cannot use them as bounds. They are independent hemispheres.
for (int edgeIndex = 0; edgeIndex < edges.length; edgeIndex++) {
final SidedPlane edge = edges[edgeIndex];
+ final SidedPlane invertedEdge = invertedEdges[edgeIndex];
final GeoPoint[] points = this.notableEdgePoints[edgeIndex];
if (!isInternalEdges.get(edgeIndex)) {
- if (edge.intersects(planetModel, p, notablePoints, points, bounds, eitherBounds.get(edge))) {
+ //System.err.println("Checking concave edge "+edge+" for intersection against plane "+p);
+ if (invertedEdge.intersects(planetModel, p, notablePoints, points, bounds, eitherBounds.get(edge))) {
//System.err.println(" intersects!");
return true;
}
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/d377e7fd/lucene/spatial3d/src/java/org/apache/lucene/spatial3d/geom/GeoConvexPolygon.java
----------------------------------------------------------------------
diff --git a/lucene/spatial3d/src/java/org/apache/lucene/spatial3d/geom/GeoConvexPolygon.java b/lucene/spatial3d/src/java/org/apache/lucene/spatial3d/geom/GeoConvexPolygon.java
index d1e0091..b631b55 100755
--- a/lucene/spatial3d/src/java/org/apache/lucene/spatial3d/geom/GeoConvexPolygon.java
+++ b/lucene/spatial3d/src/java/org/apache/lucene/spatial3d/geom/GeoConvexPolygon.java
@@ -265,6 +265,7 @@ class GeoConvexPolygon extends GeoBasePolygon {
final SidedPlane edge = edges[edgeIndex];
final GeoPoint[] points = this.notableEdgePoints[edgeIndex];
if (!isInternalEdges.get(edgeIndex)) {
+ //System.err.println("Checking convex edge "+edge+" for intersection against plane "+p);
if (edge.intersects(planetModel, p, notablePoints, points, bounds, eitherBounds.get(edge))) {
//System.err.println(" intersects!");
return true;
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/d377e7fd/lucene/spatial3d/src/java/org/apache/lucene/spatial3d/geom/GeoPoint.java
----------------------------------------------------------------------
diff --git a/lucene/spatial3d/src/java/org/apache/lucene/spatial3d/geom/GeoPoint.java b/lucene/spatial3d/src/java/org/apache/lucene/spatial3d/geom/GeoPoint.java
index 31ab0aa..662a056 100755
--- a/lucene/spatial3d/src/java/org/apache/lucene/spatial3d/geom/GeoPoint.java
+++ b/lucene/spatial3d/src/java/org/apache/lucene/spatial3d/geom/GeoPoint.java
@@ -188,6 +188,6 @@ public class GeoPoint extends Vector {
if (this.longitude == Double.NEGATIVE_INFINITY) {
return super.toString();
}
- return "[lat="+getLatitude()+", lon="+getLongitude()+"]";
+ return "[lat="+getLatitude()+", lon="+getLongitude()+"("+super.toString()+")]";
}
}
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/d377e7fd/lucene/spatial3d/src/java/org/apache/lucene/spatial3d/geom/GeoPolygonFactory.java
----------------------------------------------------------------------
diff --git a/lucene/spatial3d/src/java/org/apache/lucene/spatial3d/geom/GeoPolygonFactory.java b/lucene/spatial3d/src/java/org/apache/lucene/spatial3d/geom/GeoPolygonFactory.java
index a68f908..7fc22dd 100755
--- a/lucene/spatial3d/src/java/org/apache/lucene/spatial3d/geom/GeoPolygonFactory.java
+++ b/lucene/spatial3d/src/java/org/apache/lucene/spatial3d/geom/GeoPolygonFactory.java
@@ -395,6 +395,7 @@ public class GeoPolygonFactory {
if (confirmEdge == checkEdge) {
continue;
}
+ // Look for a point that is on the wrong side of the check edge. This means that we can't build the polygon.
final GeoPoint thePoint;
if (checkEdge.startPoint != confirmEdge.startPoint && checkEdge.endPoint != confirmEdge.startPoint && !flippedPlane.isWithin(confirmEdge.startPoint)) {
thePoint = confirmEdge.startPoint;
@@ -404,7 +405,11 @@ public class GeoPolygonFactory {
thePoint = null;
}
if (thePoint != null) {
- // Found a split!!
+ // thePoint is on the wrong side of the complementary plane. That means we cannot build a concave polygon, because the complement would not
+ // be a legal convex polygon.
+ // But we can take advantage of the fact that the distance between the edge and thePoint is less than 180 degrees, and so we can split the
+ // would-be concave polygon into three segments. The first segment includes the edge and thePoint, and uses the sense of the edge to determine the sense
+ // of the polygon.
// This should be the only problematic part of the polygon.
// We know that thePoint is on the "wrong" side of the edge -- that is, it's on the side that the
@@ -431,6 +436,8 @@ public class GeoPolygonFactory {
}
//System.out.println("...done convex part.");
+ // ??? check if we get the sense right
+
// The part preceding the bad edge, back to thePoint, needs to be recursively
// processed. So, assemble what we need, which is basically a list of edges.
Edge loopEdge = edgeBuffer.getPrevious(checkEdge);
@@ -459,6 +466,7 @@ public class GeoPolygonFactory {
return false;
}
//System.out.println("...done first part.");
+
final List<GeoPoint> secondPartPoints = new ArrayList<>();
final BitSet secondPartInternal = new BitSet();
loopEdge = edgeBuffer.getNext(checkEdge);
@@ -479,7 +487,7 @@ public class GeoPolygonFactory {
secondPartInternal,
secondPartPoints.size()-1,
0,
- new SidedPlane(checkEdge.endPoint, true, checkEdge.startPoint, thePoint),
+ new SidedPlane(checkEdge.startPoint, false, checkEdge.endPoint, thePoint),
holes,
testPoint) == false) {
return false;
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/d377e7fd/lucene/spatial3d/src/java/org/apache/lucene/spatial3d/geom/PlanetModel.java
----------------------------------------------------------------------
diff --git a/lucene/spatial3d/src/java/org/apache/lucene/spatial3d/geom/PlanetModel.java b/lucene/spatial3d/src/java/org/apache/lucene/spatial3d/geom/PlanetModel.java
index c7d45a8..f5ab8d8 100644
--- a/lucene/spatial3d/src/java/org/apache/lucene/spatial3d/geom/PlanetModel.java
+++ b/lucene/spatial3d/src/java/org/apache/lucene/spatial3d/geom/PlanetModel.java
@@ -189,6 +189,31 @@ public class PlanetModel {
return (x * x + y * y) * inverseAb * inverseAb + z * z * inverseC * inverseC - 1.0 > Vector.MINIMUM_RESOLUTION;
}
+ /** Compute a GeoPoint that's scaled to actually be on the planet surface.
+ * @param vector is the vector.
+ * @return the scaled point.
+ */
+ public GeoPoint createSurfacePoint(final Vector vector) {
+ return createSurfacePoint(vector.x, vector.y, vector.z);
+ }
+
+ /** Compute a GeoPoint that's based on (x,y,z) values, but is scaled to actually be on the planet surface.
+ * @param x is the x value.
+ * @param y is the y value.
+ * @param z is the z value.
+ * @return the scaled point.
+ */
+ public GeoPoint createSurfacePoint(final double x, final double y, final double z) {
+ // The equation of the surface is:
+ // (x^2 / a^2 + y^2 / b^2 + z^2 / c^2) = 1
+ // We will need to scale the passed-in x, y, z values:
+ // ((tx)^2 / a^2 + (ty)^2 / b^2 + (tz)^2 / c^2) = 1
+ // t^2 * (x^2 / a^2 + y^2 / b^2 + z^2 / c^2) = 1
+ // t = sqrt ( 1 / (x^2 / a^2 + y^2 / b^2 + z^2 / c^2))
+ final double t = Math.sqrt(1.0 / (x*x*inverseAbSquared + y*y*inverseAbSquared + z*z*inverseCSquared));
+ return new GeoPoint(t*x, t*y, t*z);
+ }
+
/** Compute a GeoPoint that's a bisection between two other GeoPoints.
* @param pt1 is the first point.
* @param pt2 is the second point.
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/d377e7fd/lucene/spatial3d/src/java/org/apache/lucene/spatial3d/geom/StandardXYZSolid.java
----------------------------------------------------------------------
diff --git a/lucene/spatial3d/src/java/org/apache/lucene/spatial3d/geom/StandardXYZSolid.java b/lucene/spatial3d/src/java/org/apache/lucene/spatial3d/geom/StandardXYZSolid.java
index ec2e26c..9d94c51 100644
--- a/lucene/spatial3d/src/java/org/apache/lucene/spatial3d/geom/StandardXYZSolid.java
+++ b/lucene/spatial3d/src/java/org/apache/lucene/spatial3d/geom/StandardXYZSolid.java
@@ -16,6 +16,8 @@
*/
package org.apache.lucene.spatial3d.geom;
+import java.util.Arrays;
+
/**
* 3D rectangle, bounded on six sides by X,Y,Z limits
*
@@ -152,6 +154,11 @@ class StandardXYZSolid extends BaseXYZSolid {
notableMinZPoints = glueTogether(minXminZ, maxXminZ, minYminZ, maxYminZ);
notableMaxZPoints = glueTogether(minXmaxZ, maxXmaxZ, minYmaxZ, maxYmaxZ);
+ //System.err.println(
+ // " notableMinXPoints="+Arrays.asList(notableMinXPoints)+" notableMaxXPoints="+Arrays.asList(notableMaxXPoints)+
+ // " notableMinYPoints="+Arrays.asList(notableMinYPoints)+" notableMaxYPoints="+Arrays.asList(notableMaxYPoints)+
+ // " notableMinZPoints="+Arrays.asList(notableMinZPoints)+" notableMaxZPoints="+Arrays.asList(notableMaxZPoints));
+
// Now, compute the edge points.
// This is the trickiest part of setting up an XYZSolid. We've computed intersections already, so
// we'll start there.
@@ -173,7 +180,11 @@ class StandardXYZSolid extends BaseXYZSolid {
final boolean maxXminYmaxZ = planetModel.pointOutside(maxX, minY, maxZ);
final boolean maxXmaxYminZ = planetModel.pointOutside(maxX, maxY, minZ);
final boolean maxXmaxYmaxZ = planetModel.pointOutside(maxX, maxY, maxZ);
-
+
+ //System.err.println("Outside world: minXminYminZ="+minXminYminZ+" minXminYmaxZ="+minXminYmaxZ+" minXmaxYminZ="+minXmaxYminZ+
+ // " minXmaxYmaxZ="+minXmaxYmaxZ+" maxXminYminZ="+maxXminYminZ+" maxXminYmaxZ="+maxXminYmaxZ+" maxXmaxYminZ="+maxXmaxYminZ+
+ // " maxXmaxYmaxZ="+maxXmaxYmaxZ);
+
// Look at single-plane/world intersections.
// We detect these by looking at the world model and noting its x, y, and z bounds.
@@ -286,6 +297,11 @@ class StandardXYZSolid extends BaseXYZSolid {
maxZEdges = EMPTY_POINTS;
}
+ //System.err.println(
+ // " minXEdges="+Arrays.asList(minXEdges)+" maxXEdges="+Arrays.asList(maxXEdges)+
+ // " minYEdges="+Arrays.asList(minYEdges)+" maxYEdges="+Arrays.asList(maxYEdges)+
+ // " minZEdges="+Arrays.asList(minZEdges)+" maxZEdges="+Arrays.asList(maxZEdges));
+
// Glue everything together. This is not a minimal set of edgepoints, as of now, but it does completely describe all shapes on the
// planet.
this.edgePoints = glueTogether(minXminY, minXmaxY, minXminZ, minXmaxZ,
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/d377e7fd/lucene/spatial3d/src/test/org/apache/lucene/spatial3d/TestGeo3DPoint.java
----------------------------------------------------------------------
diff --git a/lucene/spatial3d/src/test/org/apache/lucene/spatial3d/TestGeo3DPoint.java b/lucene/spatial3d/src/test/org/apache/lucene/spatial3d/TestGeo3DPoint.java
index 4edfd2d..5c876ce 100644
--- a/lucene/spatial3d/src/test/org/apache/lucene/spatial3d/TestGeo3DPoint.java
+++ b/lucene/spatial3d/src/test/org/apache/lucene/spatial3d/TestGeo3DPoint.java
@@ -782,6 +782,7 @@ public class TestGeo3DPoint extends LuceneTestCase {
if (point != null) {
boolean expected = ((deleted.contains(id) == false) && ((PointInGeo3DShapeQuery)query).getShape().isWithin(point));
if (hits.get(docID) != expected) {
+ GeoPoint scaledPoint = PlanetModel.WGS84.createSurfacePoint(point);
StringBuilder b = new StringBuilder();
if (expected) {
b.append("FAIL: id=" + id + " should have matched but did not\n");
@@ -789,10 +790,14 @@ public class TestGeo3DPoint extends LuceneTestCase {
b.append("FAIL: id=" + id + " should not have matched but did\n");
}
b.append(" shape=" + ((PointInGeo3DShapeQuery)query).getShape() + "\n");
+ b.append(" world bounds=(" +
+ " minX=" + PlanetModel.WGS84.getMinimumXValue() + " maxX=" + PlanetModel.WGS84.getMaximumXValue() +
+ " minY=" + PlanetModel.WGS84.getMinimumYValue() + " maxY=" + PlanetModel.WGS84.getMaximumYValue() +
+ " minZ=" + PlanetModel.WGS84.getMinimumZValue() + " maxZ=" + PlanetModel.WGS84.getMaximumZValue() + "\n");
b.append(" point=" + point + "\n");
b.append(" docID=" + docID + " deleted?=" + deleted.contains(id) + "\n");
b.append(" query=" + query + "\n");
- b.append(" explanation:\n " + explain("point", ((PointInGeo3DShapeQuery)query).getShape(), point, r, docID).replace("\n", "\n "));
+ b.append(" explanation:\n " + explain("point", ((PointInGeo3DShapeQuery)query).getShape(), point, scaledPoint, r, docID).replace("\n", "\n "));
fail(b.toString());
}
} else {
@@ -810,7 +815,7 @@ public class TestGeo3DPoint extends LuceneTestCase {
}
public void testShapeQueryToString() {
- assertEquals("PointInGeo3DShapeQuery: field=point: Shape: GeoStandardCircle: {planetmodel=PlanetModel.WGS84, center=[lat=0.7722082215479366, lon=0.13560747521073413], radius=0.1(5.729577951308232)}",
+ assertEquals("PointInGeo3DShapeQuery: field=point: Shape: GeoStandardCircle: {planetmodel=PlanetModel.WGS84, center=[lat=0.7722082215479366, lon=0.13560747521073413([X=0.7094263130137863, Y=0.09679758930862137, Z=0.6973564619248455])], radius=0.1(5.729577951308232)}",
Geo3DPoint.newShapeQuery("point", GeoCircleFactory.makeGeoCircle(PlanetModel.WGS84, toRadians(44.244272), toRadians(7.769736), 0.1)).toString());
}
@@ -1171,6 +1176,7 @@ public class TestGeo3DPoint extends LuceneTestCase {
final GeoShape shape;
final GeoPoint targetDocPoint;
+ final GeoPoint scaledDocPoint;
final IntersectVisitor in;
final List<Cell> stack = new ArrayList<>();
private List<Cell> stackToTargetDoc;
@@ -1183,9 +1189,10 @@ public class TestGeo3DPoint extends LuceneTestCase {
// In the first phase, we always return CROSSES to do a full scan of the BKD tree to see which leaf block the document lives in
boolean firstPhase = true;
- public ExplainingVisitor(GeoShape shape, GeoPoint targetDocPoint, IntersectVisitor in, int targetDocID, int numDims, int bytesPerDim, StringBuilder b) {
+ public ExplainingVisitor(GeoShape shape, GeoPoint targetDocPoint, GeoPoint scaledDocPoint, IntersectVisitor in, int targetDocID, int numDims, int bytesPerDim, StringBuilder b) {
this.shape = shape;
this.targetDocPoint = targetDocPoint;
+ this.scaledDocPoint = scaledDocPoint;
this.in = in;
this.targetDocID = targetDocID;
this.numDims = numDims;
@@ -1302,6 +1309,9 @@ public class TestGeo3DPoint extends LuceneTestCase {
final int relationship = xyzSolid.getRelationship(shape);
final boolean pointWithinShape = shape.isWithin(targetDocPoint);
final boolean pointWithinCell = xyzSolid.isWithin(targetDocPoint);
+ final boolean scaledWithinShape = shape.isWithin(scaledDocPoint);
+ final boolean scaledWithinCell = xyzSolid.isWithin(scaledDocPoint);
+
final String relationshipString;
switch (relationship) {
case GeoArea.CONTAINS:
@@ -1320,7 +1330,10 @@ public class TestGeo3DPoint extends LuceneTestCase {
relationshipString = "UNKNOWN";
break;
}
- return "Cell(x=" + xMin + " TO " + xMax + " y=" + yMin + " TO " + yMax + " z=" + zMin + " TO " + zMax + "); Shape relationship = "+relationshipString+"; Point within cell = "+pointWithinCell+"; Point within shape = "+pointWithinShape;
+ return "Cell(x=" + xMin + " TO " + xMax + " y=" + yMin + " TO " + yMax + " z=" + zMin + " TO " + zMax +
+ "); Shape relationship = "+relationshipString+
+ "; Point within cell = "+pointWithinCell+"; Point within shape = "+pointWithinShape+
+ "; Scaled point within cell = "+scaledWithinCell+"; Scaled point within shape = "+scaledWithinShape;
}
@Override
@@ -1340,7 +1353,7 @@ public class TestGeo3DPoint extends LuceneTestCase {
}
}
- public static String explain(String fieldName, GeoShape shape, GeoPoint targetDocPoint, IndexReader reader, int docID) throws Exception {
+ public static String explain(String fieldName, GeoShape shape, GeoPoint targetDocPoint, GeoPoint scaledDocPoint, IndexReader reader, int docID) throws Exception {
// First find the leaf reader that owns this doc:
int subIndex = ReaderUtil.subIndex(docID, reader.leaves());
@@ -1350,7 +1363,7 @@ public class TestGeo3DPoint extends LuceneTestCase {
b.append("target is in leaf " + leafReader + " of full reader " + reader + "\n");
DocIdSetBuilder hits = new DocIdSetBuilder(leafReader.maxDoc());
- ExplainingVisitor visitor = new ExplainingVisitor(shape, targetDocPoint, new PointInShapeIntersectVisitor(hits, shape), docID - reader.leaves().get(subIndex).docBase, 3, Integer.BYTES, b);
+ ExplainingVisitor visitor = new ExplainingVisitor(shape, targetDocPoint, scaledDocPoint, new PointInShapeIntersectVisitor(hits, shape), docID - reader.leaves().get(subIndex).docBase, 3, Integer.BYTES, b);
// Do first phase, where we just figure out the "path" that leads to the target docID:
leafReader.getPointValues().intersect(fieldName, visitor);