diff --git a/core/src/test/java/org/elasticsearch/common/geo/GeoDistanceTests.java b/core/src/test/java/org/elasticsearch/common/geo/GeoDistanceTests.java index 6624a91a6c2..c2e62277da7 100644 --- a/core/src/test/java/org/elasticsearch/common/geo/GeoDistanceTests.java +++ b/core/src/test/java/org/elasticsearch/common/geo/GeoDistanceTests.java @@ -81,29 +81,62 @@ public class GeoDistanceTests extends ESTestCase { 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); - GeoPoint westernPoint = new GeoPoint(48.85265, 2.308896); + private static double arcDistance(GeoPoint p1, GeoPoint p2) { + return GeoDistance.ARC.calculate(p1.lat(), p1.lon(), p2.lat(), p2.lon(), DistanceUnit.METERS); + } - // With GeoDistance.ARC both the northern and western points are within the 4km range - assertThat(GeoDistance.ARC.calculate(centre.lat(), centre.lon(), northernPoint.lat(), - northernPoint.lon(), DistanceUnit.KILOMETERS), lessThan(4D)); - assertThat(GeoDistance.ARC.calculate(centre.lat(), centre.lon(), westernPoint.lat(), - westernPoint.lon(), DistanceUnit.KILOMETERS), lessThan(4D)); + private static double planeDistance(GeoPoint p1, GeoPoint p2) { + return GeoDistance.PLANE.calculate(p1.lat(), p1.lon(), p2.lat(), p2.lon(), DistanceUnit.METERS); + } - // With GeoDistance.PLANE, only the northern point is within the 4km range, - // the western point is outside of the range due to the simple math it employs, - // meaning results will appear elliptical - assertThat(GeoDistance.PLANE.calculate(centre.lat(), centre.lon(), northernPoint.lat(), - northernPoint.lon(), DistanceUnit.KILOMETERS), lessThan(4D)); - assertThat(GeoDistance.PLANE.calculate(centre.lat(), centre.lon(), westernPoint.lat(), - westernPoint.lon(), DistanceUnit.KILOMETERS), greaterThan(4D)); + public void testArcDistanceVsPlane() { + // sameLongitude and sameLatitude are both 90 degrees away from basePoint along great circles + final GeoPoint basePoint = new GeoPoint(45, 90); + final GeoPoint sameLongitude = new GeoPoint(-45, 90); + final GeoPoint sameLatitude = new GeoPoint(45, -90); + + double sameLongitudeArcDistance = arcDistance(basePoint, sameLongitude); + double sameLatitudeArcDistance = arcDistance(basePoint, sameLatitude); + double sameLongitudePlaneDistance = planeDistance(basePoint, sameLongitude); + double sameLatitudePlaneDistance = planeDistance(basePoint, sameLatitude); + + // GeoDistance.PLANE measures the distance along a straight line in + // (lat, long) space so agrees with GeoDistance.ARC along a line of + // constant longitude but takes a longer route if there is east/west + // movement. + + assertThat("Arc and plane should agree on sameLongitude", + Math.abs(sameLongitudeArcDistance - sameLongitudePlaneDistance), lessThan(0.001)); + + assertThat("Arc and plane should disagree on sameLatitude (by >4000km)", + sameLatitudePlaneDistance - sameLatitudeArcDistance, greaterThan(4.0e6)); + + // GeoDistance.ARC calculates the great circle distance (on a sphere) so these should agree as they're both 90 degrees + assertThat("Arc distances should agree", Math.abs(sameLongitudeArcDistance - sameLatitudeArcDistance), lessThan(0.001)); + } + + public void testArcDistanceVsPlaneAccuracy() { + // These points only differ by a few degrees so the calculation methods + // should match more closely. Check that the deviation is small enough, + // but not too small. + + // The biggest deviations are away from the equator and the poles so pick a suitably troublesome latitude. + GeoPoint basePoint = new GeoPoint(randomDoubleBetween(30.0, 60.0, true), randomDoubleBetween(-180.0, 180.0, true)); + GeoPoint sameLongitude = new GeoPoint(randomDoubleBetween(-90.0, 90.0, true), basePoint.lon()); + GeoPoint sameLatitude = new GeoPoint(basePoint.lat(), basePoint.lon() + randomDoubleBetween(4.0, 10.0, true)); + + double sameLongitudeArcDistance = arcDistance(basePoint, sameLongitude); + double sameLatitudeArcDistance = arcDistance(basePoint, sameLatitude); + double sameLongitudePlaneDistance = planeDistance(basePoint, sameLongitude); + double sameLatitudePlaneDistance = planeDistance(basePoint, sameLatitude); + + assertThat("Arc and plane should agree [" + basePoint + "] to [" + sameLongitude + "] (within 1cm)", + Math.abs(sameLongitudeArcDistance - sameLongitudePlaneDistance), lessThan(0.01)); + + assertThat("Arc and plane should very roughly agree [" + basePoint + "] to [" + sameLatitude + "]", + sameLatitudePlaneDistance - sameLatitudeArcDistance, lessThan(600.0)); + + assertThat("Arc and plane should disagree by some margin [" + basePoint + "] to [" + sameLatitude + "]", + sameLatitudePlaneDistance - sameLatitudeArcDistance, greaterThan(15.0)); } }