-
Notifications
You must be signed in to change notification settings - Fork 16
Conversation
Installed via homebrew.
Does the fact that this passed mean that accelerate passes all of the tests? I had thought it was failing some of them, but maybe some of the tests have changed since then. |
The idamax nan tests have been changed to just say that something went wrong but still pass - see https://travis-ci.org/gonum/blas/jobs/108194557#L111 |
How did you solve the LAPACKE/Accelerate issue, @jonlawlor ? |
I haven't yet, this pull is just for running OpenBLAS on osx. My plan with LAPACKE is to link the reference libs against Accelerate, I'll be trying it out tonight. So is there some problem with the idamax test? Should this be passing? |
When I checked on Travis earlier today, the Accelerate build was passing,
|
The Accelerate builds pass on gonum/blas but not on gonum/lapack; see https://travis-ci.org/gonum/lapack/builds/107916819 The reason the gonum/blas builds pass is that at some point the idamax tests were changed to pass with an output to the log when a NaN is returned in place of the expected value. See https://github.com/gonum/blas/blob/master/testblas/level1double.go#L1437 I don't know what is right in that case - it could be an undefined output, because we are asking for the index of the max of a vector with a NaN in it. See for example this discussion: http://grouper.ieee.org/groups/754/email/msg00610.html where they note that BLAS predates NaN and that they leave its treatment to the implementation subject to performance. |
For a discussion on "overconstraining implementers", see OpenMathLib/OpenBLAS#624 We should probably remove the NaN tests. Accelerate seems to be failing on lapack because of linking error. |
Yes, Accelerate doesn't include the LAPACKE libs for some reason. A possible way forward is to build the reference lapack and link it against accelerate blas. I don't know how much utility a test like that would have. |
@kortschak may see it differently, but I'm personally more interested in independent tests of the lapack functions, and not that interested in just swapping in blas implementations. Our blas test suite is decent (though not perfect), and as time goes on the functions are better and better tested by lapack. In contrast, we really only have two different lapack implementations which are highly correlated -- the netlib lapack functions and my translations of them. If there even exist alternate implementaitons it would be great to be able to use blas fuzz or something to stress test our implementations. |
@btracey then do you see any benefit to running the OpenBLAS tests on osx or are we just burning cycles? |
I think it's reasonable in the BLAS tests, but I don't see a need to go through gymnastics to test the same lapack implementation with a different BLAS backing. |
OK, then what I'd like to do is add in the install script for building gonum/lapack against accelerate but exclude it from the build matrix explicitly, because it is nice to have the procedure written down and testable someplace. |
So is this pull LGTM? |
LGTM |
Argh, somehow the builds were passing yesterday but are failing now during the OpenBLAS build. I'm going to disable the osx openblas builds until we can use a different install method than homebrew, I guess. This might be related to load on travis servers. |
Installed via homebrew. It takes about twice as long as the linux builds for OpenBLAS, because
brew install
also runs a bunch of tests, which may be redundant. Follows #161PTAL @btracey @vladimir-ch