gonum / blas

A BLAS implementation for Go [DEPRECATED]
172 stars 16 forks source link

3fd5f breaks Dtrsm via gonum/matrix/mat64 tests #14

Closed kortschak closed 10 years ago

kortschak commented 10 years ago

It seems we didn't test cblas in the context of mat64 for 3fd5f072eea9adabff997d01c097a8b50baaf200; there was no commit to matrix reflecting the changes in blas. I have made those changes and now we get an out of range in TestSolve and TestLQD. TestSolve hides the panic, but TestLQD shows it to be in the Dgemv. This is not observed in a36aa1f1cd12c499eee3ef55ffeb0d0dbd6cdce7.

btracey commented 10 years ago

I do not see this panic on my machine, either with cblas or goblas. Which cblas implementation are you using?

On Jun 5, 2014, at 5:08 PM, kortschak notifications@github.com wrote:

It seems we didn't test cblas in the context of mat64 for 3fd5f07; there was no commit to matrix reflecting the changes in blas. I have made those changes and now we get an out of range in TestSolve and TestLQD. TestLQD hides the panic, but TestSolve shows it to be in the Dgemv. This is not observed in a36aa1f.

— Reply to this email directly or view it on GitHub.

kortschak commented 10 years ago

OK, this is down to the clever use of blas.ColMajor in mat64.LQFactor.applyQTo(). cblas looks fine - mat64 needs fixing.

btracey commented 10 years ago

I lied. Ignore my comment about the tests passing. Did you push the matrix changes up? I still see them in dense_arithmetic (for example).

On Jun 5, 2014, at 5:35 PM, kortschak notifications@github.com wrote:

OK, this is down to the clever use of blas.ColMajor in mat64.LQFactor.applyQTo(). cblas looks fine - mat64 needs fixing.

— Reply to this email directly or view it on GitHub.

kortschak commented 10 years ago

No, I have made not changes to the upstream master - I'd like to get this fixed first, but it might be something for @dane-unltd since he wrote that code. I'll see what I can do.

kortschak commented 10 years ago

I believe I have fixed this. Will close.