gonum / matrix

Matrix packages for the Go language [DEPRECATED]
446 stars 53 forks source link

mat64: shadowing detection has false negatives #359

Closed kortschak closed 8 years ago

kortschak commented 8 years ago

The following test fails:

func TestIssue359(t *testing.T) {
    for xi := 0; xi < 2; xi++ {
        for xj := 0; xj < 2; xj++ {
            for yi := 0; yi < 2; yi++ {
                for yj := 0; yj < 2; yj++ {
                    a := NewDense(3, 3, []float64{
                        1, 2, 3,
                        4, 5, 6,
                        7, 8, 9,
                    })
                    x := a.View(xi, xj, 2, 2).(*Dense)
                    y := a.View(yi, yj, 2, 2).(*Dense)

                    panicked, _ := panics(func() { x.checkOverlap(y.mat) })
                    if !panicked {
                        t.Errorf("expected panic for aliased with offsets x(%d,%d) y(%d,%d):\nx:\n%v\ny:\n%v",
                            xi, xj, yi, yj, Formatted(x), Formatted(y),
                        )
                    }
                }
            }
        }
    }
}

It's pretty clear this is an off-by-one - which is mirrored in the test expectations that do exist.

$ go test -run 359
--- FAIL: TestIssue359 (0.00s)
    shadow_test.go:115: expected panic for aliased with offsets x(0,0) y(0,1):
        x:
        ⎡1  2⎤
        ⎣4  5⎦
        y:
        ⎡2  3⎤
        ⎣5  6⎦
    shadow_test.go:115: expected panic for aliased with offsets x(0,0) y(1,1):
        x:
        ⎡1  2⎤
        ⎣4  5⎦
        y:
        ⎡5  6⎤
        ⎣8  9⎦
    shadow_test.go:115: expected panic for aliased with offsets x(0,1) y(0,0):
        x:
        ⎡2  3⎤
        ⎣5  6⎦
        y:
        ⎡1  2⎤
        ⎣4  5⎦
    shadow_test.go:115: expected panic for aliased with offsets x(1,0) y(1,1):
        x:
        ⎡4  5⎤
        ⎣7  8⎦
        y:
        ⎡5  6⎤
        ⎣8  9⎦
    shadow_test.go:115: expected panic for aliased with offsets x(1,1) y(0,0):
        x:
        ⎡5  6⎤
        ⎣8  9⎦
        y:
        ⎡1  2⎤
        ⎣4  5⎦
    shadow_test.go:115: expected panic for aliased with offsets x(1,1) y(1,0):
        x:
        ⎡5  6⎤
        ⎣8  9⎦
        y:
        ⎡4  5⎤
        ⎣7  8⎦
FAIL
kortschak commented 8 years ago

I think the issue is when you have a column view where one partner is immediately left of and one down from the other. This was handled incorrectly in the current rectanglesOverlap code, but the correct approach is eluding me. Changing the final return to return bTo >= 0 is part of the fix, but this then fails the existing tests when the above conditions exist.