You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by GitBox <gi...@apache.org> on 2020/02/12 18:04:55 UTC

[GitHub] [lucene-solr] nknize opened a new pull request #1253: LUCENE-9150: Restore support for dynamic PlanetModel in spatial3d

nknize opened a new pull request #1253: LUCENE-9150: Restore support for dynamic PlanetModel in spatial3d
URL: https://github.com/apache/lucene-solr/pull/1253
 
 
   This PR adds dynamic geographic datum support to Geo3D to make lucene a viable option for indexing/searching in different spatial reference systems (e.g., more accurately computing query shape relations to BKD's internal nodes using datum consistent with the spatial projection).

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] iverase commented on a change in pull request #1253: LUCENE-9150: Restore support for dynamic PlanetModel in spatial3d

Posted by GitBox <gi...@apache.org>.
iverase commented on a change in pull request #1253: LUCENE-9150: Restore support for dynamic PlanetModel in spatial3d
URL: https://github.com/apache/lucene-solr/pull/1253#discussion_r378914975
 
 

 ##########
 File path: lucene/spatial3d/src/test/org/apache/lucene/spatial3d/TestGeo3DPoint.java
 ##########
 @@ -84,6 +85,10 @@
 
 public class TestGeo3DPoint extends LuceneTestCase {
 
+  protected PlanetModel randomPlanetModel() {
+    return RandomPicks.randomFrom(random(), new PlanetModel[] {/*PlanetModel.WGS84,*/ PlanetModel.CLARKE_1866});
+  }
 
 Review comment:
   Can we add the three planet models in the random?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] iverase commented on a change in pull request #1253: LUCENE-9150: Restore support for dynamic PlanetModel in spatial3d

Posted by GitBox <gi...@apache.org>.
iverase commented on a change in pull request #1253: LUCENE-9150: Restore support for dynamic PlanetModel in spatial3d
URL: https://github.com/apache/lucene-solr/pull/1253#discussion_r378917756
 
 

 ##########
 File path: lucene/spatial3d/src/test/org/apache/lucene/spatial3d/geom/Geo3DUtil.java
 ##########
 @@ -16,89 +16,13 @@
  */
 package org.apache.lucene.spatial3d.geom;
 
