diff --git a/docs/reference/query-dsl/filters/geo-distance-filter.asciidoc b/docs/reference/query-dsl/filters/geo-distance-filter.asciidoc index ab2e8b1b80338..f5013983066d9 100644 --- a/docs/reference/query-dsl/filters/geo-distance-filter.asciidoc +++ b/docs/reference/query-dsl/filters/geo-distance-filter.asciidoc @@ -148,8 +148,9 @@ The following are options allowed on the filter: `distance_type`:: - How to compute the distance. Can either be `arc` (better precision) or - `plane` (faster). Defaults to `arc`. + How to compute the distance. Can either be `arc` (better precision), + `sloppy_arc` (faster but less precise) or `plane` (fastest). Defaults + to `sloppy_arc`. `optimize_bbox`:: diff --git a/docs/reference/search/aggregations/bucket/geodistance-aggregation.asciidoc b/docs/reference/search/aggregations/bucket/geodistance-aggregation.asciidoc index d9c9a86c8e35e..6d7e8c3dff871 100644 --- a/docs/reference/search/aggregations/bucket/geodistance-aggregation.asciidoc +++ b/docs/reference/search/aggregations/bucket/geodistance-aggregation.asciidoc @@ -80,7 +80,7 @@ By default, the distance unit is `km` but it can also accept: `mi` (miles), `in` <1> The distances will be computed as miles -There are two distance calculation modes: `arc` (the default) and `plane`. The `arc` calculation is the most accurate one but also the more expensive one in terms of performance. The `plane` is faster but less accurate. Consider using `plane` when your search context is "narrow" and spans smaller geographical areas (like cities or even countries). `plane` may return higher error mergins 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: `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 mergins 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] -------------------------------------------------- diff --git a/docs/reference/search/facets/geo-distance-facet.asciidoc b/docs/reference/search/facets/geo-distance-facet.asciidoc index 49533b76c1b78..f6db797393950 100644 --- a/docs/reference/search/facets/geo-distance-facet.asciidoc +++ b/docs/reference/search/facets/geo-distance-facet.asciidoc @@ -172,7 +172,7 @@ itself. be `mi`, `miles`, `in`, `inch`, `yd`, `yards`, `kilometers`, `mm`, `millimeters`, `cm`, `centimeters`, `m` or `meters`. |`distance_type` |How to compute the distance. Can either be `arc` -(better precision) or `plane` (faster). Defaults to `arc`. +(better precision), `sloppy_arc` (faster) or `plane` (fastest). Defaults to `sloppy_arc`. |======================================================================= ==== Value Options diff --git a/src/main/java/org/elasticsearch/common/geo/GeoDistance.java b/src/main/java/org/elasticsearch/common/geo/GeoDistance.java index b26fb285dc7fd..6e49621d57f5e 100644 --- a/src/main/java/org/elasticsearch/common/geo/GeoDistance.java +++ b/src/main/java/org/elasticsearch/common/geo/GeoDistance.java @@ -23,6 +23,8 @@ import org.elasticsearch.ElasticSearchIllegalArgumentException; import org.elasticsearch.common.unit.DistanceUnit; +import java.util.Locale; + /** * Geo distance calculation. */ @@ -48,6 +50,7 @@ public FixedSourceDistance fixedSourceDistance(double sourceLatitude, double sou return new PlaneFixedSourceDistance(sourceLatitude, sourceLongitude, unit); } }, + /** * Calculates distance factor. */ @@ -71,12 +74,23 @@ public FixedSourceDistance fixedSourceDistance(double sourceLatitude, double sou } }, /** - * Calculates distance as points in a globe. + * Calculates distance as points on a globe. */ ARC() { @Override public double calculate(double sourceLatitude, double sourceLongitude, double targetLatitude, double targetLongitude, DistanceUnit unit) { - return unit.fromMeters(SloppyMath.haversin(sourceLatitude, sourceLongitude, targetLatitude, targetLongitude) * 1000.0); + double longitudeDifference = targetLongitude - sourceLongitude; + double a = Math.toRadians(90D - sourceLatitude); + double c = Math.toRadians(90D - targetLatitude); + double factor = (Math.cos(a) * Math.cos(c)) + (Math.sin(a) * Math.sin(c) * Math.cos(Math.toRadians(longitudeDifference))); + + if (factor < -1D) { + return unit.fromMeters(Math.PI * GeoUtils.EARTH_MEAN_RADIUS); + } else if (factor >= 1D) { + return 0; + } else { + return unit.fromMeters(Math.acos(factor) * GeoUtils.EARTH_MEAN_RADIUS); + } } @Override @@ -88,8 +102,35 @@ public double normalize(double distance, DistanceUnit unit) { 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. + */ + 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.haversin(sourceLatitude, sourceLongitude, targetLatitude, targetLongitude) * 1000.0); + } + + @Override + public FixedSourceDistance fixedSourceDistance(double sourceLatitude, double sourceLongitude, DistanceUnit unit) { + return new SloppyArcFixedSourceDistance(sourceLatitude, sourceLongitude, unit); + } }; + /** + * Default {@link GeoDistance} function. This method should be used, If no specific function has been selected. + * This is an alias for SLOPPY_ARC + */ + 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); @@ -134,15 +175,31 @@ public static DistanceBoundingCheck distanceBoundingCheck(double sourceLatitude, return new SimpleDistanceBoundingCheck(topLeft, bottomRight); } - public static GeoDistance fromString(String s) { - if ("plane".equals(s)) { + /** + * Get a {@link GeoDistance} according to a given name. Valid values are + * + * + * + * @param name name of the {@link GeoDistance} + * @return a {@link GeoDistance} + */ + public static GeoDistance fromString(String name) { + name = name.toLowerCase(Locale.ROOT); + if ("plane".equals(name)) { return PLANE; - } else if ("arc".equals(s)) { + } else if ("arc".equals(name)) { return ARC; - } else if ("factor".equals(s)) { + } else if ("sloppy_arc".equals(name)) { + return SLOPPY_ARC; + } else if ("factor".equals(name)) { return FACTOR; } - throw new ElasticSearchIllegalArgumentException("No geo distance for [" + s + "]"); + throw new ElasticSearchIllegalArgumentException("No geo distance for [" + name + "]"); } public static interface FixedSourceDistance { @@ -253,18 +310,14 @@ public double calculate(double targetLatitude, double targetLongitude) { public static class FactorFixedSourceDistance implements FixedSourceDistance { - private final double sourceLatitude; private final double sourceLongitude; - private final double earthRadius; private final double a; private final double sinA; private final double cosA; public FactorFixedSourceDistance(double sourceLatitude, double sourceLongitude, DistanceUnit unit) { - this.sourceLatitude = sourceLatitude; this.sourceLongitude = sourceLongitude; - this.earthRadius = unit.getEarthRadius(); this.a = Math.toRadians(90D - sourceLatitude); this.sinA = Math.sin(a); this.cosA = Math.cos(a); @@ -278,21 +331,44 @@ public double calculate(double targetLatitude, double targetLongitude) { } } - public static class ArcFixedSourceDistance implements FixedSourceDistance { - - private final double sourceLatitude; - private final double sourceLongitude; - private final DistanceUnit unit; + /** + * 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 static abstract class FixedSourceDistanceBase implements FixedSourceDistance { + protected final double sourceLatitude; + protected final double sourceLongitude; + protected final DistanceUnit unit; - public ArcFixedSourceDistance(double sourceLatitude, double sourceLongitude, 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 unit.fromMeters(SloppyMath.haversin(sourceLatitude, sourceLongitude, targetLatitude, targetLongitude) * 1000.0); + 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); } } } diff --git a/src/main/java/org/elasticsearch/index/query/GeoDistanceFilterParser.java b/src/main/java/org/elasticsearch/index/query/GeoDistanceFilterParser.java index 7e265b896d8c4..79cf42da0ef64 100644 --- a/src/main/java/org/elasticsearch/index/query/GeoDistanceFilterParser.java +++ b/src/main/java/org/elasticsearch/index/query/GeoDistanceFilterParser.java @@ -74,7 +74,7 @@ public Filter parse(QueryParseContext parseContext) throws IOException, QueryPar double distance = 0; Object vDistance = null; DistanceUnit unit = DistanceUnit.KILOMETERS; // default unit - GeoDistance geoDistance = GeoDistance.ARC; + GeoDistance geoDistance = GeoDistance.DEFAULT; String optimizeBbox = "memory"; boolean normalizeLon = true; boolean normalizeLat = true; diff --git a/src/main/java/org/elasticsearch/index/query/GeoDistanceRangeFilterParser.java b/src/main/java/org/elasticsearch/index/query/GeoDistanceRangeFilterParser.java index 4906f94e4f1e6..1117d05af0659 100644 --- a/src/main/java/org/elasticsearch/index/query/GeoDistanceRangeFilterParser.java +++ b/src/main/java/org/elasticsearch/index/query/GeoDistanceRangeFilterParser.java @@ -76,7 +76,7 @@ public Filter parse(QueryParseContext parseContext) throws IOException, QueryPar boolean includeLower = true; boolean includeUpper = true; DistanceUnit unit = DistanceUnit.KILOMETERS; // default unit - GeoDistance geoDistance = GeoDistance.ARC; + GeoDistance geoDistance = GeoDistance.DEFAULT; String optimizeBbox = "memory"; boolean normalizeLon = true; boolean normalizeLat = true; diff --git a/src/main/java/org/elasticsearch/index/query/functionscore/DecayFunctionParser.java b/src/main/java/org/elasticsearch/index/query/functionscore/DecayFunctionParser.java index d2675bfd9a42a..783bf018b20e5 100644 --- a/src/main/java/org/elasticsearch/index/query/functionscore/DecayFunctionParser.java +++ b/src/main/java/org/elasticsearch/index/query/functionscore/DecayFunctionParser.java @@ -270,7 +270,7 @@ static class GeoFieldDataScoreFunction extends AbstractDistanceScoreFunction { private final IndexGeoPointFieldData fieldData; private GeoPointValues geoPointValues = null; - private static final GeoDistance distFunction = GeoDistance.fromString("arc"); + private static final GeoDistance distFunction = GeoDistance.DEFAULT; public GeoFieldDataScoreFunction(GeoPoint origin, double scale, double decay, double offset, DecayFunction func, IndexGeoPointFieldData fieldData) { diff --git a/src/main/java/org/elasticsearch/search/aggregations/bucket/range/geodistance/GeoDistanceParser.java b/src/main/java/org/elasticsearch/search/aggregations/bucket/range/geodistance/GeoDistanceParser.java index b72889e98c4aa..04de291b0bd2d 100644 --- a/src/main/java/org/elasticsearch/search/aggregations/bucket/range/geodistance/GeoDistanceParser.java +++ b/src/main/java/org/elasticsearch/search/aggregations/bucket/range/geodistance/GeoDistanceParser.java @@ -68,7 +68,7 @@ public AggregatorFactory parse(String aggregationName, XContentParser parser, Se List ranges = null; GeoPoint origin = null; DistanceUnit unit = DistanceUnit.KILOMETERS; - GeoDistance distanceType = GeoDistance.ARC; + GeoDistance distanceType = GeoDistance.DEFAULT; boolean keyed = false; XContentParser.Token token; diff --git a/src/main/java/org/elasticsearch/search/facet/geodistance/GeoDistanceFacetParser.java b/src/main/java/org/elasticsearch/search/facet/geodistance/GeoDistanceFacetParser.java index 6678888e9dfa4..03a199be5d1ee 100644 --- a/src/main/java/org/elasticsearch/search/facet/geodistance/GeoDistanceFacetParser.java +++ b/src/main/java/org/elasticsearch/search/facet/geodistance/GeoDistanceFacetParser.java @@ -22,7 +22,6 @@ import com.google.common.collect.Lists; import org.elasticsearch.common.component.AbstractComponent; import org.elasticsearch.common.geo.GeoDistance; -import org.elasticsearch.common.geo.GeoHashUtils; import org.elasticsearch.common.geo.GeoPoint; import org.elasticsearch.common.geo.GeoUtils; import org.elasticsearch.common.inject.Inject; @@ -32,7 +31,6 @@ import org.elasticsearch.index.fielddata.IndexGeoPointFieldData; import org.elasticsearch.index.fielddata.IndexNumericFieldData; import org.elasticsearch.index.mapper.FieldMapper; -import org.elasticsearch.index.mapper.geo.GeoPointFieldMapper; import org.elasticsearch.search.facet.FacetExecutor; import org.elasticsearch.search.facet.FacetParser; import org.elasticsearch.search.facet.FacetPhaseExecutionException; @@ -77,7 +75,7 @@ public FacetExecutor parse(String facetName, XContentParser parser, SearchContex Map params = null; GeoPoint point = new GeoPoint(); DistanceUnit unit = DistanceUnit.KILOMETERS; - GeoDistance geoDistance = GeoDistance.ARC; + GeoDistance geoDistance = GeoDistance.DEFAULT; List entries = Lists.newArrayList(); boolean normalizeLon = true; diff --git a/src/main/java/org/elasticsearch/search/sort/GeoDistanceSortParser.java b/src/main/java/org/elasticsearch/search/sort/GeoDistanceSortParser.java index 1387cec2a2b1b..6c256f9f13e92 100644 --- a/src/main/java/org/elasticsearch/search/sort/GeoDistanceSortParser.java +++ b/src/main/java/org/elasticsearch/search/sort/GeoDistanceSortParser.java @@ -54,7 +54,7 @@ public SortField parse(XContentParser parser, SearchContext context) throws Exce String fieldName = null; GeoPoint point = new GeoPoint(); DistanceUnit unit = DistanceUnit.KILOMETERS; - GeoDistance geoDistance = GeoDistance.ARC; + GeoDistance geoDistance = GeoDistance.DEFAULT; boolean reverse = false; SortMode sortMode = null; String nestedPath = null; diff --git a/src/test/java/org/apache/lucene/util/SloppyMathTests.java b/src/test/java/org/apache/lucene/util/SloppyMathTests.java index 798b0ff7a6bba..8ac10dd767823 100644 --- a/src/test/java/org/apache/lucene/util/SloppyMathTests.java +++ b/src/test/java/org/apache/lucene/util/SloppyMathTests.java @@ -20,18 +20,17 @@ package org.apache.lucene.util; import org.elasticsearch.common.geo.GeoDistance; -import org.elasticsearch.common.geo.GeoUtils; import org.elasticsearch.common.unit.DistanceUnit; import org.elasticsearch.test.ElasticsearchTestCase; import org.junit.Test; import static org.hamcrest.number.IsCloseTo.closeTo; -public class SloppyMathTests extends ElasticsearchTestCase { +public class SloppyMathTests extends ElasticsearchTestCase { @Test public void testAccuracy() { - for (double lat1 = -90; lat1 <= 90; lat1+=1) { + for (double lat1 = -89; lat1 <= 89; lat1+=1) { final double lon1 = randomLongitude(); for (double i = -180; i <= 180; i+=1) { @@ -45,12 +44,12 @@ public void testAccuracy() { @Test public void testSloppyMath() { - assertThat(GeoDistance.ARC.calculate(-46.645, -171.057, -46.644, -171.058, DistanceUnit.METERS), closeTo(134.87709, maxError(134.87709))); - assertThat(GeoDistance.ARC.calculate(-77.912, -81.173, -77.912, -81.171, DistanceUnit.METERS), closeTo(46.57161, maxError(46.57161))); - assertThat(GeoDistance.ARC.calculate(65.75, -20.708, 65.75, -20.709, DistanceUnit.METERS), closeTo(45.66996, maxError(45.66996))); - assertThat(GeoDistance.ARC.calculate(-86.9, 53.738, -86.9, 53.741, DistanceUnit.METERS), closeTo(18.03998, maxError(18.03998))); - assertThat(GeoDistance.ARC.calculate(89.041, 115.93, 89.04, 115.946, DistanceUnit.METERS), closeTo(115.11711, maxError(115.11711))); - + assertThat(GeoDistance.SLOPPY_ARC.calculate(-46.645, -171.057, -46.644, -171.058, DistanceUnit.METERS), closeTo(134.87709, maxError(134.87709))); + assertThat(GeoDistance.SLOPPY_ARC.calculate(-77.912, -81.173, -77.912, -81.171, DistanceUnit.METERS), closeTo(46.57161, maxError(46.57161))); + assertThat(GeoDistance.SLOPPY_ARC.calculate(65.75, -20.708, 65.75, -20.709, DistanceUnit.METERS), closeTo(45.66996, maxError(45.66996))); + assertThat(GeoDistance.SLOPPY_ARC.calculate(-86.9, 53.738, -86.9, 53.741, DistanceUnit.METERS), closeTo(18.03998, maxError(18.03998))); + assertThat(GeoDistance.SLOPPY_ARC.calculate(89.041, 115.93, 89.04, 115.946, DistanceUnit.METERS), closeTo(115.11711, maxError(115.11711))); + testSloppyMath(DistanceUnit.METERS, 0.01, 5, 45, 90); testSloppyMath(DistanceUnit.KILOMETERS, 0.01, 5, 45, 90); testSloppyMath(DistanceUnit.INCH, 0.01, 5, 45, 90); @@ -66,50 +65,34 @@ private void testSloppyMath(DistanceUnit unit, double...deltaDeg) { final double lon1 = randomLongitude(); logger.info("testing SloppyMath with {} at \"{}, {}\"", unit, lat1, lon1); - GeoDistance.ArcFixedSourceDistance src = new GeoDistance.ArcFixedSourceDistance(lat1, lon1, unit); - 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 + (randomDouble() - 0.5) * 2 * deltaDeg[test])); final double lon2 = lon1 + (randomDouble() - 0.5) * 2 * deltaDeg[test]; - final double lat2 = lat1 + (randomDouble() - 0.5) * 2 * deltaDeg[test]; - - final double accurate = unit.fromMeters(accurateHaversin(lat1, lon1, lat2, lon2)); - final double dist1 = GeoDistance.ARC.calculate(lat1, lon1, lat2, lon2, unit); - final double dist2 = src.calculate(lat2, lon2); + + 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+"))", dist1, closeTo(accurate, maxError(accurate))); - assertThat("distance between("+lat1+", "+lon1+") and ("+lat2+", "+lon2+"))", dist2, closeTo(accurate, maxError(accurate))); + assertThat("distance between("+lat1+", "+lon1+") and ("+lat2+", "+lon2+"))", dist, closeTo(accurate, maxError(accurate))); } } } - - // Slow but accurate implementation of the haversin function - private static double accurateHaversin(double lat1, double lon1, double lat2, double lon2) { - double longitudeDifference = lon2 - lon1; - double a = Math.toRadians(90D - lat1); - double c = Math.toRadians(90D - lat2); - double factor = (Math.cos(a) * Math.cos(c)) + (Math.sin(a) * Math.sin(c) * Math.cos(Math.toRadians(longitudeDifference))); - - if (factor < -1D) { - return Math.PI * GeoUtils.EARTH_MEAN_RADIUS; - } else if (factor >= 1D) { - return 0; - } else { - return Math.acos(factor) * GeoUtils.EARTH_MEAN_RADIUS; - } - } - + private static void assertAccurate(double lat1, double lon1, double lat2, double lon2) { - double accurate = accurateHaversin(lat1, lon1, lat2, lon2); - double sloppy = GeoDistance.ARC.calculate(lat1, lon1, lat2, lon2, DistanceUnit.METERS); + 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 final double randomLatitude() { - return (getRandom().nextDouble() - 0.5) * 180d; + // crop pole areas, sine we now there the function + // is not accurate around lat(89°, 90°) and lat(-90°, -89°) + return (getRandom().nextDouble() - 0.5) * 178.0; } private static final double randomLongitude() { - return (getRandom().nextDouble() - 0.5) * 360d; + return (getRandom().nextDouble() - 0.5) * 360.0; } }