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 2022/11/24 09:47:23 UTC

[lucene] branch main updated: More refactoring work, and fix a distance calculation.

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 <kw...@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);


Re: [lucene] branch main updated: More refactoring work, and fix a distance calculation.

Posted by Karl Wright <da...@gmail.com>.
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 <jp...@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 <kw...@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 <kw...@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-unsubscribe@lucene.apache.org
> For additional commands, e-mail: dev-help@lucene.apache.org
>
>

Re: [lucene] branch main updated: More refactoring work, and fix a distance calculation.

Posted by Adrien Grand <jp...@gmail.com>.
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 <kw...@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 <kw...@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-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org