-import org.apache.lucene.geo.Polygon;
-import org.apache.lucene.geo.GeoUtils;
-
-import java.util.List;
-import java.util.ArrayList;
-
 class Geo3DUtil {
 
-  /** How many radians are in one earth surface meter */
-  final static double RADIANS_PER_METER = 1.0 / PlanetModel.WGS84_MEAN;
   /** How many radians are in one degree */
   final static double RADIANS_PER_DEGREE = Math.PI / 180.0;
   /** How many degrees in a radian */
   final static double DEGREES_PER_RADIAN = 180.0 / Math.PI;
 
 Review comment:
   This was added because Math#toDegrees and Math#toRadians were in the forbidden APIs. I believe in master the van has been lifted (java 11) so this is not needed anymore. Not for this PR but good to have it in mind.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] nknize commented on a change in pull request #1253: LUCENE-9150: Restore support for dynamic PlanetModel in spatial3d

Posted by GitBox <gi...@apache.org>.
nknize commented on a change in pull request #1253: LUCENE-9150: Restore support for dynamic PlanetModel in spatial3d
URL: https://github.com/apache/lucene-solr/pull/1253#discussion_r386046274
 
 

 ##########
 File path: lucene/spatial3d/src/java/org/apache/lucene/spatial3d/geom/PlanetModel.java
 ##########
 @@ -118,71 +176,191 @@ public PlanetModel(final InputStream inputStream) throws IOException {
   
   @Override
   public void write(final OutputStream outputStream) throws IOException {
-    SerializableObject.writeDouble(outputStream, ab);
-    SerializableObject.writeDouble(outputStream, c);
+    SerializableObject.writeDouble(outputStream, xyScaling);
+    SerializableObject.writeDouble(outputStream, zScaling);
   }
   
   /** Does this planet model describe a sphere?
    *@return true if so.
    */
   public boolean isSphere() {
-    return this.ab == this.c;
+    return this.xyScaling == this.zScaling;
   }
   
   /** Find the minimum magnitude of all points on the ellipsoid.
    * @return the minimum magnitude for the planet.
    */
   public double getMinimumMagnitude() {
-    return Math.min(this.ab, this.c);
+    return Math.min(this.xyScaling, this.zScaling);
   }
 
   /** Find the maximum magnitude of all points on the ellipsoid.
    * @return the maximum magnitude for the planet.
    */
   public double getMaximumMagnitude() {
-    return Math.max(this.ab, this.c);
+    return Math.max(this.xyScaling, this.zScaling);
   }
   
   /** Find the minimum x value.
    *@return the minimum X value.
    */
   public double getMinimumXValue() {
-    return -this.ab;
+    return -this.xyScaling;
   }
   
   /** Find the maximum x value.
    *@return the maximum X value.
    */
   public double getMaximumXValue() {
-    return this.ab;
+    return this.xyScaling;
   }
 
   /** Find the minimum y value.
    *@return the minimum Y value.
    */
   public double getMinimumYValue() {
-    return -this.ab;
+    return -this.xyScaling;
   }
   
   /** Find the maximum y value.
    *@return the maximum Y value.
    */
   public double getMaximumYValue() {
-    return this.ab;
+    return this.xyScaling;
   }
   
   /** Find the minimum z value.
    *@return the minimum Z value.
    */
   public double getMinimumZValue() {
-    return -this.c;
+    return -this.zScaling;
   }
   
   /** Find the maximum z value.
    *@return the maximum Z value.
    */
   public double getMaximumZValue() {
-    return this.c;
+    return this.zScaling;
+  }
+
+  /** return the calculated mean radius (in meters) */
+  public double getMeanRadiusMeters() {
+    return this.r1;
+  }
+
+  /** encode the provided value from double to integer space */
+  public int encodeValue(double x) {
+    if (x > getMaximumMagnitude()) {
+      throw new IllegalArgumentException("value=" + x + " is out-of-bounds (greater than planetMax=" + getMaximumMagnitude() + ")");
+    }
+    if (x == getMaximumMagnitude()) {
+      x = Math.nextDown(x);
+    }
+    if (x < -getMaximumMagnitude()) {
+      throw new IllegalArgumentException("value=" + x + " is out-of-bounds (less than than -planetMax=" + -getMaximumMagnitude() + ")");
+    }
+    long result = (long) Math.floor(x / DECODE);
+    assert result >= Integer.MIN_VALUE;
+    assert result <= Integer.MAX_VALUE;
+    return (int) result;
+  }
+
+  /**
+   * Decodes a given integer back into the radian value according to the defined planet model
+   */
+  public double decodeValue(int x) {
+    double result;
+    if (x == MIN_ENCODED_VALUE) {
+      // We must special case this, because -MAX_VALUE is not guaranteed to land precisely at a floor value, and we don't ever want to
+      // return a value outside of the planet's range (I think?).  The max value is "safe" because we floor during encode:
+      result = -MAX_VALUE;
+    } else if (x == MAX_ENCODED_VALUE) {
+      result = MAX_VALUE;
+    } else {
+      // We decode to the center value; this keeps the encoding stable
+      result = (x+0.5) * DECODE;
+    }
+    assert result >= -MAX_VALUE && result <= MAX_VALUE;
+    return result;
+  }
+
+  /** Encode a provided GeoPoint into DocValue sortable integer space */
 
 Review comment:
   I added a getter for the docValueEncoder in PlanetModel

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] asfgit merged pull request #1253: LUCENE-9150: Restore support for dynamic PlanetModel in spatial3d

Posted by GitBox <gi...@apache.org>.
asfgit merged pull request #1253: LUCENE-9150: Restore support for dynamic PlanetModel in spatial3d
URL: https://github.com/apache/lucene-solr/pull/1253
 
 
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] iverase commented on a change in pull request #1253: LUCENE-9150: Restore support for dynamic PlanetModel in spatial3d

