Skip to content

Commit

Permalink
Use two bounding boxes for circles touching the date line
Browse files Browse the repository at this point in the history 
Index seeks with distance queries where the circle touches the date line
are now planned with two small bounding boxes at either side of the
date line and concatenating the results, instead of having one
than bounding box extending over all longitudes.
  • Loading branch information
@sherfert
sherfert committed Mar 22, 2018
1 parent 4e7a3b6 commit c867e80
Show file tree
Hide file tree
Showing 4 changed files with 175 additions and 42 deletions.
14 changes: 8 additions & 6 deletions .../src/main/scala/org/neo4j/cypher/internal/runtime/interpreted/pipes/NodeIndexSeeker.scala
View file
Expand Up @@ -29,6 +29,8 @@ import org.neo4j.values.AnyValue
import org.neo4j.values.storable._
import org.neo4j.values.virtual.NodeValue

import collection.JavaConverters._

/**
* Mixin trait with functionality for executing logical index queries.
*
Expand Down Expand Up @@ -122,12 +124,12 @@ trait NodeIndexSeeker {
val valueRange = range.map(expr => makeValueNeoSafe(expr(row, state)))
(valueRange.distance, valueRange.point) match {
case (distance: NumberValue, point: PointValue) =>
val bbox = point.getCoordinateReferenceSystem.getCalculator.boundingBox(point, distance.doubleValue())
List(List(IndexQuery.range(propertyIds.head,
bbox.first(),
range.inclusive,
bbox.other(),
range.inclusive
val bboxes = point.getCoordinateReferenceSystem.getCalculator.boundingBox(point, distance.doubleValue()).asScala
bboxes.map( bbox => List(IndexQuery.range(propertyIds.head,
bbox.first(),
range.inclusive,
bbox.other(),
range.inclusive
)))
case _ => Nil
}
Expand Down
122 changes: 98 additions & 24 deletions ...neo4j/cypher/internal/runtime/interpreted/commands/expressions/DistanceFunctionTest.scala
View file
Expand Up @@ -23,15 +23,22 @@ import org.neo4j.cypher.internal.util.v3_4.test_helpers.CypherFunSuite
import org.neo4j.values.storable.{CRSCalculator, CoordinateReferenceSystem, PointValue, Values}
import org.scalactic.{Equality, TolerantNumerics}
import org.scalatest.matchers.{MatchResult, Matcher}
import collection.JavaConverters._

import scala.language.implicitConversions

class DistanceFunctionTest extends CypherFunSuite {

implicit def javaToScalaPair(pair: org.neo4j.helpers.collection.Pair[PointValue, PointValue]): (PointValue, PointValue) = (pair.first(), pair.other())

def boundingBox(center: PointValue, distance: Double): (PointValue, PointValue) =
center.getCoordinateReferenceSystem.getCalculator.boundingBox(center, distance)
def boundingBox(center: PointValue, distance: Double): Seq[(PointValue, PointValue)] =
center.getCoordinateReferenceSystem.getCalculator.boundingBox(center, distance).asScala.map(pair => (pair.first(), pair.other()))

def boundingBoxLengthOne(center: PointValue, distance: Double): (PointValue, PointValue) = {
val boxes = boundingBox(center, distance)
boxes should have length 1
boxes.head
}

def distance(p1: PointValue, p2: PointValue): Double =
p1.getCoordinateReferenceSystem.getCalculator.distance(p1, p2)
Expand All @@ -57,7 +64,7 @@ class DistanceFunctionTest extends CypherFunSuite {
for (point <- points; distance <- distances) {
val calculator = point.getCoordinateReferenceSystem.getCalculator
withClue(s"Calculating bounding box with distance $distance of $point\n") {
val (bottomLeft, topRight) = boundingBox(point, distance)
val boxes = boundingBox(point, distance)
var minLat = Double.MaxValue
var maxLat = Double.MinValue
var minLong = Double.MaxValue
Expand All @@ -66,7 +73,7 @@ class DistanceFunctionTest extends CypherFunSuite {
// Test that points on the circle lie inside the bounding box
for (brng <- 0.0 to 2.0 * Math.PI by 0.01) {
val dest = destinationPoint(point, distance, brng)
dest should beInsideBoundingBox(bottomLeft, topRight, tolerant = true)
dest should beInsideOneBoundingBox(boxes, tolerant = true)
val destLat = dest.coordinate()(1)
val destLong = dest.coordinate()(0)

Expand All @@ -82,22 +89,28 @@ class DistanceFunctionTest extends CypherFunSuite {
val delta = 1.0

if (!southPoleIncluded) {
makePoint(minLong, minLat - delta) shouldNot beInsideBoundingBox(bottomLeft, topRight)
makePoint(minLong, minLat - delta) shouldNot beInsideOneBoundingBox(boxes)
}
if (!northPoleIncluded) {
makePoint(maxLong, maxLat + delta) shouldNot beInsideBoundingBox(bottomLeft, topRight)
makePoint(maxLong, maxLat + delta) shouldNot beInsideOneBoundingBox(boxes)
}
if (!northPoleIncluded && !southPoleIncluded) {
makePoint(minLong - delta, minLat) shouldNot beInsideBoundingBox(bottomLeft, topRight)
makePoint(maxLong + delta, maxLat) shouldNot beInsideBoundingBox(bottomLeft, topRight)
makePoint(minLong - delta, minLat) shouldNot beInsideOneBoundingBox(boxes)
makePoint(maxLong + delta, maxLat) shouldNot beInsideOneBoundingBox(boxes)
}

// Special cases where poles are included
if (northPoleIncluded) {
boxes should have length 1
val (bottomLeft, topRight) = boxes.head

bottomLeft.coordinate()(0) should be(-180)
topRight.coordinate()(0) should be(180)
topRight.coordinate()(1) should be(90)
} else if (southPoleIncluded) {
boxes should have length 1
val (bottomLeft, topRight) = boxes.head

bottomLeft.coordinate()(0) should be(-180)
bottomLeft.coordinate()(1) should be(-90)
topRight.coordinate()(0) should be(180)
Expand All @@ -108,16 +121,37 @@ class DistanceFunctionTest extends CypherFunSuite {

test("distance zero bounding box returns same point in WGS84") {
val point = Values.pointValue(CoordinateReferenceSystem.WGS84, 0, 0)
val (bottomLeft, topRight) = boundingBox(point, 0.0)
val (bottomLeft, topRight) = boundingBoxLengthOne(point, 0.0)
bottomLeft should equal(point)
topRight should equal(point)
}

test("bounding box touching the date line should extend to the whole longitude in WGS84") {
test("bounding box touching the date line from west") {
val point = Values.pointValue(CoordinateReferenceSystem.WGS84, -180, 0)
val (bottomLeft, topRight) = boundingBox(point, 1000.0)
bottomLeft.coordinate()(0) should be(-180)
topRight.coordinate()(0) should be(180)
val boxes = boundingBox(point, 1000.0)
boxes should have length 2
val ((bottomLeft1, topRight1), (bottomLeft2, topRight2)) = (boxes.head, boxes(1))
bottomLeft1.coordinate()(0) shouldBe >(0.0)
topRight1.coordinate()(0) should be(180)
bottomLeft2.coordinate()(0) should be(-180)
topRight2.coordinate()(0) shouldBe <(0.0)

bottomLeft1.coordinate()(1) should be(bottomLeft2.coordinate()(1))
topRight1.coordinate()(1) should be(topRight2.coordinate()(1))
}

test("bounding box touching the date line from east") {
val point = Values.pointValue(CoordinateReferenceSystem.WGS84, 180, 0)
val boxes = boundingBox(point, 1000.0)
boxes should have length 2
val ((bottomLeft1, topRight1), (bottomLeft2, topRight2)) = (boxes.head, boxes(1))
bottomLeft1.coordinate()(0) shouldBe >(0.0)
topRight1.coordinate()(0) should be(180)
bottomLeft2.coordinate()(0) should be(-180)
topRight2.coordinate()(0) shouldBe <(0.0)

bottomLeft1.coordinate()(1) should be(bottomLeft2.coordinate()(1))
topRight1.coordinate()(1) should be(topRight2.coordinate()(1))
}

test("distance should account for wraparound in longitude in WGS84") {
Expand All @@ -129,15 +163,15 @@ class DistanceFunctionTest extends CypherFunSuite {

test("bounding box including the north pole should be extended to all longitudes in WGS84") {
val farNorth = Values.pointValue(CoordinateReferenceSystem.WGS84, 0, 90.0)
val (bottomLeft, topRight) = boundingBox(farNorth, 100.0)
val (bottomLeft, topRight) = boundingBoxLengthOne(farNorth, 100.0)
bottomLeft.coordinate()(0) should be(-180)
topRight.coordinate()(0) should be(180)
topRight.coordinate()(1) should be(90)
}

test("bounding box including the south pole should be extended to all longitudes in WGS84") {
val farSouth = Values.pointValue(CoordinateReferenceSystem.WGS84, 0, -90.0)
val (bottomLeft, topRight) = boundingBox(farSouth, 100.0)
val (bottomLeft, topRight) = boundingBoxLengthOne(farSouth, 100.0)
bottomLeft.coordinate()(0) should be(-180)
bottomLeft.coordinate()(1) should be(-90)
topRight.coordinate()(0) should be(180)
Expand All @@ -153,16 +187,47 @@ class DistanceFunctionTest extends CypherFunSuite {

test("distance zero bounding box returns same point in WGS84-3D") {
val point = Values.pointValue(CoordinateReferenceSystem.WGS84_3D, 0, 0, 0)
val (bottomLeft, topRight) = boundingBox(point, 0.0)
val (bottomLeft, topRight) = boundingBoxLengthOne(point, 0.0)
bottomLeft should equal(point)
topRight should equal(point)
}

test("bounding box touching the date line should extend to the whole longitude in WGS84-3D") {
test("bounding box touching the date line from west in WGS84-3D") {
val point = Values.pointValue(CoordinateReferenceSystem.WGS84_3D, -180, 0, 0)
val (bottomLeft, topRight) = boundingBox(point, 1000.0)
bottomLeft.coordinate()(0) should be(-180)
topRight.coordinate()(0) should be(180)
val boxes = boundingBox(point, 1000.0)
boxes should have length 2
val ((bottomLeft1, topRight1), (bottomLeft2, topRight2)) = (boxes.head, boxes(1))
bottomLeft1.coordinate()(0) shouldBe >(0.0)
topRight1.coordinate()(0) should be(180)
bottomLeft2.coordinate()(0) should be(-180)
topRight2.coordinate()(0) shouldBe <(0.0)

bottomLeft1.coordinate()(1) should be(bottomLeft2.coordinate()(1))
topRight1.coordinate()(1) should be(topRight2.coordinate()(1))

bottomLeft1.coordinate()(2) should be(-1000)
bottomLeft2.coordinate()(2) should be(-1000)
topRight1.coordinate()(2) should be(1000)
topRight2.coordinate()(2) should be(1000)
}

test("bounding box touching the date line from east in WGS84-3D") {
val point = Values.pointValue(CoordinateReferenceSystem.WGS84_3D, 180, 0, 0)
val boxes = boundingBox(point, 1000.0)
boxes should have length 2
val ((bottomLeft1, topRight1), (bottomLeft2, topRight2)) = (boxes.head, boxes(1))
bottomLeft1.coordinate()(0) shouldBe >(0.0)
topRight1.coordinate()(0) should be(180)
bottomLeft2.coordinate()(0) should be(-180)
topRight2.coordinate()(0) shouldBe <(0.0)

bottomLeft1.coordinate()(1) should be(bottomLeft2.coordinate()(1))
topRight1.coordinate()(1) should be(topRight2.coordinate()(1))

bottomLeft1.coordinate()(2) should be(-1000)
bottomLeft2.coordinate()(2) should be(-1000)
topRight1.coordinate()(2) should be(1000)
topRight2.coordinate()(2) should be(1000)
}

test("distance should account for wraparound in longitude in WGS84-3D") {
Expand All @@ -174,15 +239,15 @@ class DistanceFunctionTest extends CypherFunSuite {

test("bounding box including the north pole should be extended to all longitudes in WGS84-3D") {
val farNorth = Values.pointValue(CoordinateReferenceSystem.WGS84_3D, 0, 90.0, 0)
val (bottomLeft, topRight) = boundingBox(farNorth, 100.0)
val (bottomLeft, topRight) = boundingBoxLengthOne(farNorth, 100.0)
bottomLeft.coordinate()(0) should be(-180)
topRight.coordinate()(0) should be(180)
topRight.coordinate()(1) should be(90)
}

test("bounding box including the south pole should be extended to all longitudes in WGS84-3D") {
val farSouth = Values.pointValue(CoordinateReferenceSystem.WGS84_3D, 0, -90.0, 0)
val (bottomLeft, topRight) = boundingBox(farSouth, 100.0)
val (bottomLeft, topRight) = boundingBoxLengthOne(farSouth, 100.0)
bottomLeft.coordinate()(0) should be(-180)
bottomLeft.coordinate()(1) should be(-90)
topRight.coordinate()(0) should be(180)
Expand All @@ -199,7 +264,7 @@ class DistanceFunctionTest extends CypherFunSuite {
test("bounding box should gives reasonable results in WGS84") {
implicit val doubleEquality = TolerantNumerics.tolerantDoubleEquality(0.0001)
val malmo = Values.pointValue(CoordinateReferenceSystem.WGS84, 13.0, 56.0)
val (bottomLeft, topRight) = boundingBox(malmo, 1000.0)
val (bottomLeft, topRight) = boundingBoxLengthOne(malmo, 1000.0)
bottomLeft.coordinate()(0) should equal(12.984)
bottomLeft.coordinate()(1) should equal(55.991)
topRight.coordinate()(0) should equal(13.016)
Expand All @@ -209,7 +274,7 @@ class DistanceFunctionTest extends CypherFunSuite {
test("bounding box should consider height in WGS84-3D") {
implicit val doubleEquality = TolerantNumerics.tolerantDoubleEquality(0.0001)
val malmo = Values.pointValue(CoordinateReferenceSystem.WGS84_3D, 13.0, 56.0, 1000)
val (bottomLeft, topRight) = boundingBox(malmo, 1000.0)
val (bottomLeft, topRight) = boundingBoxLengthOne(malmo, 1000.0)
bottomLeft.coordinate()(0) should equal(12.984)
bottomLeft.coordinate()(1) should equal(55.991)
bottomLeft.coordinate()(2) should equal(0.0)
Expand Down Expand Up @@ -253,6 +318,15 @@ class DistanceFunctionTest extends CypherFunSuite {
}
}

private def beInsideOneBoundingBox(boxes: Seq[(PointValue, PointValue)], tolerant: Boolean = false): Matcher[PointValue] = new Matcher[PointValue] {
override def apply(point: PointValue): MatchResult = {
MatchResult(
matches = boxes.exists { case (bottomLeft, topRight) => insideBoundingBox(point, bottomLeft, topRight, tolerant) },
rawFailureMessage = s"$point should be inside one of $boxes, but was not.",
rawNegatedFailureMessage = s"$point should not be inside one of $boxes, but was.")
}
}

// from https://www.movable-type.co.uk/scripts/latlong.html
private def destinationPoint(startingPoint: PointValue, d: Double, brng: Double): PointValue = {
if (d == 0.0) {
Expand Down
41 changes: 29 additions & 12 deletions community/values/src/main/java/org/neo4j/values/storable/CRSCalculator.java
View file
Expand Up @@ -19,6 +19,10 @@
*/
package org.neo4j.values.storable;

