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

Reply via email to