You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lucene.apache.org by iverase <gi...@git.apache.org> on 2017/12/08 15:39:05 UTC
[GitHub] lucene-solr pull request #288: LUCENE-8086
GitHub user iverase opened a pull request:
https://github.com/apache/lucene-solr/pull/288
LUCENE-8086
Here are the changes, in particular:
- Geo3dFactory: Use GeoExactCircle for non-spherical planets.
- Geo3dCircleShape: Remove method relate.
- Geo3DShape: Use new factory method for building GeoBbox from bounds object.
- Geo3dDistanceCalculator: use pointonbearing from planet model.
- Test refactoring
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/iverase/lucene-solr master
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/lucene-solr/pull/288.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #288
----
commit 73d9ce324b217c633ae72b24233099dd2afc43d5
Author: iverase <iv...@eso.org>
Date: 2017-12-08T15:33:22Z
LUCENE-8086
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr pull request #288: LUCENE-8086
Posted by dsmiley <gi...@git.apache.org>.
Github user dsmiley commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/288#discussion_r155822689
--- Diff: lucene/spatial-extras/src/java/org/apache/lucene/spatial/spatial4j/Geo3dShapeFactory.java ---
@@ -150,10 +176,21 @@ public Rectangle rect(double minX, double maxX, double minY, double maxY) {
@Override
public Circle circle(double x, double y, double distance) {
- GeoCircle circle = GeoCircleFactory.makeGeoCircle(planetModel,
- y * DistanceUtils.DEGREES_TO_RADIANS,
- x * DistanceUtils.DEGREES_TO_RADIANS,
- distance * DistanceUtils.DEGREES_TO_RADIANS);
+ GeoCircle circle;
+ if (planetModel.ab == planetModel.c) {
--- End diff --
Should there be a method on planetModel that more descriptively characterizes the condition? (e.g. isSpherical?) Just a suggestion; perhaps not if it's too hard to give an appropriate name. If not then maybe add a comment here so we know what "ab" being equal to "c" means.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr pull request #288: LUCENE-8086
Posted by dsmiley <gi...@git.apache.org>.
Github user dsmiley commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/288#discussion_r155824604
--- Diff: lucene/spatial-extras/src/test/org/apache/lucene/spatial/spatial4j/Geo3dShapeRectRelationTestCase.java ---
@@ -155,16 +107,12 @@ protected Geo3dShape generateRandomShape(Point nearP) {
ulhcPoint = lrhcPoint;
lrhcPoint = temp;
}
- final GeoBBox shape = GeoBBoxFactory.makeGeoBBox(planetModel, ulhcPoint.getY() * DEGREES_TO_RADIANS,
- lrhcPoint.getY() * DEGREES_TO_RADIANS,
- ulhcPoint.getX() * DEGREES_TO_RADIANS,
- lrhcPoint.getX() * DEGREES_TO_RADIANS);
- return new Geo3dShape(shape, ctx);
+ return (Geo3dShape<?>) ctx.getShapeFactory().rect(lrhcPoint, ulhcPoint);
--- End diff --
change is good but the variable names are wrong. `rect(lowerLeft, upperRight)`
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr pull request #288: LUCENE-8086
Posted by iverase <gi...@git.apache.org>.
Github user iverase commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/288#discussion_r155843390
--- Diff: lucene/spatial-extras/src/java/org/apache/lucene/spatial/spatial4j/Geo3dShapeFactory.java ---
@@ -67,6 +74,25 @@ public SpatialContext getSpatialContext() {
return context;
}
+ /**
+ * Set the accuracy for circles.
+ *
+ * "Accuracy" is defined as the maximum linear distance between any point on the
+ * surface circle and planes that describe the circle. Therefore on WSG84, since the
+ * radius of earth is 6,371,000 meters, an accuracy of 1e-6 corresponds to 6.3 meters.
+ * For an accuracy of 1.0 meters, the value of 1.6e-7.
+ *
+ * The default value is set to 10m (1.6e-6).
+ *
+ * Note that accuracy has no effect when the planet model is a sphere. In that case circles
+ * are always fully precise.
+ *
+ * @param circleAccuracy the provided accuracy as a linear distance.
--- End diff --
I need to ask Karl Wright if that is what it means, but I guess so. I will update accordingly.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr pull request #288: LUCENE-8086
Posted by dsmiley <gi...@git.apache.org>.
Github user dsmiley commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/288#discussion_r155938315
--- Diff: lucene/spatial-extras/src/test/org/apache/lucene/spatial/spatial4j/ShapeRectRelationTestCase.java ---
@@ -28,15 +28,15 @@
import static org.locationtech.spatial4j.distance.DistanceUtils.DEGREES_TO_RADIANS;
-public abstract class Geo3dShapeRectRelationTestCase extends RandomizedShapeTestCase {
+public abstract class ShapeRectRelationTestCase extends RandomizedShapeTestCase {
protected final static double RADIANS_PER_DEGREE = Math.PI/180.0;
@Rule
public final TestLog testLog = TestLog.instance;
protected int maxRadius = 180;
- public Geo3dShapeRectRelationTestCase() {
+ public ShapeRectRelationTestCase() {
super(SpatialContext.GEO);
--- End diff --
I think the subclass should pass in the context.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr pull request #288: LUCENE-8086
Posted by iverase <gi...@git.apache.org>.
Github user iverase commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/288#discussion_r155844229
--- Diff: lucene/spatial-extras/src/test/org/apache/lucene/spatial/spatial4j/Geo3dShapeRectRelationTestCase.java ---
@@ -155,16 +107,12 @@ protected Geo3dShape generateRandomShape(Point nearP) {
ulhcPoint = lrhcPoint;
lrhcPoint = temp;
}
- final GeoBBox shape = GeoBBoxFactory.makeGeoBBox(planetModel, ulhcPoint.getY() * DEGREES_TO_RADIANS,
- lrhcPoint.getY() * DEGREES_TO_RADIANS,
- ulhcPoint.getX() * DEGREES_TO_RADIANS,
- lrhcPoint.getX() * DEGREES_TO_RADIANS);
- return new Geo3dShape(shape, ctx);
+ return (Geo3dShape<?>) ctx.getShapeFactory().rect(lrhcPoint, ulhcPoint);
--- End diff --
Indeed!
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr pull request #288: LUCENE-8086
Posted by dsmiley <gi...@git.apache.org>.
Github user dsmiley commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/288#discussion_r155938328
--- Diff: lucene/spatial-extras/src/java/org/apache/lucene/spatial/spatial4j/Geo3dShapeFactory.java ---
@@ -150,10 +176,21 @@ public Rectangle rect(double minX, double maxX, double minY, double maxY) {
@Override
public Circle circle(double x, double y, double distance) {
- GeoCircle circle = GeoCircleFactory.makeGeoCircle(planetModel,
- y * DistanceUtils.DEGREES_TO_RADIANS,
- x * DistanceUtils.DEGREES_TO_RADIANS,
- distance * DistanceUtils.DEGREES_TO_RADIANS);
+ GeoCircle circle;
+ if (planetModel.ab == planetModel.c) {
--- End diff --
@DaddyWri comments?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr pull request #288: LUCENE-8086
Posted by dsmiley <gi...@git.apache.org>.
Github user dsmiley commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/288#discussion_r155826399
--- Diff: lucene/spatial-extras/src/java/org/apache/lucene/spatial/spatial4j/Geo3dCircleShape.java ---
@@ -67,16 +64,4 @@ public Point getCenter() {
}
return center;
}
-
--- End diff --
Yay
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr pull request #288: LUCENE-8086
Posted by dsmiley <gi...@git.apache.org>.
Github user dsmiley commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/288#discussion_r155938309
--- Diff: lucene/spatial-extras/src/test/org/apache/lucene/spatial/spatial4j/ShapeRectRelationTestCase.java ---
@@ -75,9 +75,9 @@ public void testGeoCircleRect() {
new Geo3dRectIntersectionTestHelper(ctx) {
--- End diff --
Should be renamed as this class is no longer Geo3d specific
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr pull request #288: LUCENE-8086
Posted by dsmiley <gi...@git.apache.org>.
Github user dsmiley commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/288#discussion_r155822251
--- Diff: lucene/spatial-extras/src/java/org/apache/lucene/spatial/spatial4j/Geo3dShapeFactory.java ---
@@ -67,6 +74,25 @@ public SpatialContext getSpatialContext() {
return context;
}
+ /**
+ * Set the accuracy for circles.
+ *
+ * "Accuracy" is defined as the maximum linear distance between any point on the
+ * surface circle and planes that describe the circle. Therefore on WSG84, since the
+ * radius of earth is 6,371,000 meters, an accuracy of 1e-6 corresponds to 6.3 meters.
+ * For an accuracy of 1.0 meters, the value of 1.6e-7.
+ *
+ * The default value is set to 10m (1.6e-6).
+ *
+ * Note that accuracy has no effect when the planet model is a sphere. In that case circles
+ * are always fully precise.
+ *
+ * @param circleAccuracy the provided accuracy as a linear distance.
--- End diff --
by "linear distance" do you mean decimal degrees as is used in other parts of the Spatial4j API? If so please say "decimal degrees". If not, perhaps it should be in that unit?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr pull request #288: LUCENE-8086
Posted by dsmiley <gi...@git.apache.org>.
Github user dsmiley commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/288#discussion_r155825386
--- Diff: lucene/spatial-extras/src/test/org/apache/lucene/spatial/spatial4j/Geo3dShapeRectRelationTestCase.java ---
@@ -16,29 +16,15 @@
*/
package org.apache.lucene.spatial.spatial4j;
-import java.util.ArrayList;
-import java.util.List;
-
-import org.apache.lucene.spatial3d.geom.GeoPath;
-import org.apache.lucene.spatial3d.geom.GeoPolygon;
+import org.junit.Rule;
+import org.junit.Test;
import org.locationtech.spatial4j.TestLog;
import org.locationtech.spatial4j.context.SpatialContext;
-import org.locationtech.spatial4j.distance.DistanceUtils;
import org.locationtech.spatial4j.shape.Circle;
import org.locationtech.spatial4j.shape.Point;
import org.locationtech.spatial4j.shape.RectIntersectionTestHelper;
-import org.apache.lucene.spatial3d.geom.LatLonBounds;
--- End diff --
If I get this right, you've removed the Geo3D dependencies of this test. Yet it's still named to be Geo3d?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr pull request #288: LUCENE-8086
Posted by dsmiley <gi...@git.apache.org>.
Github user dsmiley commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/288#discussion_r155821440
--- Diff: lucene/spatial-extras/src/java/org/apache/lucene/spatial/spatial4j/Geo3dShapeFactory.java ---
@@ -55,6 +55,13 @@
private SpatialContext context;
private PlanetModel planetModel;
+ /**
+ * Default accuracy for circles when not using the unit sphere.
+ * It is equivalent to 10m on the surface of the earth.
+ */
+ private static double DEFAULT_CIRCLE_ACCURACY = 1.6e-6;
--- End diff --
should be final
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr pull request #288: LUCENE-8086
Posted by dsmiley <gi...@git.apache.org>.
Github user dsmiley commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/288#discussion_r155826444
--- Diff: lucene/spatial-extras/src/java/org/apache/lucene/spatial/spatial4j/Geo3dDistanceCalculator.java ---
@@ -73,62 +74,20 @@ public boolean within(Point from, double toX, double toY, double distance) {
@Override
public Point pointOnBearing(Point from, double distDEG, double bearingDEG, SpatialContext ctx, Point reuse) {
- // Algorithm using Vincenty's formulae (https://en.wikipedia.org/wiki/Vincenty%27s_formulae)
- // which takes into account that planets may not be spherical.
- //Code adaptation from http://www.movable-type.co.uk/scripts/latlong-vincenty.html
Geo3dPointShape geoFrom = (Geo3dPointShape) from;
GeoPoint point = (GeoPoint) geoFrom.shape;
- double lat = point.getLatitude();
- double lon = point.getLongitude();
double dist = DistanceUtils.DEGREES_TO_RADIANS * distDEG;
double bearing = DistanceUtils.DEGREES_TO_RADIANS * bearingDEG;
-
--- End diff --
Yay
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr pull request #288: LUCENE-8086
Posted by iverase <gi...@git.apache.org>.
Github user iverase commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/288#discussion_r155844958
--- Diff: lucene/spatial-extras/src/test/org/apache/lucene/spatial/spatial4j/Geo3dShapeRectRelationTestCase.java ---
@@ -16,29 +16,15 @@
*/
package org.apache.lucene.spatial.spatial4j;
-import java.util.ArrayList;
-import java.util.List;
-
-import org.apache.lucene.spatial3d.geom.GeoPath;
-import org.apache.lucene.spatial3d.geom.GeoPolygon;
+import org.junit.Rule;
+import org.junit.Test;
import org.locationtech.spatial4j.TestLog;
import org.locationtech.spatial4j.context.SpatialContext;
-import org.locationtech.spatial4j.distance.DistanceUtils;
import org.locationtech.spatial4j.shape.Circle;
import org.locationtech.spatial4j.shape.Point;
import org.locationtech.spatial4j.shape.RectIntersectionTestHelper;
-import org.apache.lucene.spatial3d.geom.LatLonBounds;
--- End diff --
I didn't dear to change it but that was the idea of the effort, I will remove Geo3d.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr pull request #288: LUCENE-8086
Posted by iverase <gi...@git.apache.org>.
Github user iverase commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/288#discussion_r155843938
--- Diff: lucene/spatial-extras/src/java/org/apache/lucene/spatial/spatial4j/Geo3dShapeFactory.java ---
@@ -150,10 +176,21 @@ public Rectangle rect(double minX, double maxX, double minY, double maxY) {
@Override
public Circle circle(double x, double y, double distance) {
- GeoCircle circle = GeoCircleFactory.makeGeoCircle(planetModel,
- y * DistanceUtils.DEGREES_TO_RADIANS,
- x * DistanceUtils.DEGREES_TO_RADIANS,
- distance * DistanceUtils.DEGREES_TO_RADIANS);
+ GeoCircle circle;
+ if (planetModel.ab == planetModel.c) {
--- End diff --
I think spatial3d is a low level library in that respect, so it shouldn't have such a method. Karl Wright has the last word, comment would be fine
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr pull request #288: LUCENE-8086
Posted by iverase <gi...@git.apache.org>.
Github user iverase closed the pull request at:
https://github.com/apache/lucene-solr/pull/288
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr pull request #288: LUCENE-8086
Posted by iverase <gi...@git.apache.org>.
Github user iverase commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/288#discussion_r155844074
--- Diff: lucene/spatial-extras/src/java/org/apache/lucene/spatial/spatial4j/Geo3dShapeFactory.java ---
@@ -55,6 +55,13 @@
private SpatialContext context;
private PlanetModel planetModel;
+ /**
+ * Default accuracy for circles when not using the unit sphere.
+ * It is equivalent to 10m on the surface of the earth.
+ */
+ private static double DEFAULT_CIRCLE_ACCURACY = 1.6e-6;
--- End diff --
Indeed!
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org