Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merge NETLIB packages #2

Merged
merged 1,427 commits into from
May 25, 2017
Merged

Merge NETLIB packages #2

merged 1,427 commits into from
May 25, 2017

Conversation

kortschak
Copy link
Member

No description provided.

vladimir-ch and others added 30 commits October 14, 2016 00:38
This requires that s side parameters be changed back to side to avoid
name collision.
Updates #142
Cleaned up coverage script and fixed travis.yml bug. See gonum/integrate#33.
Cleaned up coverage script and fixed travis.yml bug. See gonum/integrate#33.
Cleaned up coverage script and fixed travis.yml bug. See gonum/integrate#33.
@kortschak kortschak requested a review from btracey May 25, 2017 02:24
@kortschak kortschak force-pushed the merge_three branch 7 times, most recently from 9c28b1f to 601080c Compare May 25, 2017 04:13
@kortschak
Copy link
Member Author

This passes on linux/OpenBLAS but fails on osx/OpenBLAS with a variety of errors. I'm not in a position to debug the osx failures or try alternative backing implementations, so I have just excluded all possibilities for osx. The fixes for this can come from someone who has the appropriate hardware and will.

@btracey Please take a look.

/cc @jonlawlor

@kortschak
Copy link
Member Author

To debug locally invoke

bash # The test scripts terminate the session hard on linux - this may not be necessary on osx.
CGO_LDFLAGS="-L/usr/local -lopenblas" source .travis/osx/OpenBLAS/test.sh

@btracey
Copy link
Member

btracey commented May 25, 2017

This is what I get

brendan:~/Documents/mygo/src/gonum.org/v1/netlib/lapack$ CGO_LDFLAGS="-L/Users/brendan/software/OpenBLAS -lopenblas" go test
--- FAIL: TestDlaswp (0.00s)
	dlaswp.go:126: Case 2 (m=5,n=3,k1=0,k2=2,extra=0): unexpected A
		{5 3 3 [4 5 1 7 8 9 2 6 1 10 11 12 13 14 15]}
		{5 3 3 [4 5 6 7 8 9 10 11 12 1 2 3 13 14 15]}
	dlaswp.go:126: Case 3 (m=5,n=3,k1=0,k2=2,extra=0): unexpected A
		{5 3 3 [2 3 8 1 7 3 4 5 6 10 11 12 13 14 15]}
		{5 3 3 [10 11 12 1 2 3 4 5 6 7 8 9 13 14 15]}
2017/05/24 23:02:20 Dgecon one norm mismatch. Want 0.024054837369015203, got 0.020928829230190113.
--- FAIL: TestDormlq (0.00s)
	dorml2.go:139: Multiplication mismatch. IsLeft = false. IsTrans = false
	dorml2.go:139: Multiplication mismatch. IsLeft = false. IsTrans = false
	dorml2.go:139: Multiplication mismatch. IsLeft = false. IsTrans = false
	dorml2.go:139: Multiplication mismatch. IsLeft = false. IsTrans = true
	dorml2.go:139: Multiplication mismatch. IsLeft = false. IsTrans = true
	dorml2.go:139: Multiplication mismatch. IsLeft = false. IsTrans = true
FAIL

@btracey
Copy link
Member

btracey commented May 25, 2017

Which also happen on the old repo

@btracey
Copy link
Member

btracey commented May 25, 2017

(Which does not happen in the native implementation)

@btracey
Copy link
Member

btracey commented May 25, 2017

Turns out I hadn't pulled from OpenBlas in a while. When I updated my OpenBLAS installation, the tests all pass on OSX. Is it possible that's the problem on travis?

@btracey
Copy link
Member

btracey commented May 25, 2017

Ah, I wonder if this is a problem with brew not having the latest version. We currently have brew install homebrew/science/openblas. Can we pull from the git directory directly like we do in linux? This way we are using the same version in both operating systems.

@kortschak
Copy link
Member Author

I can try that.

@kortschak
Copy link
Member Author

That will depend on xcode presumably. I don't want to go there.

@btracey
Copy link
Member

btracey commented May 25, 2017

Why would it depend on Xcode? On my mac I do

git clone https://github.com/xianyi/OpenBLAS
cd OpenBLAS
make

The only reason this would need Xcode is for the compilers, which I would imagine travis has without xcode.

@kortschak
Copy link
Member Author

The compiler is sort of important for make and I'd be surprised if fortran was present on the osx machines (we use apt to get gfortran on the linux builders). I'll try, but it will feel even worse than cooking in someone else's kitchen - unless the kitchen was just hit by a cyclone.

@btracey
Copy link
Member

btracey commented May 25, 2017

That's right, I forgot that line was there in travis. The alternative I guess is to not test on osx?

@kortschak
Copy link
Member Author

There are two alternatives. I happy with either.

  1. Don't test on osx.
  2. Don't test lapack on osx (this is what we did previously and now just involves splitting the testing infrastructure up a bit - I'm fine with that but I'd rather get this PR in first so we can move on with the gonum repo).

@btracey
Copy link
Member

btracey commented May 25, 2017

I think it would be nice to keep testing Accelerate for BLAS, as it's an independent implementation and has helped finding bugs. I'm okay with not testing lapack on osx. It's the same program that we/I recommend installing from source, so there hopefully aren't OS specific issues.

@btracey
Copy link
Member

btracey commented May 25, 2017

LGTM

@kortschak
Copy link
Member Author

kortschak commented May 25, 2017 via email

@kortschak kortschak merged commit c33417d into master May 25, 2017
@kortschak kortschak deleted the merge_three branch May 25, 2017 12:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants