Karl, this commit has been failing precommit because it introduced dead code. I just pushed a fix.
On Thu, Nov 24, 2022 at 10:47 AM <kwri...@apache.org> wrote: > > This is an automated email from the ASF dual-hosted git repository. > > kwright pushed a commit to branch main > in repository https://gitbox.apache.org/repos/asf/lucene.git > > > The following commit(s) were added to refs/heads/main by this push: > new 839dfb5a2dc More refactoring work, and fix a distance calculation. > 839dfb5a2dc is described below > > commit 839dfb5a2dc46c4b2d16d9db5ea9f31ca1e8d907 > Author: Karl David Wright <kwri...@apache.org> > AuthorDate: Wed Nov 23 23:36:15 2022 -0500 > > More refactoring work, and fix a distance calculation. > --- > .../lucene/spatial3d/geom/GeoDegeneratePath.java | 32 ++++++++++--- > .../lucene/spatial3d/geom/GeoStandardPath.java | 54 > ++++++++++++---------- > .../apache/lucene/spatial3d/geom/TestGeoPath.java | 12 +++-- > 3 files changed, 62 insertions(+), 36 deletions(-) > > diff --git > a/lucene/spatial3d/src/java/org/apache/lucene/spatial3d/geom/GeoDegeneratePath.java > > b/lucene/spatial3d/src/java/org/apache/lucene/spatial3d/geom/GeoDegeneratePath.java > index 524451ac68a..d1a452ca566 100644 > --- > a/lucene/spatial3d/src/java/org/apache/lucene/spatial3d/geom/GeoDegeneratePath.java > +++ > b/lucene/spatial3d/src/java/org/apache/lucene/spatial3d/geom/GeoDegeneratePath.java > @@ -282,7 +282,7 @@ class GeoDegeneratePath extends GeoBasePath { > minDistance = newDistance; > } > } > - return minDistance; > + return distanceStyle.fromAggregationForm(minDistance); > } > > @Override > @@ -468,6 +468,15 @@ class GeoDegeneratePath extends GeoBasePath { > return this.point.isIdentical(x, y, z); > } > > + public boolean isWithinSection(final double x, final double y, final > double z) { > + for (final Membership cutoffPlane : cutoffPlanes) { > + if (!cutoffPlane.isWithin(x, y, z)) { > + return false; > + } > + } > + return true; > + } > + > /** > * Compute interior path distance. > * > @@ -502,7 +511,7 @@ class GeoDegeneratePath extends GeoBasePath { > return Double.POSITIVE_INFINITY; > } > } > - return distanceStyle.computeDistance(this.point, x, y, z); > + return > distanceStyle.toAggregationForm(distanceStyle.computeDistance(this.point, x, > y, z)); > } > > /** > @@ -516,7 +525,7 @@ class GeoDegeneratePath extends GeoBasePath { > */ > public double outsideDistance( > final DistanceStyle distanceStyle, final double x, final double y, > final double z) { > - return distanceStyle.computeDistance(this.point, x, y, z); > + return > distanceStyle.toAggregationForm(distanceStyle.computeDistance(this.point, x, > y, z)); > } > > /** > @@ -578,7 +587,7 @@ class GeoDegeneratePath extends GeoBasePath { > > @Override > public String toString() { > - return point.toString(); > + return "SegmentEndpoint: " + point; > } > } > > @@ -659,6 +668,10 @@ class GeoDegeneratePath extends GeoBasePath { > && normalizedConnectingPlane.evaluateIsZero(x, y, z); > } > > + public boolean isWithinSection(final double x, final double y, final > double z) { > + return startCutoffPlane.isWithin(x, y, z) && > endCutoffPlane.isWithin(x, y, z); > + } > + > /** > * Compute path center distance (distance from path to current point). > * > @@ -671,7 +684,7 @@ class GeoDegeneratePath extends GeoBasePath { > public double pathCenterDistance( > final DistanceStyle distanceStyle, final double x, final double y, > final double z) { > // First, if this point is outside the endplanes of the segment, > return POSITIVE_INFINITY. > - if (!startCutoffPlane.isWithin(x, y, z) || !endCutoffPlane.isWithin(x, > y, z)) { > + if (!isWithinSection(x, y, z)) { > return Double.POSITIVE_INFINITY; > } > // (1) Compute normalizedPerpPlane. If degenerate, then there is no > such plane, which means > @@ -710,7 +723,7 @@ class GeoDegeneratePath extends GeoBasePath { > "Can't find world intersection for point x=" + x + " y=" + y + > " z=" + z); > } > } > - return distanceStyle.computeDistance(thePoint, x, y, z); > + return > distanceStyle.toAggregationForm(distanceStyle.computeDistance(thePoint, x, y, > z)); > } > > /** > @@ -726,7 +739,7 @@ class GeoDegeneratePath extends GeoBasePath { > public double nearestPathDistance( > final DistanceStyle distanceStyle, final double x, final double y, > final double z) { > // First, if this point is outside the endplanes of the segment, > return POSITIVE_INFINITY. > - if (!startCutoffPlane.isWithin(x, y, z) || !endCutoffPlane.isWithin(x, > y, z)) { > + if (!isWithinSection(x, y, z)) { > return Double.POSITIVE_INFINITY; > } > // (1) Compute normalizedPerpPlane. If degenerate, then there is no > such plane, which means > @@ -892,5 +905,10 @@ class GeoDegeneratePath extends GeoBasePath { > .addPoint(end) > .addPlane(planetModel, normalizedConnectingPlane, > startCutoffPlane, endCutoffPlane); > } > + > + @Override > + public String toString() { > + return "PathSegment: " + start + " to " + end; > + } > } > } > diff --git > a/lucene/spatial3d/src/java/org/apache/lucene/spatial3d/geom/GeoStandardPath.java > > b/lucene/spatial3d/src/java/org/apache/lucene/spatial3d/geom/GeoStandardPath.java > index 9b46e3a63b0..250387d275c 100755 > --- > a/lucene/spatial3d/src/java/org/apache/lucene/spatial3d/geom/GeoStandardPath.java > +++ > b/lucene/spatial3d/src/java/org/apache/lucene/spatial3d/geom/GeoStandardPath.java > @@ -436,6 +436,14 @@ class GeoStandardPath extends GeoBasePath { > this.pathCenterDistance = pathCenterDistance; > this.distanceAlongPath = distanceAlongPath; > } > + > + @Override > + public String toString() { > + return "DistancePair: pathCenterDistance=" > + + pathCenterDistance > + + ",distanceAlongPath=" > + + distanceAlongPath; > + } > } > > /** > @@ -887,10 +895,12 @@ class GeoStandardPath extends GeoBasePath { > if (!isWithinSection(x, y, z)) { > return null; > } > - return new DistancePair( > - pathCenterDistance(distanceStyle, x, y, z), > - distanceStyle.aggregateDistances( > - getStartingDistance(distanceStyle), > nearestPathDistance(distanceStyle, x, y, z))); > + final DistancePair rval = > + new DistancePair( > + pathCenterDistance(distanceStyle, x, y, z), > + distanceStyle.aggregateDistances( > + getStartingDistance(distanceStyle), > nearestPathDistance(distanceStyle, x, y, z))); > + return rval; > } > > @Override > @@ -924,7 +934,7 @@ class GeoStandardPath extends GeoBasePath { > if (!isWithinSection(x, y, z)) { > return Double.POSITIVE_INFINITY; > } > - return > distanceStyle.toAggregationForm(distanceStyle.computeDistance(this.point, x, > y, z)); > + return distanceStyle.toAggregationForm(0.0); > } > > @Override > @@ -1149,10 +1159,8 @@ class GeoStandardPath extends GeoBasePath { > @Override > public double nearestPathDistance( > final DistanceStyle distanceStyle, final double x, final double y, > final double z) { > - for (final Membership cutoff : cutoffPlanes) { > - if (!cutoff.isWithin(x, y, z)) { > - return Double.POSITIVE_INFINITY; > - } > + if (!isWithinSection(x, y, z)) { > + return Double.POSITIVE_INFINITY; > } > return super.nearestPathDistance(distanceStyle, x, y, z); > } > @@ -1160,10 +1168,8 @@ class GeoStandardPath extends GeoBasePath { > @Override > public double pathCenterDistance( > final DistanceStyle distanceStyle, final double x, final double y, > final double z) { > - for (final Membership cutoff : cutoffPlanes) { > - if (!cutoff.isWithin(x, y, z)) { > - return Double.POSITIVE_INFINITY; > - } > + if (!isWithinSection(x, y, z)) { > + return Double.POSITIVE_INFINITY; > } > return super.pathCenterDistance(distanceStyle, x, y, z); > } > @@ -1318,10 +1324,8 @@ class GeoStandardPath extends GeoBasePath { > @Override > public double nearestPathDistance( > final DistanceStyle distanceStyle, final double x, final double y, > final double z) { > - for (final Membership cutoff : cutoffPlanes) { > - if (!cutoff.isWithin(x, y, z)) { > - return Double.POSITIVE_INFINITY; > - } > + if (!isWithinSection(x, y, z)) { > + return Double.POSITIVE_INFINITY; > } > return super.nearestPathDistance(distanceStyle, x, y, z); > } > @@ -1329,10 +1333,8 @@ class GeoStandardPath extends GeoBasePath { > @Override > public double pathCenterDistance( > final DistanceStyle distanceStyle, final double x, final double y, > final double z) { > - for (final Membership cutoff : cutoffPlanes) { > - if (!cutoff.isWithin(x, y, z)) { > - return Double.POSITIVE_INFINITY; > - } > + if (!isWithinSection(x, y, z)) { > + return Double.POSITIVE_INFINITY; > } > return super.pathCenterDistance(distanceStyle, x, y, z); > } > @@ -1537,10 +1539,12 @@ class GeoStandardPath extends GeoBasePath { > if (!isWithinSection(x, y, z)) { > return null; > } > - return new DistancePair( > - pathCenterDistance(distanceStyle, x, y, z), > - distanceStyle.aggregateDistances( > - getStartingDistance(distanceStyle), > nearestPathDistance(distanceStyle, x, y, z))); > + final DistancePair rval = > + new DistancePair( > + pathCenterDistance(distanceStyle, x, y, z), > + distanceStyle.aggregateDistances( > + getStartingDistance(distanceStyle), > nearestPathDistance(distanceStyle, x, y, z))); > + return rval; > } > > private double computeStartingDistance(final DistanceStyle > distanceStyle) { > diff --git > a/lucene/spatial3d/src/test/org/apache/lucene/spatial3d/geom/TestGeoPath.java > b/lucene/spatial3d/src/test/org/apache/lucene/spatial3d/geom/TestGeoPath.java > index ccafbae3574..f93466bef61 100755 > --- > a/lucene/spatial3d/src/test/org/apache/lucene/spatial3d/geom/TestGeoPath.java > +++ > b/lucene/spatial3d/src/test/org/apache/lucene/spatial3d/geom/TestGeoPath.java > @@ -40,7 +40,6 @@ public class TestGeoPath extends LuceneTestCase { > gp = new GeoPoint(PlanetModel.SPHERE, -0.15, 0.05); > assertEquals(Double.POSITIVE_INFINITY, > p.computeDistance(DistanceStyle.ARC, gp), 0.000001); > gp = new GeoPoint(PlanetModel.SPHERE, 0.0, 0.25); > - System.out.println("Calling problematic computeDistance..."); > assertEquals(0.20 + 0.05, p.computeDistance(DistanceStyle.ARC, gp), > 0.000001); > gp = new GeoPoint(PlanetModel.SPHERE, 0.0, -0.05); > assertEquals(0.0 + 0.05, p.computeDistance(DistanceStyle.ARC, gp), > 0.000001); > @@ -415,9 +414,9 @@ public class TestGeoPath extends LuceneTestCase { > // Construct a path with a width > final GeoPath legacyPath = > GeoPathFactory.makeGeoPath(PlanetModel.SPHERE, 1e-6, pathPoints); > // Compute the inside distance to the atPoint using zero-width path > - // final double distance = > thisPath.computeNearestDistance(DistanceStyle.ARC, carPoint); > + final double distance = > thisPath.computeNearestDistance(DistanceStyle.ARC, carPoint); > // Compute the inside distance using legacy path > - // final double legacyDistance = > legacyPath.computeNearestDistance(DistanceStyle.ARC, carPoint); > + final double legacyDistance = > legacyPath.computeNearestDistance(DistanceStyle.ARC, carPoint); > // Compute the inside distance using the legacy formula > final double oldFormulaDistance = > thisPath.computeDistance(DistanceStyle.ARC, carPoint); > // Compute the inside distance using the legacy formula with the legacy > shape > @@ -426,7 +425,12 @@ public class TestGeoPath extends LuceneTestCase { > // These should be about the same, but something is wrong with > GeoDegeneratePath and they > // aren't. > // More research needed. > - // assertEquals(legacyDistance, distance, 1e-12); > + System.out.println( > + "Zero width path nearestDistance = " > + + distance > + + ", standard path nearestDistance = " > + + legacyDistance); > + assertEquals(legacyDistance, distance, 1e-12); > assertEquals(oldFormulaLegacyDistance, oldFormulaDistance, 1e-12); > // This isn't true because example search center is off of the path. > // assertEquals(oldFormulaDistance, distance, 1e-12); > -- Adrien --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org