Thanks, I was unable to get to this until this morning. The code was dead because the corresponding call hadn't been included. Fixed now.
Karl On Thu, Nov 24, 2022 at 5:50 AM Adrien Grand <jpou...@gmail.com> wrote: > 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 > >