Browse Source

Reduce GeoDistance insanity

GeoDistance query, sort, and scripts make use of a crazy GeoDistance enum for handling 4 different ways of computing geo distance: SLOPPY_ARC, ARC, FACTOR, and PLANE. Only two of these are necessary: ARC, PLANE. This commit removes SLOPPY_ARC, and FACTOR and cleans up the way Geo distance is computed.
Nicholas Knize 9 years ago
parent
commit
b41d5747f0

+ 9 - 368
core/src/main/java/org/elasticsearch/common/geo/GeoDistance.java

@@ -19,18 +19,10 @@
 
 package org.elasticsearch.common.geo;
 
-import org.apache.lucene.util.Bits;
-import org.apache.lucene.util.SloppyMath;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.common.io.stream.Writeable;
 import org.elasticsearch.common.unit.DistanceUnit;
-import org.elasticsearch.index.fielddata.FieldData;
-import org.elasticsearch.index.fielddata.GeoPointValues;
-import org.elasticsearch.index.fielddata.MultiGeoPointValues;
-import org.elasticsearch.index.fielddata.NumericDoubleValues;
-import org.elasticsearch.index.fielddata.SortedNumericDoubleValues;
-import org.elasticsearch.index.fielddata.SortingNumericDoubleValues;
 
 import java.io.IOException;
 import java.util.Locale;