import java.util.Arrays;
import java.util.Collections;
import java.util.List;

import org.neo4j.helpers.collection.Pair;

import static java.lang.Math.asin;
Expand All @@ -34,7 +38,7 @@ public abstract class CRSCalculator
{
public abstract double distance( PointValue p1, PointValue p2 );

public abstract Pair<PointValue,PointValue> boundingBox( PointValue center, double distance );
public abstract List<Pair<PointValue,PointValue>> boundingBox( PointValue center, double distance );

protected static double pythagoras( double[] a, double[] b )
{
Expand Down Expand Up @@ -65,7 +69,7 @@ public double distance( PointValue p1, PointValue p2 )
}

@Override
public Pair<PointValue,PointValue> boundingBox( PointValue center, double distance )
public List<Pair<PointValue,PointValue>> boundingBox( PointValue center, double distance )
{
assert center.getCoordinateReferenceSystem().getDimension() == dimension;
double[] coordinates = center.coordinate();
Expand All @@ -77,7 +81,7 @@ public Pair<PointValue,PointValue> boundingBox( PointValue center, double distan
max[i] = coordinates[i] + distance;
}
CoordinateReferenceSystem crs = center.getCoordinateReferenceSystem();
return Pair.of( Values.pointValue( crs, min ), Values.pointValue( crs, max ) );
return Collections.singletonList( Pair.of( Values.pointValue( crs, min ), Values.pointValue( crs, max ) ) );
}
}

