gonum / blas

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

add OpenBLAS build for osx #163

Closed jonlawlor closed 8 years ago

jonlawlor commented 8 years ago

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 #161

PTAL @btracey @vladimir-ch

btracey commented 8 years ago

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.

jonlawlor commented 8 years ago

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

vladimir-ch commented 8 years ago

How did you solve the LAPACKE/Accelerate issue, @jonlawlor ?

jonlawlor commented 8 years ago

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?

vladimir-ch commented 8 years ago

I haven't yet, this pull is just for running OpenBLAS on osx.

When I checked on Travis earlier today, the Accelerate build was passing, so I'm just wondering. I probably missed something.

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?

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

jonlawlor commented 8 years ago

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.

btracey commented 8 years ago

For a discussion on "overconstraining implementers", see https://github.com/xianyi/OpenBLAS/issues/624

We should probably remove the NaN tests.

Accelerate seems to be failing on lapack because of linking error.

jonlawlor commented 8 years ago

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.

btracey commented 8 years ago

@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.

jonlawlor commented 8 years ago

@btracey then do you see any benefit to running the OpenBLAS tests on osx or are we just burning cycles?

btracey commented 8 years ago

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.

jonlawlor commented 8 years ago

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.

jonlawlor commented 8 years ago

So is this pull LGTM?

btracey commented 8 years ago

LGTM

jonlawlor commented 8 years ago

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.