Posted by GitBox <gi...@apache.org>.
iverase commented on a change in pull request #1253: LUCENE-9150: Restore support for dynamic PlanetModel in spatial3d
URL: https://github.com/apache/lucene-solr/pull/1253#discussion_r378909350
 
 

 ##########
 File path: lucene/spatial3d/src/java/org/apache/lucene/spatial3d/Geo3DDocValuesField.java
 ##########
 @@ -478,9 +303,211 @@ public static SortField newOutsideLargePolygonSort(final String field, final Pol
    * @return SortField ordering documents by distance
    * @throws IllegalArgumentException if {@code field} is null or path has invalid coordinates.
    */
-  public static SortField newOutsidePathSort(final String field, final double[] pathLatitudes, final double[] pathLongitudes, final double pathWidthMeters) {
-    final GeoOutsideDistance shape = Geo3DUtil.fromPath(pathLatitudes, pathLongitudes, pathWidthMeters);
-    return new Geo3DPointOutsideSortField(field, shape);
+  public static SortField newOutsidePathSort(final String field, final double[] pathLatitudes, final double[] pathLongitudes, final double pathWidthMeters, final PlanetModel planetModel) {
+    final GeoOutsideDistance shape = Geo3DUtil.fromPath(planetModel, pathLatitudes, pathLongitudes, pathWidthMeters);
+    return new Geo3DPointOutsideSortField(field, planetModel, shape);
   }
 
+  /** Utility class for encoding / decoding from lat/lon (decimal degrees) into sortable doc value numerics (integers) */
+  public static class DocValueEncoder {
 
 Review comment:
   I think this class should be inside PlanetModel. We should not create any dependency in the geom package and if we keep this class here, then planet model has a decency outside.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] nknize commented on a change in pull request #1253: LUCENE-9150: Restore support for dynamic PlanetModel in spatial3d

Posted by GitBox <gi...@apache.org>.
nknize commented on a change in pull request #1253: LUCENE-9150: Restore support for dynamic PlanetModel in spatial3d
URL: https://github.com/apache/lucene-solr/pull/1253#discussion_r386046242
 
 

 ##########
 File path: lucene/spatial3d/src/java/org/apache/lucene/spatial3d/Geo3DDocValuesField.java
 ##########
 @@ -478,9 +303,211 @@ public static SortField newOutsideLargePolygonSort(final String field, final Pol
    * @return SortField ordering documents by distance
    * @throws IllegalArgumentException if {@code field} is null or path has invalid coordinates.
    */
-  public static SortField newOutsidePathSort(final String field, final double[] pathLatitudes, final double[] pathLongitudes, final double pathWidthMeters) {
-    final GeoOutsideDistance shape = Geo3DUtil.fromPath(pathLatitudes, pathLongitudes, pathWidthMeters);
-    return new Geo3DPointOutsideSortField(field, shape);
+  public static SortField newOutsidePathSort(final String field, final double[] pathLatitudes, final double[] pathLongitudes, final double pathWidthMeters, final PlanetModel planetModel) {
+    final GeoOutsideDistance shape = Geo3DUtil.fromPath(planetModel, pathLatitudes, pathLongitudes, pathWidthMeters);
+    return new Geo3DPointOutsideSortField(field, planetModel, shape);
   }
 
+  /** Utility class for encoding / decoding from lat/lon (decimal degrees) into sortable doc value numerics (integers) */
+  public static class DocValueEncoder {
 
 Review comment:
   +1

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] nknize commented on a change in pull request #1253: LUCENE-9150: Restore support for dynamic PlanetModel in spatial3d

Posted by GitBox <gi...@apache.org>.
nknize commented on a change in pull request #1253: LUCENE-9150: Restore support for dynamic PlanetModel in spatial3d
URL: https://github.com/apache/lucene-solr/pull/1253#discussion_r386046244
 
 

 ##########
 File path: lucene/spatial3d/src/java/org/apache/lucene/spatial3d/geom/PlanetModel.java
 ##########
 @@ -20,6 +20,8 @@
 import java.io.OutputStream;
 import java.io.IOException;
 
+import org.apache.lucene.spatial3d.Geo3DDocValuesField.DocValueEncoder;
+
 
 Review comment:
   +1

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] iverase commented on a change in pull request #1253: LUCENE-9150: Restore support for dynamic PlanetModel in spatial3d

Posted by GitBox <gi...@apache.org>.
iverase commented on a change in pull request #1253: LUCENE-9150: Restore support for dynamic PlanetModel in spatial3d
URL: https://github.com/apache/lucene-solr/pull/1253#discussion_r386092274
 
 

 ##########
 File path: lucene/spatial3d/src/java/org/apache/lucene/spatial3d/geom/PlanetModel.java
 ##########
 @@ -383,30 +509,233 @@ public GeoPoint surfacePointOnBearing(final GeoPoint from, final double dist, fi
       Δσ = B * sinσ * (cos2σM + B / 4.0 * (cosσ * (-1.0 + 2.0 * cos2σM * cos2σM) -
           B / 6.0 * cos2σM * (-3.0 + 4.0 * sinσ * sinσ) * (-3.0 + 4.0 * cos2σM * cos2σM)));
       σʹ = σ;
-      σ = dist / (c * inverseScale * A) + Δσ;
+      σ = dist / (zScaling * inverseScale * A) + Δσ;
     } while (Math.abs(σ - σʹ) >= Vector.MINIMUM_RESOLUTION && ++iterations < 100);
     double x = sinU1 * sinσ - cosU1 * cosσ * cosα1;
-    double φ2 = Math.atan2(sinU1 * cosσ + cosU1 * sinσ * cosα1, (1.0 - flattening) * Math.sqrt(sinα * sinα + x * x));
+    double φ2 = Math.atan2(sinU1 * cosσ + cosU1 * sinσ * cosα1, (1.0 - scaledFlattening) * Math.sqrt(sinα * sinα + x * x));
     double λ = Math.atan2(sinσ * sinα1, cosU1 * cosσ - sinU1 * sinσ * cosα1);
-    double C = flattening / 16.0 * cosSqα * (4.0 + flattening * (4.0 - 3.0 * cosSqα));
-    double L = λ - (1.0 - C) * flattening * sinα *
+    double C = scaledFlattening / 16.0 * cosSqα * (4.0 + scaledFlattening * (4.0 - 3.0 * cosSqα));
+    double L = λ - (1.0 - C) * scaledFlattening * sinα *
         (σ + C * sinσ * (cos2σM + C * cosσ * (-1.0 + 2.0 * cos2σM * cos2σM)));
     double λ2 = (lon + L + 3.0 * Math.PI) % (2.0 * Math.PI) - Math.PI;  // normalise to -180..+180
 
     return new GeoPoint(this, φ2, λ2);
   }
 
+  /** Utility class for encoding / decoding from lat/lon (decimal degrees) into sortable doc value numerics (integers) */
+  public static class DocValueEncoder {
+    private final PlanetModel planetModel;
+
+    // These are the multiplicative constants we need to use to arrive at values that fit in 21 bits.
+    // The formula we use to go from double to encoded value is:  Math.floor((value - minimum) * factor + 0.5)
+    // If we plug in maximum for value, we should get 0x1FFFFF.
+    // So, 0x1FFFFF = Math.floor((maximum - minimum) * factor + 0.5)
+    // We factor out the 0.5 and Math.floor by stating instead:
+    // 0x1FFFFF = (maximum - minimum) * factor
+    // So, factor = 0x1FFFFF / (maximum - minimum)
+
+    private final static double inverseMaximumValue = 1.0 / (double)(0x1FFFFF);
+
+    private final double inverseXFactor;
+    private final double inverseYFactor;
+    private final double inverseZFactor;
+
+    private final double xFactor;
+    private final double yFactor;
+    private final double zFactor;
+
+    // Fudge factor for step adjustments.  This is here solely to handle inaccuracies in bounding boxes
+    // that occur because of quantization.  For unknown reasons, the fudge factor needs to be
+    // 10.0 rather than 1.0.  See LUCENE-7430.
+
+    private final static double STEP_FUDGE = 10.0;
+
+    // These values are the delta between a value and the next value in each specific dimension
+
+    private final double xStep;
+    private final double yStep;
+    private final double zStep;
+
+    /** construct an encoder/decoder instance from the provided PlanetModel definition */
+    public DocValueEncoder(final PlanetModel planetModel) {
 
 Review comment:
   make constructor private?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] nknize commented on a change in pull request #1253: LUCENE-9150: Restore support for dynamic PlanetModel in spatial3d

Posted by GitBox <gi...@apache.org>.
nknize commented on a change in pull request #1253: LUCENE-9150: Restore support for dynamic PlanetModel in spatial3d
URL: https://github.com/apache/lucene-solr/pull/1253#discussion_r386046464
 
 

 ##########
 File path: lucene/spatial3d/src/test/org/apache/lucene/spatial3d/TestGeo3DPoint.java
 ##########
 @@ -84,6 +85,10 @@
 
 public class TestGeo3DPoint extends LuceneTestCase {
 
+  protected PlanetModel randomPlanetModel() {
+    return RandomPicks.randomFrom(random(), new PlanetModel[] {/*PlanetModel.WGS84,*/ PlanetModel.CLARKE_1866});
+  }
 
 Review comment:
   +1

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] iverase commented on a change in pull request #1253: LUCENE-9150: Restore support for dynamic PlanetModel in spatial3d

Posted by GitBox <gi...@apache.org>.
iverase commented on a change in pull request #1253: LUCENE-9150: Restore support for dynamic PlanetModel in spatial3d
URL: https://github.com/apache/lucene-solr/pull/1253#discussion_r378913539
 
 

 ##########
 File path: lucene/spatial3d/src/java/org/apache/lucene/spatial3d/geom/PlanetModel.java
 ##########
 @@ -20,6 +20,8 @@
 import java.io.OutputStream;
 import java.io.IOException;
 
+import org.apache.lucene.spatial3d.Geo3DDocValuesField.DocValueEncoder;
+
 
 Review comment:
   As I said before, we should not have a dependency outside of this package.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] iverase commented on a change in pull request #1253: LUCENE-9150: Restore support for dynamic PlanetModel in spatial3d

Posted by GitBox <gi...@apache.org>.
iverase commented on a change in pull request #1253: LUCENE-9150: Restore support for dynamic PlanetModel in spatial3d
URL: https://github.com/apache/lucene-solr/pull/1253#discussion_r378914442
 
 

 ##########
 File path: lucene/spatial3d/src/java/org/apache/lucene/spatial3d/geom/PlanetModel.java
 ##########
 @@ -118,71 +176,191 @@ public PlanetModel(final InputStream inputStream) throws IOException {
   
   @Override
   public void write(final OutputStream outputStream) throws IOException {
-    SerializableObject.writeDouble(outputStream, ab);
-    SerializableObject.writeDouble(outputStream, c);
+    SerializableObject.writeDouble(outputStream, xyScaling);
+    SerializableObject.writeDouble(outputStream, zScaling);
   }
   
   /** Does this planet model describe a sphere?
    *@return true if so.
    */
   public boolean isSphere() {
-    return this.ab == this.c;
+    return this.xyScaling == this.zScaling;
   }
   
   /** Find the minimum magnitude of all points on the ellipsoid.
    * @return the minimum magnitude for the planet.
    */
   public double getMinimumMagnitude() {
-    return Math.min(this.ab, this.c);
+    return Math.min(this.xyScaling, this.zScaling);
   }
 
   /** Find the maximum magnitude of all points on the ellipsoid.
    * @return the maximum magnitude for the planet.
    */
   public double getMaximumMagnitude() {
-    return Math.max(this.ab, this.c);
+    return Math.max(this.xyScaling, this.zScaling);
   }
   
   /** Find the minimum x value.
    *@return the minimum X value.
    */
   public double getMinimumXValue() {
-    return -this.ab;
+    return -this.xyScaling;
   }
   
   /** Find the maximum x value.
    *@return the maximum X value.
    */
   public double getMaximumXValue() {
-    return this.ab;
+    return this.xyScaling;
   }
 
   /** Find the minimum y value.
    *@return the minimum Y value.
    */
   public double getMinimumYValue() {
-    return -this.ab;
+    return -this.xyScaling;
   }
   
   /** Find the maximum y value.
    *@return the maximum Y value.
    */
   public double getMaximumYValue() {
-    return this.ab;
+    return this.xyScaling;
   }
   
   /** Find the minimum z value.
    *@return the minimum Z value.
    */
   public double getMinimumZValue() {
-    return -this.c;
+    return -this.zScaling;
   }
   
   /** Find the maximum z value.
    *@return the maximum Z value.
    */
   public double getMaximumZValue() {
-    return this.c;
+    return this.zScaling;
+  }
+
+  /** return the calculated mean radius (in meters) */
+  public double getMeanRadiusMeters() {
+    return this.r1;
+  }
+
+  /** encode the provided value from double to integer space */
+  public int encodeValue(double x) {
+    if (x > getMaximumMagnitude()) {
+      throw new IllegalArgumentException("value=" + x + " is out-of-bounds (greater than planetMax=" + getMaximumMagnitude() + ")");
+    }
+    if (x == getMaximumMagnitude()) {
+      x = Math.nextDown(x);
+    }
+    if (x < -getMaximumMagnitude()) {
+      throw new IllegalArgumentException("value=" + x + " is out-of-bounds (less than than -planetMax=" + -getMaximumMagnitude() + ")");
+    }
+    long result = (long) Math.floor(x / DECODE);
+    assert result >= Integer.MIN_VALUE;
+    assert result <= Integer.MAX_VALUE;
+    return (int) result;
+  }
+
+  /**
+   * Decodes a given integer back into the radian value according to the defined planet model
+   */
+  public double decodeValue(int x) {
+    double result;
+    if (x == MIN_ENCODED_VALUE) {
+      // We must special case this, because -MAX_VALUE is not guaranteed to land precisely at a floor value, and we don't ever want to
+      // return a value outside of the planet's range (I think?).  The max value is "safe" because we floor during encode:
+      result = -MAX_VALUE;
+    } else if (x == MAX_ENCODED_VALUE) {
+      result = MAX_VALUE;
+    } else {
+      // We decode to the center value; this keeps the encoding stable
+      result = (x+0.5) * DECODE;
+    }
+    assert result >= -MAX_VALUE && result <= MAX_VALUE;
+    return result;
+  }
+
+  /** Encode a provided GeoPoint into DocValue sortable integer space */
 
 Review comment:
   I wonder if instead of adding all those methods, the planet model should just return the docValueEncoder?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] nknize commented on a change in pull request #1253: LUCENE-9150: Restore support for dynamic PlanetModel in spatial3d

Posted by GitBox <gi...@apache.org>.
nknize commented on a change in pull request #1253: LUCENE-9150: Restore support for dynamic PlanetModel in spatial3d
URL: https://github.com/apache/lucene-solr/pull/1253#discussion_r386676232
 
 

 ##########
 File path: lucene/spatial3d/src/java/org/apache/lucene/spatial3d/geom/PlanetModel.java
 ##########
 @@ -383,30 +509,233 @@ public GeoPoint surfacePointOnBearing(final GeoPoint from, final double dist, fi
       Δσ = B * sinσ * (cos2σM + B / 4.0 * (cosσ * (-1.0 + 2.0 * cos2σM * cos2σM) -
           B / 6.0 * cos2σM * (-3.0 + 4.0 * sinσ * sinσ) * (-3.0 + 4.0 * cos2σM * cos2σM)));
       σʹ = σ;
-      σ = dist / (c * inverseScale * A) + Δσ;
+      σ = dist / (zScaling * inverseScale * A) + Δσ;
     } while (Math.abs(σ - σʹ) >= Vector.MINIMUM_RESOLUTION && ++iterations < 100);
     double x = sinU1 * sinσ - cosU1 * cosσ * cosα1;
-    double φ2 = Math.atan2(sinU1 * cosσ + cosU1 * sinσ * cosα1, (1.0 - flattening) * Math.sqrt(sinα * sinα + x * x));
+    double φ2 = Math.atan2(sinU1 * cosσ + cosU1 * sinσ * cosα1, (1.0 - scaledFlattening) * Math.sqrt(sinα * sinα + x * x));
     double λ = Math.atan2(sinσ * sinα1, cosU1 * cosσ - sinU1 * sinσ * cosα1);
-    double C = flattening / 16.0 * cosSqα * (4.0 + flattening * (4.0 - 3.0 * cosSqα));
-    double L = λ - (1.0 - C) * flattening * sinα *
+    double C = scaledFlattening / 16.0 * cosSqα * (4.0 + scaledFlattening * (4.0 - 3.0 * cosSqα));
+    double L = λ - (1.0 - C) * scaledFlattening * sinα *
         (σ + C * sinσ * (cos2σM + C * cosσ * (-1.0 + 2.0 * cos2σM * cos2σM)));
     double λ2 = (lon + L + 3.0 * Math.PI) % (2.0 * Math.PI) - Math.PI;  // normalise to -180..+180
 
     return new GeoPoint(this, φ2, λ2);
   }
 
+  /** Utility class for encoding / decoding from lat/lon (decimal degrees) into sortable doc value numerics (integers) */
+  public static class DocValueEncoder {
+    private final PlanetModel planetModel;
+
+    // These are the multiplicative constants we need to use to arrive at values that fit in 21 bits.
+    // The formula we use to go from double to encoded value is:  Math.floor((value - minimum) * factor + 0.5)
+    // If we plug in maximum for value, we should get 0x1FFFFF.
+    // So, 0x1FFFFF = Math.floor((maximum - minimum) * factor + 0.5)
+    // We factor out the 0.5 and Math.floor by stating instead:
+    // 0x1FFFFF = (maximum - minimum) * factor
+    // So, factor = 0x1FFFFF / (maximum - minimum)
+
+    private final static double inverseMaximumValue = 1.0 / (double)(0x1FFFFF);
+
+    private final double inverseXFactor;
+    private final double inverseYFactor;
+    private final double inverseZFactor;
+
+    private final double xFactor;
+    private final double yFactor;
+    private final double zFactor;
+
+    // Fudge factor for step adjustments.  This is here solely to handle inaccuracies in bounding boxes
+    // that occur because of quantization.  For unknown reasons, the fudge factor needs to be
+    // 10.0 rather than 1.0.  See LUCENE-7430.
+
+    private final static double STEP_FUDGE = 10.0;
+
+    // These values are the delta between a value and the next value in each specific dimension
+
+    private final double xStep;
+    private final double yStep;
+    private final double zStep;
+
+    /** construct an encoder/decoder instance from the provided PlanetModel definition */
+    public DocValueEncoder(final PlanetModel planetModel) {
 
 Review comment:
   :+1: good call! 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org