Expand Down Expand Up @@ -128,11 +132,11 @@ public double distance( PointValue p1, PointValue p2 )
@Override
// http://janmatuschek.de/LatitudeLongitudeBoundingCoordinates
// But calculating in degrees instead of radians to avoid rounding errors
public Pair<PointValue,PointValue> boundingBox( PointValue center, double distance )
public List<Pair<PointValue,PointValue>> boundingBox( PointValue center, double distance )
{
if ( distance == 0.0 )
{
return Pair.of( center, center );
return Collections.singletonList( Pair.of( center, center ) );
}

// Extend the distance slightly to assure that all relevant points lies inside the bounding box,
Expand All @@ -151,11 +155,11 @@ public Pair<PointValue,PointValue> boundingBox( PointValue center, double distan
// If your query circle includes one of the poles
if ( latMax >= 90 )
{
return boundingBoxOf( -180, 180, latMin, 90, center, distance );
return Collections.singletonList( boundingBoxOf( -180, 180, latMin, 90, center, distance ) );
}
else if ( latMin <= -90 )
{
return boundingBoxOf( -180, 180, -90, latMax, center, distance );
return Collections.singletonList( boundingBoxOf( -180, 180, -90, latMax, center, distance ) );
}
else
{
Expand All @@ -164,15 +168,28 @@ else if ( latMin <= -90 )
double lonMax = lon + deltaLon;

// If you query circle wraps around the dateline
// Large rectangle covering all longitudes
// TODO implement two rectangle solution instead
if ( lonMin < -180 || lonMax > 180 )
if ( lonMin < -180 && lonMax > 180 )
{
// Large rectangle covering all longitudes
return Collections.singletonList( boundingBoxOf( -180, 180, latMin, latMax, center, distance ) );
}
else if ( lonMin < -180 )
{
// two small rectangles east and west of dateline
Pair<PointValue,PointValue> box1 = boundingBoxOf( lonMin + 360, 180, latMin, latMax, center, distance );
Pair<PointValue,PointValue> box2 = boundingBoxOf( -180, lonMax, latMin, latMax, center, distance );
return Arrays.asList( box1, box2 );
}
else if ( lonMax > 180 )
{
return boundingBoxOf( -180, 180, latMin, latMax, center, distance );
// two small rectangles east and west of dateline
Pair<PointValue,PointValue> box1 = boundingBoxOf( lonMin, 180, latMin, latMax, center, distance );
Pair<PointValue,PointValue> box2 = boundingBoxOf( -180, lonMax - 360, latMin, latMax, center, distance );
return Arrays.asList( box1, box2 );
}
else
{
return boundingBoxOf( lonMin, lonMax, latMin, latMax, center, distance );
return Collections.singletonList( boundingBoxOf( lonMin, lonMax, latMin, latMax, center, distance ) );
}
}
}
Expand Down

0 comments on commit c867e80

Please sign in to comment.