diff --git a/s2/predicates.go b/s2/predicates.go index c872525..700604e 100644 --- a/s2/predicates.go +++ b/s2/predicates.go @@ -145,11 +145,11 @@ func RobustSign(a, b, c Point) Direction { // points are rare in practice so it seems better to simply fall back to // exact arithmetic in that case. func stableSign(a, b, c Point) Direction { - ab := a.Sub(b.Vector) + ab := b.Sub(a.Vector) ab2 := ab.Norm2() - bc := b.Sub(c.Vector) + bc := c.Sub(b.Vector) bc2 := bc.Norm2() - ca := c.Sub(a.Vector) + ca := a.Sub(c.Vector) ca2 := ca.Norm2() // Now compute the determinant ((A-C)x(B-C)).C, where the vertices have been @@ -170,7 +170,7 @@ func stableSign(a, b, c Point) Direction { e1, e2, op = bc, ab, b.Vector } - det := e1.Cross(e2).Dot(op) + det := -e1.Cross(e2).Dot(op) maxErr := detErrorMultiplier * math.Sqrt(e1.Norm2()*e2.Norm2()) // If the determinant isn't zero, within maxErr, we know definitively the point ordering. diff --git a/s2/predicates_test.go b/s2/predicates_test.go index 95252b6..c30efa7 100644 --- a/s2/predicates_test.go +++ b/s2/predicates_test.go @@ -17,6 +17,7 @@ limitations under the License. package s2 import ( + "math" "testing" "github.com/golang/geo/r3" @@ -235,6 +236,54 @@ func TestPredicatesRobustSign(t *testing.T) { */ } +func TestPredicatesStableSignFailureRate(t *testing.T) { + const earthRadiusKm = 6371.01 + const iters = 1000 + + // Verify that stableSign is able to handle most cases where the three + // points are as collinear as possible. (For reference, triageSign fails + // almost 100% of the time on this test.) + // + // Note that the failure rate *decreases* as the points get closer together, + // and the decrease is approximately linear. For example, the failure rate + // is 0.4% for collinear points spaced 1km apart, but only 0.0004% for + // collinear points spaced 1 meter apart. + // + // 1km spacing: < 1% (actual is closer to 0.4%) + // 10km spacing: < 10% (actual is closer to 4%) + want := 0.01 + spacing := 1.0 + + // Estimate the probability that stableSign will not be able to compute + // the determinant sign of a triangle A, B, C consisting of three points + // that are as collinear as possible and spaced the given distance apart + // by counting up the times it returns Indeterminate. + failureCount := 0 + m := math.Tan(spacing / earthRadiusKm) + for iter := 0; iter < iters; iter++ { + f := randomFrame() + a := f.col(0) + x := f.col(1) + + b := Point{a.Sub(x.Mul(m)).Normalize()} + c := Point{a.Add(x.Mul(m)).Normalize()} + sign := stableSign(a, b, c) + if sign != Indeterminate { + // TODO(roberts): Once exactSign is implemented, uncomment this case. + //if got := exactSign(a, b, c, true); got != sign { + // t.Errorf("exactSign(%v, %v, %v, true) = %v, want %v", a, b, c, got, sign) + //} + } else { + failureCount++ + } + } + + rate := float64(failureCount) / float64(iters) + if rate >= want { + t.Errorf("stableSign failure rate for spacing %v km = %v, want %v", spacing, rate, want) + } +} + func BenchmarkSign(b *testing.B) { p1 := PointFromCoords(-3, -1, 4) p2 := PointFromCoords(2, -1, -3)