@@ -39,101 +31,9 @@ import java.util.Locale;
  * Geo distance calculation.
  */
 public enum GeoDistance implements Writeable {
-    /**
-     * Calculates distance as points on a plane. Faster, but less accurate than {@link #ARC}.
-     * @deprecated use {@link GeoUtils#planeDistance}
-     */
-    @Deprecated
-    PLANE {
-        @Override
-        public double calculate(double sourceLatitude, double sourceLongitude, double targetLatitude, double targetLongitude, DistanceUnit unit) {
-            double px = targetLongitude - sourceLongitude;
-            double py = targetLatitude - sourceLatitude;
-            return Math.sqrt(px * px + py * py) * unit.getDistancePerDegree();
-        }
-
-        @Override
-        public double normalize(double distance, DistanceUnit unit) {
-            return distance;
-        }
-
-        @Override
-        public FixedSourceDistance fixedSourceDistance(double sourceLatitude, double sourceLongitude, DistanceUnit unit) {
-            return new PlaneFixedSourceDistance(sourceLatitude, sourceLongitude, unit);
-        }
-    },
-
-    /**
-     * Calculates distance factor.
-     * Note: {@code calculate} is simply returning the RHS of the spherical law of cosines from 2 lat,lon points.
-     * {@code normalize} also returns the RHS of the spherical law of cosines for a given distance
-     * @deprecated use {@link SloppyMath#haversinMeters} to get distance in meters, law of cosines is being removed
-     */
-    @Deprecated
-    FACTOR {
-        @Override
-        public double calculate(double sourceLatitude, double sourceLongitude, double targetLatitude, double targetLongitude, DistanceUnit unit) {
-            double longitudeDifference = targetLongitude - sourceLongitude;
-            double a = Math.toRadians(90D - sourceLatitude);
-            double c = Math.toRadians(90D - targetLatitude);
-            return (Math.cos(a) * Math.cos(c)) + (Math.sin(a) * Math.sin(c) * Math.cos(Math.toRadians(longitudeDifference)));
-        }
-
-        @Override
-        public double normalize(double distance, DistanceUnit unit) {
-            return Math.cos(distance / unit.getEarthRadius());
-        }
-
-        @Override
-        public FixedSourceDistance fixedSourceDistance(double sourceLatitude, double sourceLongitude, DistanceUnit unit) {
-            return new FactorFixedSourceDistance(sourceLatitude, sourceLongitude);
-        }
-    },
-    /**
-     * Calculates distance as points on a globe.
-     * @deprecated use {@link GeoUtils#arcDistance}
-     */
-    @Deprecated
-    ARC {
-        @Override
-        public double calculate(double sourceLatitude, double sourceLongitude, double targetLatitude, double targetLongitude, DistanceUnit unit) {
-            double result = SloppyMath.haversinMeters(sourceLatitude, sourceLongitude, targetLatitude, targetLongitude);
-            return unit.fromMeters(result);
-        }
-
-        @Override
-        public double normalize(double distance, DistanceUnit unit) {
-            return distance;
-        }
-
-        @Override
-        public FixedSourceDistance fixedSourceDistance(double sourceLatitude, double sourceLongitude, DistanceUnit unit) {
-            return new ArcFixedSourceDistance(sourceLatitude, sourceLongitude, unit);
-        }
-    },
-    /**
-     * Calculates distance as points on a globe in a sloppy way. Close to the pole areas the accuracy
-     * of this function decreases.
-     */
-    @Deprecated
-    SLOPPY_ARC {
-
-        @Override
-        public double normalize(double distance, DistanceUnit unit) {
-            return distance;
-        }
-
-        @Override
-        public double calculate(double sourceLatitude, double sourceLongitude, double targetLatitude, double targetLongitude, DistanceUnit unit) {
-            return unit.fromMeters(SloppyMath.haversinMeters(sourceLatitude, sourceLongitude, targetLatitude, targetLongitude));
-        }
-
-        @Override
-        public FixedSourceDistance fixedSourceDistance(double sourceLatitude, double sourceLongitude, DistanceUnit unit) {
-            return new SloppyArcFixedSourceDistance(sourceLatitude, sourceLongitude, unit);
-        }
-    };
+    PLANE, ARC;
 
+    /** Creates a GeoDistance instance from an input stream */
     public static GeoDistance readFromStream(StreamInput in) throws IOException {
         int ord = in.readVInt();
         if (ord < 0 || ord >= values().length) {
@@ -142,70 +42,17 @@ public enum GeoDistance implements Writeable {
         return GeoDistance.values()[ord];
     }
 
+    /** Writes an instance of a GeoDistance object to an output stream */
     @Override
     public void writeTo(StreamOutput out) throws IOException {
         out.writeVInt(this.ordinal());
     }
 
-    /**
-     * Default {@link GeoDistance} function. This method should be used, If no specific function has been selected.
-     * This is an alias for <code>SLOPPY_ARC</code>
-     */
-    @Deprecated
-    public static final GeoDistance DEFAULT = SLOPPY_ARC;
-
-    public abstract double normalize(double distance, DistanceUnit unit);
-
-    public abstract double calculate(double sourceLatitude, double sourceLongitude, double targetLatitude, double targetLongitude, DistanceUnit unit);
-
-    public abstract FixedSourceDistance fixedSourceDistance(double sourceLatitude, double sourceLongitude, DistanceUnit unit);
-
-    private static final double MIN_LAT = Math.toRadians(-90d);  // -PI/2
-    private static final double MAX_LAT = Math.toRadians(90d);   //  PI/2
-    private static final double MIN_LON = Math.toRadians(-180d); // -PI
-    private static final double MAX_LON = Math.toRadians(180d);  //  PI
-
-    public static DistanceBoundingCheck distanceBoundingCheck(double sourceLatitude, double sourceLongitude, double distance, DistanceUnit unit) {
-        // angular distance in radians on a great circle
-        // assume worst-case: use the minor axis
-        double radDist = unit.toMeters(distance) / GeoUtils.EARTH_SEMI_MINOR_AXIS;
-
-        double radLat = Math.toRadians(sourceLatitude);
-        double radLon = Math.toRadians(sourceLongitude);
-
-        double minLat = radLat - radDist;
-        double maxLat = radLat + radDist;
-
-        double minLon, maxLon;
-        if (minLat > MIN_LAT && maxLat < MAX_LAT) {
-            double deltaLon = Math.asin(Math.sin(radDist) / Math.cos(radLat));
-            minLon = radLon - deltaLon;
-            if (minLon < MIN_LON) minLon += 2d * Math.PI;
-            maxLon = radLon + deltaLon;
-            if (maxLon > MAX_LON) maxLon -= 2d * Math.PI;
-        } else {
-            // a pole is within the distance
-            minLat = Math.max(minLat, MIN_LAT);
-            maxLat = Math.min(maxLat, MAX_LAT);
-            minLon = MIN_LON;
-            maxLon = MAX_LON;
-        }
-
-        GeoPoint topLeft = new GeoPoint(Math.toDegrees(maxLat), Math.toDegrees(minLon));
-        GeoPoint bottomRight = new GeoPoint(Math.toDegrees(minLat), Math.toDegrees(maxLon));
-        if (minLon > maxLon) {
-            return new Meridian180DistanceBoundingCheck(topLeft, bottomRight);
-        }
-        return new SimpleDistanceBoundingCheck(topLeft, bottomRight);
-    }
-
     /**
      * Get a {@link GeoDistance} according to a given name. Valid values are
      *
      * <ul>
      *     <li><b>plane</b> for <code>GeoDistance.PLANE</code></li>
-     *     <li><b>sloppy_arc</b> for <code>GeoDistance.SLOPPY_ARC</code></li>
-     *     <li><b>factor</b> for <code>GeoDistance.FACTOR</code></li>
      *     <li><b>arc</b> for <code>GeoDistance.ARC</code></li>
      * </ul>
      *
@@ -218,222 +65,16 @@ public enum GeoDistance implements Writeable {
             return PLANE;
         } else if ("arc".equals(name)) {
             return ARC;
-        } else if ("sloppy_arc".equals(name)) {
-            return SLOPPY_ARC;
-        } else if ("factor".equals(name)) {
-            return FACTOR;
         }
         throw new IllegalArgumentException("No geo distance for [" + name + "]");
     }
 
-    public interface FixedSourceDistance {
-
-        double calculate(double targetLatitude, double targetLongitude);
-    }
-
-    public interface DistanceBoundingCheck {
-
-        boolean isWithin(double targetLatitude, double targetLongitude);
-
-        GeoPoint topLeft();
-
-        GeoPoint bottomRight();
-    }
-
-    public static final AlwaysDistanceBoundingCheck ALWAYS_INSTANCE = new AlwaysDistanceBoundingCheck();
-
-    private static class AlwaysDistanceBoundingCheck implements DistanceBoundingCheck {
-        @Override
-        public boolean isWithin(double targetLatitude, double targetLongitude) {
-            return true;
-        }
-
-        @Override
-        public GeoPoint topLeft() {
-            return null;
-        }
-
-        @Override
-        public GeoPoint bottomRight() {
-            return null;
-        }
-    }
-
-    public static class Meridian180DistanceBoundingCheck implements DistanceBoundingCheck {
-
-        private final GeoPoint topLeft;
-        private final GeoPoint bottomRight;
-
-        public Meridian180DistanceBoundingCheck(GeoPoint topLeft, GeoPoint bottomRight) {
-            this.topLeft = topLeft;
-            this.bottomRight = bottomRight;
-        }
-
-        @Override
-        public boolean isWithin(double targetLatitude, double targetLongitude) {
-            return (targetLatitude >= bottomRight.lat() && targetLatitude <= topLeft.lat()) &&
-                    (targetLongitude >= topLeft.lon() || targetLongitude <= bottomRight.lon());
-        }
-
-        @Override
-        public GeoPoint topLeft() {
-            return topLeft;
-        }
-
-        @Override
-        public GeoPoint bottomRight() {
-            return bottomRight;
-        }
-    }
-
-    public static class SimpleDistanceBoundingCheck implements DistanceBoundingCheck {
-        private final GeoPoint topLeft;
-        private final GeoPoint bottomRight;
-
-        public SimpleDistanceBoundingCheck(GeoPoint topLeft, GeoPoint bottomRight) {
-            this.topLeft = topLeft;
-            this.bottomRight = bottomRight;
-        }
-
-        @Override
-        public boolean isWithin(double targetLatitude, double targetLongitude) {
-            return (targetLatitude >= bottomRight.lat() && targetLatitude <= topLeft.lat()) &&
-                    (targetLongitude >= topLeft.lon() && targetLongitude <= bottomRight.lon());
-        }
-
-        @Override
-        public GeoPoint topLeft() {
-            return topLeft;
-        }
-
-        @Override
-        public GeoPoint bottomRight() {
-            return bottomRight;
-        }
-    }
-
-    public static class PlaneFixedSourceDistance implements FixedSourceDistance {
-
-        private final double sourceLatitude;
-        private final double sourceLongitude;
-        private final double distancePerDegree;
-
-        public PlaneFixedSourceDistance(double sourceLatitude, double sourceLongitude, DistanceUnit unit) {
-            this.sourceLatitude = sourceLatitude;
-            this.sourceLongitude = sourceLongitude;
-            this.distancePerDegree = unit.getDistancePerDegree();
-        }
-
-        @Override
-        public double calculate(double targetLatitude, double targetLongitude) {
-            double px = targetLongitude - sourceLongitude;
-            double py = targetLatitude - sourceLatitude;
-            return Math.sqrt(px * px + py * py) * distancePerDegree;
-        }
-    }
-
-    public static class FactorFixedSourceDistance implements FixedSourceDistance {
-
-        private final double sourceLongitude;
-
-        private final double a;
-        private final double sinA;
-        private final double cosA;
-
-        public FactorFixedSourceDistance(double sourceLatitude, double sourceLongitude) {
-            this.sourceLongitude = sourceLongitude;
-            this.a = Math.toRadians(90D - sourceLatitude);
-            this.sinA = Math.sin(a);
-            this.cosA = Math.cos(a);
-        }
-
-        @Override
-        public double calculate(double targetLatitude, double targetLongitude) {
-            double longitudeDifference = targetLongitude - sourceLongitude;
-            double c = Math.toRadians(90D - targetLatitude);
-            return (cosA * Math.cos(c)) + (sinA * Math.sin(c) * Math.cos(Math.toRadians(longitudeDifference)));
-        }
-    }
-
-    /**
-     * Basic implementation of {@link FixedSourceDistance}. This class keeps the basic parameters for a distance
-     * functions based on a fixed source. Namely latitude, longitude and unit.
-     */
-    public abstract static class FixedSourceDistanceBase implements FixedSourceDistance {
-        protected final double sourceLatitude;
-        protected final double sourceLongitude;
-        protected final DistanceUnit unit;
-
-        public FixedSourceDistanceBase(double sourceLatitude, double sourceLongitude, DistanceUnit unit) {
-            this.sourceLatitude = sourceLatitude;
-            this.sourceLongitude = sourceLongitude;
-            this.unit = unit;
-        }
-    }
-
-    public static class ArcFixedSourceDistance extends FixedSourceDistanceBase {
-
-        public ArcFixedSourceDistance(double sourceLatitude, double sourceLongitude, DistanceUnit unit) {
-            super(sourceLatitude, sourceLongitude, unit);
-        }
-
-        @Override
-        public double calculate(double targetLatitude, double targetLongitude) {
-            return ARC.calculate(sourceLatitude, sourceLongitude, targetLatitude, targetLongitude, unit);
-        }
-
-    }
-
-    public static class SloppyArcFixedSourceDistance extends FixedSourceDistanceBase {
-
-        public SloppyArcFixedSourceDistance(double sourceLatitude, double sourceLongitude, DistanceUnit unit) {
-            super(sourceLatitude, sourceLongitude, unit);
-        }
-
-        @Override
-        public double calculate(double targetLatitude, double targetLongitude) {
-            return SLOPPY_ARC.calculate(sourceLatitude, sourceLongitude, targetLatitude, targetLongitude, unit);
-        }
-    }
-
-
-    /**
-     * Return a {@link SortedNumericDoubleValues} instance that returns the distances to a list of geo-points for each document.
-     */
-    public static SortedNumericDoubleValues distanceValues(final MultiGeoPointValues geoPointValues, final FixedSourceDistance... distances) {
-        final GeoPointValues singleValues = FieldData.unwrapSingleton(geoPointValues);
-        if (singleValues != null && distances.length == 1) {
-            final Bits docsWithField = FieldData.unwrapSingletonBits(geoPointValues);
-            return FieldData.singleton(new NumericDoubleValues() {
-
-                @Override
-                public double get(int docID) {
-                    if (docsWithField != null && !docsWithField.get(docID)) {
-                        return 0d;
-                    }
-                    final GeoPoint point = singleValues.get(docID);
-                    return distances[0].calculate(point.lat(), point.lon());
-                }
-
-            }, docsWithField);
-        } else {
-            return new SortingNumericDoubleValues() {
-
-                @Override
-                public void setDocument(int doc) {
-                    geoPointValues.setDocument(doc);
-                    resize(geoPointValues.count() * distances.length);
-                    int valueCounter = 0;
-                    for (FixedSourceDistance distance : distances) {
-                        for (int i = 0; i < geoPointValues.count(); ++i) {
-                            final GeoPoint point = geoPointValues.valueAt(i);
-                            values[valueCounter] = distance.calculate(point.lat(), point.lon());
-                            valueCounter++;
-                        }
-                    }
-                    sort();
-                }
-            };
+    /** compute the distance between two points using the selected algorithm (PLANE, ARC) */
+    public double calculate(double srcLat, double srcLon, double dstLat, double dstLon, DistanceUnit unit) {
+        if (this == PLANE) {
+            return DistanceUnit.convert(GeoUtils.planeDistance(srcLat, srcLon, dstLat, dstLon),
+                DistanceUnit.METERS, unit);
         }
+        return DistanceUnit.convert(GeoUtils.arcDistance(srcLat, srcLon, dstLat, dstLon), DistanceUnit.METERS, unit);
     }
 }

+ 65 - 9
core/src/main/java/org/elasticsearch/common/geo/GeoUtils.java

@@ -19,13 +19,21 @@
 
 package org.elasticsearch.common.geo;
 
+import org.apache.lucene.geo.Rectangle;
 import org.apache.lucene.spatial.prefix.tree.GeohashPrefixTree;
 import org.apache.lucene.spatial.prefix.tree.QuadPrefixTree;
+import org.apache.lucene.util.Bits;
 import org.apache.lucene.util.SloppyMath;
 import org.elasticsearch.ElasticsearchParseException;
 import org.elasticsearch.common.unit.DistanceUnit;
 import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.common.xcontent.XContentParser.Token;
+import org.elasticsearch.index.fielddata.FieldData;
+import org.elasticsearch.index.fielddata.GeoPointValues;
+import org.elasticsearch.index.fielddata.MultiGeoPointValues;
+import org.elasticsearch.index.fielddata.NumericDoubleValues;
+import org.elasticsearch.index.fielddata.SortedNumericDoubleValues;
+import org.elasticsearch.index.fielddata.SortingNumericDoubleValues;
 
 import java.io.IOException;
 
@@ -65,14 +73,6 @@ public class GeoUtils {
     /** rounding error for quantized latitude and longitude values */
     public static final double TOLERANCE = 1E-6;
 
-    /** Returns the minimum between the provided distance 'initialRadius' and the
-     * maximum distance/radius from the point 'center' before overlapping
-     **/
-    public static double maxRadialDistance(GeoPoint center, double initialRadius) {
-        final double maxRadius = maxRadialDistanceMeters(center.lat(), center.lon());
-        return Math.min(initialRadius, maxRadius);
-    }
-
     /** Returns true if latitude is actually a valid latitude value.*/
     public static boolean isValidLatitude(double latitude) {
         if (Double.isNaN(latitude) || Double.isInfinite(latitude) || latitude < GeoUtils.MIN_LAT || latitude > GeoUtils.MAX_LAT) {
@@ -481,7 +481,8 @@ public class GeoUtils {
 
     /**
      * Return the distance (in meters) between 2 lat,lon geo points using a simple tangential plane
-     * this provides a faster alternative to {@link GeoUtils#arcDistance} when points are within 5 km
+     * this provides a faster alternative to {@link GeoUtils#arcDistance} but is innaccurate for distances greater than
+     * 4 decimal degrees
      */
     public static double planeDistance(double lat1, double lon1, double lat2, double lon2) {
         double x = (lon2 - lon1) * SloppyMath.TO_RADIANS * Math.cos((lat2 + lat1) / 2.0 * SloppyMath.TO_RADIANS);
@@ -489,6 +490,61 @@ public class GeoUtils {
         return Math.sqrt(x * x + y * y) * EARTH_MEAN_RADIUS;
     }
 
+    /** check if point is within a rectangle
+     * todo: move this to lucene Rectangle class
+     */
+    public static boolean rectangleContainsPoint(Rectangle r, double lat, double lon) {
+        if (lat >= r.minLat && lat <= r.maxLat) {
+            // if rectangle crosses the dateline we only check if the lon is >= min or max
+            return r.crossesDateline() ? lon >= r.minLon || lon <= r.maxLon : lon >= r.minLon && lon <= r.maxLon;
+        }
+        return false;
+    }
+
+    /**
+     * Return a {@link SortedNumericDoubleValues} instance that returns the distances to a list of geo-points
+     * for each document.
+     */
+    public static SortedNumericDoubleValues distanceValues(final GeoDistance distance,
+                                                           final DistanceUnit unit,
+                                                           final MultiGeoPointValues geoPointValues,
+                                                           final GeoPoint... fromPoints) {
+        final GeoPointValues singleValues = FieldData.unwrapSingleton(geoPointValues);
+        if (singleValues != null && fromPoints.length == 1) {
+            final Bits docsWithField = FieldData.unwrapSingletonBits(geoPointValues);
+            return FieldData.singleton(new NumericDoubleValues() {
+
+                @Override
+                public double get(int docID) {
+                    if (docsWithField != null && !docsWithField.get(docID)) {
+                        return 0d;
+                    }
+                    final GeoPoint to = singleValues.get(docID);
+                    final GeoPoint from = fromPoints[0];
+                    return distance.calculate(from.lat(), from.lon(), to.lat(), to.lon(), unit);
+                }
+
+            }, docsWithField);
+        } else {
+            return new SortingNumericDoubleValues() {
+                @Override
+                public void setDocument(int doc) {
+                    geoPointValues.setDocument(doc);
+                    resize(geoPointValues.count() * fromPoints.length);
+                    int v = 0;
+                    for (GeoPoint from : fromPoints) {
+                        for (int i = 0; i < geoPointValues.count(); ++i) {
+                            final GeoPoint point = geoPointValues.valueAt(i);
+                            values[v] = distance.calculate(from.lat(), from.lon(), point.lat(), point.lon(), unit);
+                            v++;
+                        }
+                    }
+                    sort();
+                }
+            };
+        }
+    }
+
     private GeoUtils() {
     }
 }

+ 3 - 5
core/src/main/java/org/elasticsearch/index/query/GeoDistanceQueryBuilder.java

@@ -55,7 +55,7 @@ public class GeoDistanceQueryBuilder extends AbstractQueryBuilder<GeoDistanceQue
     /** Default for distance unit computation. */
     public static final DistanceUnit DEFAULT_DISTANCE_UNIT = DistanceUnit.DEFAULT;
     /** Default for geo distance computation. */
-    public static final GeoDistance DEFAULT_GEO_DISTANCE = GeoDistance.DEFAULT;
+    public static final GeoDistance DEFAULT_GEO_DISTANCE = GeoDistance.ARC;
     /** Default for optimising query through pre computed bounding box query. */
     @Deprecated
     public static final String DEFAULT_OPTIMIZE_BBOX = "memory";
@@ -82,7 +82,7 @@ public class GeoDistanceQueryBuilder extends AbstractQueryBuilder<GeoDistanceQue
     /** Point to use as center. */
     private GeoPoint center = new GeoPoint(Double.NaN, Double.NaN);
     /** Algorithm to use for distance computation. */
-    private GeoDistance geoDistance = DEFAULT_GEO_DISTANCE;
+    private GeoDistance geoDistance = GeoDistance.ARC;
     /** Whether or not to use a bbox for pre-filtering. TODO change to enum? */
     private String optimizeBbox = null;
     /** How strict should geo coordinate validation be? */
@@ -287,9 +287,7 @@ public class GeoDistanceQueryBuilder extends AbstractQueryBuilder<GeoDistanceQue
             GeoUtils.normalizePoint(center, true, true);
         }
 
-        double normDistance = geoDistance.normalize(this.distance, DistanceUnit.DEFAULT);
-
-        return LatLonPoint.newDistanceQuery(fieldType.name(), center.lat(), center.lon(), normDistance);
+        return LatLonPoint.newDistanceQuery(fieldType.name(), center.lat(), center.lon(), this.distance);
     }
 
     @Override

+ 1 - 1
core/src/main/java/org/elasticsearch/index/query/functionscore/DecayFunctionBuilder.java

@@ -329,7 +329,7 @@ public abstract class DecayFunctionBuilder<DFB extends DecayFunctionBuilder<DFB>
         private final GeoPoint origin;
         private final IndexGeoPointFieldData fieldData;
 
-        private static final GeoDistance distFunction = GeoDistance.DEFAULT;
+        private static final GeoDistance distFunction = GeoDistance.ARC;
 
         public GeoFieldDataScoreFunction(GeoPoint origin, double scale, double decay, double offset, DecayFunction func,
                                          IndexGeoPointFieldData fieldData, MultiValueMode mode) {

+ 1 - 1
core/src/main/java/org/elasticsearch/search/aggregations/bucket/range/geodistance/GeoDistanceAggregationBuilder.java

@@ -201,7 +201,7 @@ public class GeoDistanceAggregationBuilder extends ValuesSourceAggregationBuilde
     private GeoPoint origin;
     private List<Range> ranges = new ArrayList<>();
     private DistanceUnit unit = DistanceUnit.DEFAULT;
-    private GeoDistance distanceType = GeoDistance.DEFAULT;
+    private GeoDistance distanceType = GeoDistance.ARC;
     private boolean keyed = false;
 
     public GeoDistanceAggregationBuilder(String name, GeoPoint origin) {

+ 6 - 7
core/src/main/java/org/elasticsearch/search/aggregations/bucket/range/geodistance/GeoDistanceRangeAggregatorFactory.java

@@ -22,8 +22,8 @@ package org.elasticsearch.search.aggregations.bucket.range.geodistance;
 import org.apache.lucene.index.LeafReaderContext;
 import org.apache.lucene.index.SortedNumericDocValues;
 import org.elasticsearch.common.geo.GeoDistance;
-import org.elasticsearch.common.geo.GeoDistance.FixedSourceDistance;
 import org.elasticsearch.common.geo.GeoPoint;
+import org.elasticsearch.common.geo.GeoUtils;
 import org.elasticsearch.common.unit.DistanceUnit;
 import org.elasticsearch.index.fielddata.MultiGeoPointValues;
 import org.elasticsearch.index.fielddata.SortedBinaryDocValues;
@@ -85,16 +85,16 @@ public class GeoDistanceRangeAggregatorFactory
 
         private final ValuesSource.GeoPoint source;
         private final GeoDistance distanceType;
-        private final DistanceUnit unit;
+        private final DistanceUnit units;
         private final org.elasticsearch.common.geo.GeoPoint origin;
 
-        public DistanceSource(ValuesSource.GeoPoint source, GeoDistance distanceType, org.elasticsearch.common.geo.GeoPoint origin,
-                DistanceUnit unit) {
+        public DistanceSource(ValuesSource.GeoPoint source, GeoDistance distanceType,
+                org.elasticsearch.common.geo.GeoPoint origin, DistanceUnit units) {
             this.source = source;
             // even if the geo points are unique, there's no guarantee the
             // distances are
             this.distanceType = distanceType;
-            this.unit = unit;
+            this.units = units;
             this.origin = origin;
         }
 
@@ -111,8 +111,7 @@ public class GeoDistanceRangeAggregatorFactory
         @Override
         public SortedNumericDoubleValues doubleValues(LeafReaderContext ctx) {
             final MultiGeoPointValues geoValues = source.geoPointValues(ctx);
-            final FixedSourceDistance distance = distanceType.fixedSourceDistance(origin.getLat(), origin.getLon(), unit);
-            return GeoDistance.distanceValues(geoValues, distance);
+            return GeoUtils.distanceValues(distanceType, units, geoValues, origin);
         }
 
         @Override

+ 25 - 26
core/src/main/java/org/elasticsearch/search/sort/GeoDistanceSortBuilder.java

@@ -31,7 +31,6 @@ import org.elasticsearch.Version;
 import org.elasticsearch.common.ParseField;
 import org.elasticsearch.common.ParsingException;
 import org.elasticsearch.common.geo.GeoDistance;
-import org.elasticsearch.common.geo.GeoDistance.FixedSourceDistance;
 import org.elasticsearch.common.geo.GeoPoint;
 import org.elasticsearch.common.geo.GeoUtils;
 import org.elasticsearch.common.io.stream.StreamInput;
@@ -80,7 +79,7 @@ public class GeoDistanceSortBuilder extends SortBuilder<GeoDistanceSortBuilder>
     private final String fieldName;
     private final List<GeoPoint> points = new ArrayList<>();
 
-    private GeoDistance geoDistance = GeoDistance.DEFAULT;
+    private GeoDistance geoDistance = GeoDistance.ARC;
     private DistanceUnit unit = DistanceUnit.DEFAULT;
 
     private SortMode sortMode = null;
@@ -390,17 +389,18 @@ public class GeoDistanceSortBuilder extends SortBuilder<GeoDistanceSortBuilder>
      * Creates a new {@link GeoDistanceSortBuilder} from the query held by the {@link QueryParseContext} in
      * {@link org.elasticsearch.common.xcontent.XContent} format.
      *
-     * @param context the input parse context. The state on the parser contained in this context will be changed as a side effect of this
-     *        method call
-     * @param elementName in some sort syntax variations the field name precedes the xContent object that specifies further parameters, e.g.
-     *        in '{ "foo": { "order" : "asc"} }'. When parsing the inner object, the field name can be passed in via this argument
+     * @param context the input parse context. The state on the parser contained in this context will be changed as a
+     *                side effect of this method call
+     * @param elementName in some sort syntax variations the field name precedes the xContent object that specifies
+     *                    further parameters, e.g. in '{ "foo": { "order" : "asc"} }'. When parsing the inner object,
+     *                    the field name can be passed in via this argument
      */
     public static GeoDistanceSortBuilder fromXContent(QueryParseContext context, String elementName) throws IOException {
         XContentParser parser = context.parser();
         String fieldName = null;
         List<GeoPoint> geoPoints = new ArrayList<>();
         DistanceUnit unit = DistanceUnit.DEFAULT;
-        GeoDistance geoDistance = GeoDistance.DEFAULT;
+        GeoDistance geoDistance = GeoDistance.ARC;
         SortOrder order = SortOrder.ASC;
         SortMode sortMode = null;
         QueryBuilder nestedFilter = null;
@@ -505,12 +505,10 @@ public class GeoDistanceSortBuilder extends SortBuilder<GeoDistanceSortBuilder>
     @Override
     public SortFieldAndFormat build(QueryShardContext context) throws IOException {
         final boolean indexCreatedBeforeV2_0 = context.indexVersionCreated().before(Version.V_2_0_0);
-        // validation was not available prior to 2.x, so to support bwc percolation queries we only ignore_malformed on 2.x created indexes
-        List<GeoPoint> localPoints = new ArrayList<>();
-        for (GeoPoint geoPoint : this.points) {
-            localPoints.add(new GeoPoint(geoPoint));
-        }
 
+        // validation was not available prior to 2.x, so to support bwc percolation queries we only ignore_malformed
+        // on 2.x created indexes
+        GeoPoint[] localPoints =  points.toArray(new GeoPoint[points.size()]);
         if (!indexCreatedBeforeV2_0 && !GeoValidationMethod.isIgnoreMalformed(validation)) {
             for (GeoPoint point : localPoints) {
                 if (GeoUtils.isValidLatitude(point.lat()) == false) {
@@ -544,7 +542,8 @@ public class GeoDistanceSortBuilder extends SortBuilder<GeoDistanceSortBuilder>
 
         MappedFieldType fieldType = context.fieldMapper(fieldName);
         if (fieldType == null) {
-            throw new IllegalArgumentException("failed to find mapper for [" + fieldName + "] for geo distance based sort");
+            throw new IllegalArgumentException("failed to find mapper for [" + fieldName
+                + "] for geo distance based sort");
         }
         final IndexGeoPointFieldData geoIndexFieldData = context.getForField(fieldType);
         final Nested nested = resolveNested(context, nestedPath, nestedFilter);
@@ -554,17 +553,12 @@ public class GeoDistanceSortBuilder extends SortBuilder<GeoDistanceSortBuilder>
                 && finalSortMode == MultiValueMode.MIN // LatLonDocValuesField internally picks the closest point
                 && unit == DistanceUnit.METERS
                 && reverse == false
-                && localPoints.size() == 1) {
+                && localPoints.length == 1) {
             return new SortFieldAndFormat(
-                    LatLonDocValuesField.newDistanceSort(fieldName, localPoints.get(0).lat(), localPoints.get(0).lon()),
+                    LatLonDocValuesField.newDistanceSort(fieldName, localPoints[0].lat(), localPoints[0].lon()),
                     DocValueFormat.RAW);
         }
 
-        final FixedSourceDistance[] distances = new FixedSourceDistance[localPoints.size()];
-        for (int i = 0; i < localPoints.size(); i++) {
-            distances[i] = geoDistance.fixedSourceDistance(localPoints.get(i).lat(), localPoints.get(i).lon(), unit);
-        }
-
         IndexFieldData.XFieldComparatorSource geoDistanceComparatorSource = new IndexFieldData.XFieldComparatorSource() {
 
             @Override
@@ -573,12 +567,15 @@ public class GeoDistanceSortBuilder extends SortBuilder<GeoDistanceSortBuilder>
             }
 
             @Override
-            public FieldComparator<?> newComparator(String fieldname, int numHits, int sortPos, boolean reversed) throws IOException {
+            public FieldComparator<?> newComparator(String fieldname, int numHits, int sortPos, boolean reversed)
+                throws IOException {
                 return new FieldComparator.DoubleComparator(numHits, null, null) {
                     @Override
-                    protected NumericDocValues getNumericDocValues(LeafReaderContext context, String field) throws IOException {
+                    protected NumericDocValues getNumericDocValues(LeafReaderContext context, String field)
+                        throws IOException {
                         final MultiGeoPointValues geoPointValues = geoIndexFieldData.load(context).getGeoPointValues();
-                        final SortedNumericDoubleValues distanceValues = GeoDistance.distanceValues(geoPointValues, distances);
+                        final SortedNumericDoubleValues distanceValues = GeoUtils.distanceValues(geoDistance, unit,
+                            geoPointValues, localPoints);
                         final NumericDoubleValues selectedValues;
                         if (nested == null) {
                             selectedValues = finalSortMode.select(distanceValues, Double.POSITIVE_INFINITY);
@@ -595,14 +592,16 @@ public class GeoDistanceSortBuilder extends SortBuilder<GeoDistanceSortBuilder>
 
         };
 
-        return new SortFieldAndFormat(new SortField(fieldName, geoDistanceComparatorSource, reverse), DocValueFormat.RAW);
+        return new SortFieldAndFormat(new SortField(fieldName, geoDistanceComparatorSource, reverse),
+            DocValueFormat.RAW);
     }
 
     static void parseGeoPoints(XContentParser parser, List<GeoPoint> geoPoints) throws IOException {
         while (!parser.nextToken().equals(XContentParser.Token.END_ARRAY)) {
             if (parser.currentToken() == XContentParser.Token.VALUE_NUMBER) {
-                // we might get here if the geo point is " number, number] " and the parser already moved over the opening bracket
-                // in this case we cannot use GeoUtils.parseGeoPoint(..) because this expects an opening bracket
+                // we might get here if the geo point is " number, number] " and the parser already moved over the
+                // opening bracket in this case we cannot use GeoUtils.parseGeoPoint(..) because this expects an opening
+                // bracket
                 double lon = parser.doubleValue();
                 parser.nextToken();
                 if (!parser.currentToken().equals(XContentParser.Token.VALUE_NUMBER)) {

+ 0 - 88
core/src/test/java/org/apache/lucene/util/SloppyMathTests.java

@@ -1,88 +0,0 @@
-/*
- * Licensed to Elasticsearch under one or more contributor
- * license agreements. See the NOTICE file distributed with
- * this work for additional information regarding copyright
- * ownership. Elasticsearch licenses this file to you under
- * the Apache License, Version 2.0 (the "License"); you may
- * not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *    http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied.  See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-
-package org.apache.lucene.util;
-
-import org.elasticsearch.common.geo.GeoDistance;
-import org.elasticsearch.common.unit.DistanceUnit;
-import org.elasticsearch.test.ESTestCase;
-
-import static org.hamcrest.number.IsCloseTo.closeTo;
-
-public class SloppyMathTests extends ESTestCase {
-    public void testAccuracy() {
-        for (double lat1 = -89; lat1 <= 89; lat1+=1) {
-            final double lon1 = randomLongitude();
-
-            for (double i = -180; i <= 180; i+=1) {
-                final double lon2 = i;
-                final double lat2 = randomLatitude();
-
-                assertAccurate(lat1, lon1, lat2, lon2);
-            }
-        }
-    }
-
-    public void testSloppyMath() {
-        testSloppyMath(DistanceUnit.METERS, 0.01, 5, 45, 90);
-        testSloppyMath(DistanceUnit.KILOMETERS, 0.01, 5, 45, 90);
-        testSloppyMath(DistanceUnit.INCH, 0.01, 5, 45, 90);
-        testSloppyMath(DistanceUnit.MILES, 0.01, 5, 45, 90);
-    }
-
-    private static double maxError(double distance) {
-        return distance / 1000.0;
-    }
-
-    private void testSloppyMath(DistanceUnit unit, double...deltaDeg) {
-        final double lat1 = randomLatitude();
-        final double lon1 = randomLongitude();
-        logger.info("testing SloppyMath with {} at \"{}, {}\"", unit, lat1, lon1);
-
-        for (int test = 0; test < deltaDeg.length; test++) {
-            for (int i = 0; i < 100; i++) {
-                // crop pole areas, sine we now there the function
-                // is not accurate around lat(89°, 90°) and lat(-90°, -89°)
-                final double lat2 = Math.max(-89.0, Math.min(+89.0, lat1 + (random().nextDouble() - 0.5) * 2 * deltaDeg[test]));
-                final double lon2 = lon1 + (random().nextDouble() - 0.5) * 2 * deltaDeg[test];
-
-                final double accurate = GeoDistance.ARC.calculate(lat1, lon1, lat2, lon2, unit);
-                final double dist = GeoDistance.SLOPPY_ARC.calculate(lat1, lon1, lat2, lon2, unit);
-
-                assertThat("distance between("+lat1+", "+lon1+") and ("+lat2+", "+lon2+"))", dist, closeTo(accurate, maxError(accurate)));
-            }
-        }
-    }
-
-    private static void assertAccurate(double lat1, double lon1, double lat2, double lon2) {
-        double accurate = GeoDistance.ARC.calculate(lat1, lon1, lat2, lon2, DistanceUnit.METERS);
-        double sloppy = GeoDistance.SLOPPY_ARC.calculate(lat1, lon1, lat2, lon2, DistanceUnit.METERS);
-        assertThat("distance between("+lat1+", "+lon1+") and ("+lat2+", "+lon2+"))", sloppy, closeTo(accurate, maxError(accurate)));
-    }
-
-    private static double randomLatitude() {
-        // crop pole areas, sine we now there the function
-        // is not accurate around lat(89°, 90°) and lat(-90°, -89°)
-        return (random().nextDouble() - 0.5) * 178.0;
-    }
-
-    private static double randomLongitude() {
-        return (random().nextDouble() - 0.5) * 360.0;
-    }
-}

+ 19 - 12
core/src/test/java/org/elasticsearch/common/geo/GeoDistanceTests.java

@@ -18,6 +18,7 @@
  */
 package org.elasticsearch.common.geo;
 
+import org.apache.lucene.geo.Rectangle;
 import org.elasticsearch.common.io.stream.BytesStreamOutput;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.unit.DistanceUnit;
@@ -38,12 +39,10 @@ public class GeoDistanceTests extends ESTestCase {
     public void testGeoDistanceSerialization() throws IOException  {
         // make sure that ordinals don't change, because we rely on then in serialization
         assertThat(GeoDistance.PLANE.ordinal(), equalTo(0));
-        assertThat(GeoDistance.FACTOR.ordinal(), equalTo(1));
-        assertThat(GeoDistance.ARC.ordinal(), equalTo(2));
-        assertThat(GeoDistance.SLOPPY_ARC.ordinal(), equalTo(3));
-        assertThat(GeoDistance.values().length, equalTo(4));
+        assertThat(GeoDistance.ARC.ordinal(), equalTo(1));
+        assertThat(GeoDistance.values().length, equalTo(2));
 
-        GeoDistance geoDistance = randomFrom(GeoDistance.PLANE, GeoDistance.FACTOR, GeoDistance.ARC, GeoDistance.SLOPPY_ARC);
+        GeoDistance geoDistance = randomFrom(GeoDistance.PLANE, GeoDistance.ARC);
         try (BytesStreamOutput out = new BytesStreamOutput()) {
             geoDistance.writeTo(out);
             try (StreamInput in = out.bytes().streamInput()) {;
@@ -70,16 +69,24 @@ public class GeoDistanceTests extends ESTestCase {
 
     public void testDistanceCheck() {
         // Note, is within is an approximation, so, even though 0.52 is outside 50mi, we still get "true"
-        GeoDistance.DistanceBoundingCheck check = GeoDistance.distanceBoundingCheck(0, 0, 50, DistanceUnit.MILES);
-        assertThat(check.isWithin(0.5, 0.5), equalTo(true));
-        assertThat(check.isWithin(0.52, 0.52), equalTo(true));
-        assertThat(check.isWithin(1, 1), equalTo(false));
+        double radius = DistanceUnit.convert(50, DistanceUnit.MILES, DistanceUnit.METERS);
+        Rectangle box = Rectangle.fromPointDistance(0, 0, radius);
+        assertThat(GeoUtils.rectangleContainsPoint(box, 0.5, 0.5), equalTo(true));
+        assertThat(GeoUtils.rectangleContainsPoint(box, 0.52, 0.52), equalTo(true));
+        assertThat(GeoUtils.rectangleContainsPoint(box, 1, 1), equalTo(false));
 
-        check = GeoDistance.distanceBoundingCheck(0, 179, 200, DistanceUnit.MILES);
-        assertThat(check.isWithin(0, -179), equalTo(true));
-        assertThat(check.isWithin(0, -178), equalTo(false));
+        radius = DistanceUnit.convert(200, DistanceUnit.MILES, DistanceUnit.METERS);
+        box = Rectangle.fromPointDistance(0, 179, radius);
+        assertThat(GeoUtils.rectangleContainsPoint(box, 0, -179), equalTo(true));
+        assertThat(GeoUtils.rectangleContainsPoint(box, 0, -178), equalTo(false));
     }
 
+    /**
+     * The old plane calculation in 1.x/2.x incorrectly computed the plane distance in decimal degrees. This test is
+     * well intended but bogus. todo: fix w/ new plane distance calculation
+     * note: plane distance error varies by latitude so the test will need to correctly estimate expected error
+     */
+    @AwaitsFix(bugUrl = "old plane calculation incorrectly computed everything in degrees. fix this bogus test")
     public void testArcDistanceVsPlaneInEllipsis() {
         GeoPoint centre = new GeoPoint(48.8534100, 2.3488000);
         GeoPoint northernPoint = new GeoPoint(48.8801108681, 2.35152032666);

+ 4 - 4
core/src/test/java/org/elasticsearch/index/query/GeoDistanceQueryBuilderTests.java

@@ -303,7 +303,7 @@ public class GeoDistanceQueryBuilderTests extends AbstractQueryTestCase<GeoDista
                 "  \"geo_distance\" : {\n" +
                 "    \"pin.location\" : [ -70.0, 40.0 ],\n" +
                 "    \"distance\" : 12000.0,\n" +
-                "    \"distance_type\" : \"sloppy_arc\",\n" +
+                "    \"distance_type\" : \"arc\",\n" +
                 "    \"validation_method\" : \"STRICT\",\n" +
                 "    \"ignore_unmapped\" : false,\n" +
                 "    \"boost\" : 1.0\n" +
@@ -322,7 +322,7 @@ public class GeoDistanceQueryBuilderTests extends AbstractQueryTestCase<GeoDista
                 "  \"geo_distance\" : {\n" +
                 "    \"pin.location\" : [ -70.0, 40.0 ],\n" +
                 "    \"distance\" : 12000.0,\n" +
-                "    \"distance_type\" : \"sloppy_arc\",\n" +
+                "    \"distance_type\" : \"arc\",\n" +
                 "    \"optimize_bbox\" : \"memory\",\n" +
                 "    \"validation_method\" : \"STRICT\",\n" +
                 "    \"ignore_unmapped\" : false,\n" +
@@ -340,7 +340,7 @@ public class GeoDistanceQueryBuilderTests extends AbstractQueryTestCase<GeoDista
                 "  \"geo_distance\" : {\n" +
                 "    \"pin.location\" : [ -70.0, 40.0 ],\n" +
                 "    \"distance\" : 12000.0,\n" +
-                "    \"distance_type\" : \"sloppy_arc\",\n" +
+                "    \"distance_type\" : \"arc\",\n" +
                 "    \"coerce\" : true,\n" +
                 "    \"ignore_unmapped\" : false,\n" +
                 "    \"boost\" : 1.0\n" +
@@ -356,7 +356,7 @@ public class GeoDistanceQueryBuilderTests extends AbstractQueryTestCase<GeoDista
                 "  \"geo_distance\" : {\n" +
                 "    \"pin.location\" : [ -70.0, 40.0 ],\n" +
                 "    \"distance\" : 12000.0,\n" +
-                "    \"distance_type\" : \"sloppy_arc\",\n" +
+                "    \"distance_type\" : \"arc\",\n" +
                 "    \"ignore_malformed\" : true,\n" +
                 "    \"ignore_unmapped\" : false,\n" +
                 "    \"boost\" : 1.0\n" +

+ 18 - 18
core/src/test/java/org/elasticsearch/search/sort/GeoDistanceSortBuilderIT.java

@@ -99,32 +99,32 @@ public class GeoDistanceSortBuilderIT extends ESIntegTestCase {
                 .addSort(new GeoDistanceSortBuilder(LOCATION_FIELD, q).sortMode(SortMode.MIN).order(SortOrder.ASC))
                 .execute().actionGet();
         assertOrderedSearchHits(searchResponse, "d1", "d2");
-        assertThat((Double)searchResponse.getHits().getAt(0).getSortValues()[0], closeTo(GeoDistance.SLOPPY_ARC.calculate(2, 2, 3, 2, DistanceUnit.METERS), 10d));
-        assertThat((Double)searchResponse.getHits().getAt(1).getSortValues()[0], closeTo(GeoDistance.SLOPPY_ARC.calculate(2, 1, 5, 1, DistanceUnit.METERS), 10d));
+        assertThat((Double)searchResponse.getHits().getAt(0).getSortValues()[0], closeTo(GeoDistance.ARC.calculate(2, 2, 3, 2, DistanceUnit.METERS), 10d));
+        assertThat((Double)searchResponse.getHits().getAt(1).getSortValues()[0], closeTo(GeoDistance.ARC.calculate(2, 1, 5, 1, DistanceUnit.METERS), 10d));
 
         searchResponse = client().prepareSearch()
                 .setQuery(matchAllQuery())
                 .addSort(new GeoDistanceSortBuilder(LOCATION_FIELD, q).sortMode(SortMode.MIN).order(SortOrder.DESC))
                 .execute().actionGet();
         assertOrderedSearchHits(searchResponse, "d2", "d1");
-        assertThat((Double)searchResponse.getHits().getAt(0).getSortValues()[0], closeTo(GeoDistance.SLOPPY_ARC.calculate(2, 1, 5, 1, DistanceUnit.METERS), 10d));
-        assertThat((Double)searchResponse.getHits().getAt(1).getSortValues()[0], closeTo(GeoDistance.SLOPPY_ARC.calculate(2, 2, 3, 2, DistanceUnit.METERS), 10d));
+        assertThat((Double)searchResponse.getHits().getAt(0).getSortValues()[0], closeTo(GeoDistance.ARC.calculate(2, 1, 5, 1, DistanceUnit.METERS), 10d));
+        assertThat((Double)searchResponse.getHits().getAt(1).getSortValues()[0], closeTo(GeoDistance.ARC.calculate(2, 2, 3, 2, DistanceUnit.METERS), 10d));
 
         searchResponse = client().prepareSearch()
                 .setQuery(matchAllQuery())
                 .addSort(new GeoDistanceSortBuilder(LOCATION_FIELD, q).sortMode(SortMode.MAX).order(SortOrder.ASC))
                 .execute().actionGet();
         assertOrderedSearchHits(searchResponse, "d1", "d2");
-        assertThat((Double)searchResponse.getHits().getAt(0).getSortValues()[0], closeTo(GeoDistance.SLOPPY_ARC.calculate(2, 2, 4, 1, DistanceUnit.METERS), 10d));
-        assertThat((Double)searchResponse.getHits().getAt(1).getSortValues()[0], closeTo(GeoDistance.SLOPPY_ARC.calculate(2, 1, 6, 2, DistanceUnit.METERS), 10d));
+        assertThat((Double)searchResponse.getHits().getAt(0).getSortValues()[0], closeTo(GeoDistance.ARC.calculate(2, 2, 4, 1, DistanceUnit.METERS), 10d));
+        assertThat((Double)searchResponse.getHits().getAt(1).getSortValues()[0], closeTo(GeoDistance.ARC.calculate(2, 1, 6, 2, DistanceUnit.METERS), 10d));
 
         searchResponse = client().prepareSearch()
                 .setQuery(matchAllQuery())
                 .addSort(new GeoDistanceSortBuilder(LOCATION_FIELD, q).sortMode(SortMode.MAX).order(SortOrder.DESC))
                 .execute().actionGet();
         assertOrderedSearchHits(searchResponse, "d2", "d1");
-        assertThat((Double)searchResponse.getHits().getAt(0).getSortValues()[0], closeTo(GeoDistance.SLOPPY_ARC.calculate(2, 1, 6, 2, DistanceUnit.METERS), 10d));
-        assertThat((Double)searchResponse.getHits().getAt(1).getSortValues()[0], closeTo(GeoDistance.SLOPPY_ARC.calculate(2, 2, 4, 1, DistanceUnit.METERS), 10d));
+        assertThat((Double)searchResponse.getHits().getAt(0).getSortValues()[0], closeTo(GeoDistance.ARC.calculate(2, 1, 6, 2, DistanceUnit.METERS), 10d));
+        assertThat((Double)searchResponse.getHits().getAt(1).getSortValues()[0], closeTo(GeoDistance.ARC.calculate(2, 2, 4, 1, DistanceUnit.METERS), 10d));
     }
 
     public void testSingeToManyAvgMedian() throws ExecutionException, InterruptedException, IOException {
@@ -157,16 +157,16 @@ public class GeoDistanceSortBuilderIT extends ESIntegTestCase {
                 .addSort(new GeoDistanceSortBuilder(LOCATION_FIELD, q).sortMode(SortMode.AVG).order(SortOrder.ASC))
                 .execute().actionGet();
         assertOrderedSearchHits(searchResponse, "d2", "d1");
-        assertThat((Double)searchResponse.getHits().getAt(0).getSortValues()[0], closeTo(GeoDistance.SLOPPY_ARC.calculate(0, 0, 0, 4, DistanceUnit.METERS), 10d));
-        assertThat((Double)searchResponse.getHits().getAt(1).getSortValues()[0], closeTo(GeoDistance.SLOPPY_ARC.calculate(0, 0, 0, 5, DistanceUnit.METERS), 10d));
+        assertThat((Double)searchResponse.getHits().getAt(0).getSortValues()[0], closeTo(GeoDistance.ARC.calculate(0, 0, 0, 4, DistanceUnit.METERS), 10d));
+        assertThat((Double)searchResponse.getHits().getAt(1).getSortValues()[0], closeTo(GeoDistance.ARC.calculate(0, 0, 0, 5, DistanceUnit.METERS), 10d));
 
         searchResponse = client().prepareSearch()
                 .setQuery(matchAllQuery())
                 .addSort(new GeoDistanceSortBuilder(LOCATION_FIELD, q).sortMode(SortMode.MEDIAN).order(SortOrder.ASC))
                 .execute().actionGet();
         assertOrderedSearchHits(searchResponse, "d1", "d2");
-        assertThat((Double)searchResponse.getHits().getAt(0).getSortValues()[0], closeTo(GeoDistance.SLOPPY_ARC.calculate(0, 0, 0, 4, DistanceUnit.METERS), 10d));
-        assertThat((Double)searchResponse.getHits().getAt(1).getSortValues()[0], closeTo(GeoDistance.SLOPPY_ARC.calculate(0, 0, 0, 5, DistanceUnit.METERS), 10d));
+        assertThat((Double)searchResponse.getHits().getAt(0).getSortValues()[0], closeTo(GeoDistance.ARC.calculate(0, 0, 0, 4, DistanceUnit.METERS), 10d));
+        assertThat((Double)searchResponse.getHits().getAt(1).getSortValues()[0], closeTo(GeoDistance.ARC.calculate(0, 0, 0, 5, DistanceUnit.METERS), 10d));
     }
 
     protected void createShuffeldJSONArray(XContentBuilder builder, GeoPoint[] pointsArray) throws IOException {
@@ -238,16 +238,16 @@ public class GeoDistanceSortBuilderIT extends ESIntegTestCase {
                 .addSort(geoDistanceSortBuilder.sortMode(SortMode.MIN).order(SortOrder.ASC))
                 .execute().actionGet();
         assertOrderedSearchHits(searchResponse, "d1", "d2");
-        assertThat((Double) searchResponse.getHits().getAt(0).getSortValues()[0], closeTo(GeoDistance.SLOPPY_ARC.calculate(2.5, 1, 2, 1, DistanceUnit.METERS), 1.e-1));
-        assertThat((Double) searchResponse.getHits().getAt(1).getSortValues()[0], closeTo(GeoDistance.SLOPPY_ARC.calculate(4.5, 1, 2, 1, DistanceUnit.METERS), 1.e-1));
+        assertThat((Double) searchResponse.getHits().getAt(0).getSortValues()[0], closeTo(GeoDistance.ARC.calculate(2.5, 1, 2, 1, DistanceUnit.METERS), 1.e-1));
+        assertThat((Double) searchResponse.getHits().getAt(1).getSortValues()[0], closeTo(GeoDistance.ARC.calculate(4.5, 1, 2, 1, DistanceUnit.METERS), 1.e-1));
 
         searchResponse = client().prepareSearch()
                 .setQuery(matchAllQuery())
                 .addSort(geoDistanceSortBuilder.sortMode(SortMode.MAX).order(SortOrder.ASC))
                 .execute().actionGet();
         assertOrderedSearchHits(searchResponse, "d1", "d2");
-        assertThat((Double) searchResponse.getHits().getAt(0).getSortValues()[0], closeTo(GeoDistance.SLOPPY_ARC.calculate(3.25, 4, 2, 1, DistanceUnit.METERS), 1.e-1));
-        assertThat((Double) searchResponse.getHits().getAt(1).getSortValues()[0], closeTo(GeoDistance.SLOPPY_ARC.calculate(5.25, 4, 2, 1, DistanceUnit.METERS), 1.e-1));
+        assertThat((Double) searchResponse.getHits().getAt(0).getSortValues()[0], closeTo(GeoDistance.ARC.calculate(3.25, 4, 2, 1, DistanceUnit.METERS), 1.e-1));
+        assertThat((Double) searchResponse.getHits().getAt(1).getSortValues()[0], closeTo(GeoDistance.ARC.calculate(5.25, 4, 2, 1, DistanceUnit.METERS), 1.e-1));
 
     }
 
@@ -314,8 +314,8 @@ public class GeoDistanceSortBuilderIT extends ESIntegTestCase {
 
     private static void checkCorrectSortOrderForGeoSort(SearchResponse searchResponse) {
         assertOrderedSearchHits(searchResponse, "d2", "d1");
-        assertThat((Double) searchResponse.getHits().getAt(0).getSortValues()[0], closeTo(GeoDistance.SLOPPY_ARC.calculate(2, 2, 1, 2, DistanceUnit.METERS), 1.e-1));
-        assertThat((Double) searchResponse.getHits().getAt(1).getSortValues()[0], closeTo(GeoDistance.SLOPPY_ARC.calculate(2, 2, 1, 1, DistanceUnit.METERS), 1.e-1));
+        assertThat((Double) searchResponse.getHits().getAt(0).getSortValues()[0], closeTo(GeoDistance.ARC.calculate(2, 2, 1, 2, DistanceUnit.METERS), 1.e-1));
+        assertThat((Double) searchResponse.getHits().getAt(1).getSortValues()[0], closeTo(GeoDistance.ARC.calculate(2, 2, 1, 1, DistanceUnit.METERS), 1.e-1));
     }
 
     protected void createQPoints(List<String> qHashes, List<GeoPoint> qPoints) {

+ 5 - 5
core/src/test/java/org/elasticsearch/search/sort/GeoDistanceSortBuilderTests.java

@@ -256,7 +256,7 @@ public class GeoDistanceSortBuilderTests extends AbstractSortTestCase<GeoDistanc
             assertEquals("illegal latitude value [269.384765625] for [GeoDistanceSort] for field [reverse].", e.getMessage());
         }
     }
-    
+
     public void testCoerceIsDeprecated() throws IOException {
         String json = "{\n" +
                 "  \"testname\" : [ {\n" +
@@ -264,7 +264,7 @@ public class GeoDistanceSortBuilderTests extends AbstractSortTestCase<GeoDistanc
                 "    \"lon\" : -51.94128329747579\n" +
                 "  } ],\n" +
                 "  \"unit\" : \"m\",\n" +
-                "  \"distance_type\" : \"sloppy_arc\",\n" +
+                "  \"distance_type\" : \"arc\",\n" +
                 "  \"mode\" : \"MAX\",\n" +
                 "  \"coerce\" : true\n" +
                 "}";
@@ -283,7 +283,7 @@ public class GeoDistanceSortBuilderTests extends AbstractSortTestCase<GeoDistanc
                 "    \"lon\" : -51.94128329747579\n" +
                 "  } ],\n" +
                 "  \"unit\" : \"m\",\n" +
-                "  \"distance_type\" : \"sloppy_arc\",\n" +
+                "  \"distance_type\" : \"arc\",\n" +
                 "  \"mode\" : \"MAX\",\n" +
                 "  \"ignore_malformed\" : true\n" +
                 "}";
@@ -302,7 +302,7 @@ public class GeoDistanceSortBuilderTests extends AbstractSortTestCase<GeoDistanc
                 "    \"lon\" : -51.94128329747579\n" +
                 "  } ],\n" +
                 "  \"unit\" : \"m\",\n" +
-                "  \"distance_type\" : \"sloppy_arc\",\n" +
+                "  \"distance_type\" : \"arc\",\n" +
                 "  \"mode\" : \"SUM\"\n" +
                 "}";
         XContentParser itemParser = createParser(JsonXContent.jsonXContent, json);
@@ -319,7 +319,7 @@ public class GeoDistanceSortBuilderTests extends AbstractSortTestCase<GeoDistanc
                 "    \"VDcvDuFjE\" : [ \"7umzzv8eychg\", \"dmdgmt5z13uw\", " +
                 "    \"ezu09wxw6v4c\", \"kc7s3515p6k6\", \"jgeuvjwrmfzn\", \"kcpcfj7ruyf8\" ],\n" +
                 "    \"unit\" : \"m\",\n" +
-                "    \"distance_type\" : \"sloppy_arc\",\n" +
+                "    \"distance_type\" : \"arc\",\n" +
                 "    \"mode\" : \"MAX\",\n" +
                 "    \"nested_filter\" : {\n" +
                 "      \"ids\" : {\n" +

+ 1 - 1
docs/reference/aggregations/bucket/geodistance-aggregation.asciidoc

@@ -83,7 +83,7 @@ By default, the distance unit is `m` (metres) but it can also accept: `mi` (mile
 
 <1> The distances will be computed as miles
 
-There are three distance calculation modes: `sloppy_arc` (the default), `arc` (most accurate) and `plane` (fastest). The `arc` calculation is the most accurate one but also the more expensive one in terms of performance. The `sloppy_arc` is faster but less accurate. The `plane` is the fastest but least accurate distance function. Consider using `plane` when your search context is "narrow" and spans smaller geographical areas (like cities or even countries). `plane` may return higher error margins for searches across very large areas (e.g. cross continent search). The distance calculation type can be set using the `distance_type` parameter:
+There are two distance calculation modes: `arc` (the default), and `plane`. The `arc` calculation is the most accurate. The `plane` is the fastest but least accurate. Consider using `plane` when your search context is "narrow", and spans smaller geographical areas (~5km). `plane` will return higher error margins for searches across very large areas (e.g. cross continent search). The distance calculation type can be set using the `distance_type` parameter:
 
 [source,js]
 --------------------------------------------------

+ 1 - 1
docs/reference/query-dsl/geo-distance-query.asciidoc

@@ -191,7 +191,7 @@ The following are options allowed on the filter:
 
 `distance_type`::
 
-    How to compute the distance. Can either be `sloppy_arc` (default), `arc` (slightly more precise but significantly slower) or `plane` (faster, but inaccurate on long distances and close to the poles).
+    How to compute the distance. Can either be `arc` (default), or `plane` (faster, but inaccurate on long distances and close to the poles).
 
 `optimize_bbox`::
 

+ 2 - 2
docs/reference/search/request/sort.asciidoc

@@ -229,7 +229,7 @@ GET /_search
                 "order" : "asc",
                 "unit" : "km",
 		"mode" : "min",
-		"distance_type" : "sloppy_arc"
+		"distance_type" : "arc"
             }
         }
     ],
@@ -244,7 +244,7 @@ GET /_search
 
 `distance_type`::
 
-    How to compute the distance. Can either be `sloppy_arc` (default), `arc` (slightly more precise but significantly slower) or `plane` (faster, but inaccurate on long distances and close to the poles).
+    How to compute the distance. Can either be `arc` (default), or `plane` (faster, but inaccurate on long distances and close to the poles).
 
 `mode`::