search

gonum / matrix

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

Add MulTri and fix bugs in list_test #415

Closed btracey closed 7 years ago

btracey commented 7 years ago

This exposed some unexpected bugs, so I wanted to keep the actual implementation simple as far as fast paths.

btracey commented 7 years ago

Here meaning in the comments?

kortschak commented 7 years ago

Here being a PR comment.

btracey commented 7 years ago

Bugs fixed in list_test: 1) Made the testMatrices have a non-zero size. Triangular types don't have to return a valid TriKind when the have zero size. list_test tried to be careful with this, and it was, but when it comes to actually testing triangular methods, it's very useful to have them be non-zero size so that the Triangular method can be called on them. For example, in MulTri, all of the matrices need to have the same TriKind. The easiest way to check this for the legalTypes function is to call Triangular(), but originally the test matrices have 0 size, so this is not a valid check. It would be possible to check triType directly somehow, but there is no real harm in giving them an actual size.

2) Fixed the order in the switch statement in retranspose. We would like the returned matrix to inherit the most specific transposed type possible. For example, our TransposeTri is both a Transposer and a TransposeTrier. When the matrix got re-transposed, it no longer implemented Triangular (since it was only a Transposer). The ordering change fixes that.

3) maybeSame only checks that the types and sizes matched. For triangular types, it also needs to check that the kinds are the same.

4) The test when both input matrices could shadow the receiver was wrong. It assigned bSame to the receiver, and then checked that bSame was an untransposer. This is not correct, since the receiver can never be untransposed. The question is if the original matrix was transposed. I don't think it has any effect on this test since a triangular matrix can't be shadowed by its transpose, but I noticed the bug while debugging and fixed it. It didn't cause any failures before because we re-compute the answer, so if you have a failed transpose it just gives a different answer. Also, basically the only way this can happen is for the matrix to be square, so there are no size mismatch issues.

5) It also fixed some small "bugs" in switch statements that hadn't yet been coded for a TriDense receiver.