py-numpy: rework blas/lapack #2365

davydden · 2016-11-18T22:28:37Z

another part of #2361 plus a lots of extras.

tested py-numpy@1.11.2 by calling

spack activate py-numpy; spack load python;

followed by (see https://github.com/Homebrew/homebrew-python/blob/master/numpy.rb#L123-L126)

python -c 'import numpy as np; t = np.ones((3,3),int); assert t.sum() == 9; assert np.dot(t,t).sum() == 27; np.show_config()'

on...

macOS Sierra + clang:

spack install py-numpy+blas+lapack ^openblas

lapack_opt_info:
    libraries = ['openblas', 'openblas']
    library_dirs = ['/Users/davydden/spack/opt/spack/darwin-sierra-x86_64/clang-8.0.0-apple/openblas-0.2.19-7qdxwznmqzlbqtusssagtxkypivod6ct/lib']
    define_macros = [('HAVE_CBLAS', None)]
    language = c
blas_opt_info:
    libraries = ['openblas', 'openblas']
    library_dirs = ['/Users/davydden/spack/opt/spack/darwin-sierra-x86_64/clang-8.0.0-apple/openblas-0.2.19-7qdxwznmqzlbqtusssagtxkypivod6ct/lib']
    define_macros = [('HAVE_CBLAS', None)]
    language = c
openblas_info:
    libraries = ['openblas', 'openblas']
    library_dirs = ['/Users/davydden/spack/opt/spack/darwin-sierra-x86_64/clang-8.0.0-apple/openblas-0.2.19-7qdxwznmqzlbqtusssagtxkypivod6ct/lib']
    define_macros = [('HAVE_CBLAS', None)]
    language = c
openblas_lapack_info:
    libraries = ['openblas', 'openblas']
    library_dirs = ['/Users/davydden/spack/opt/spack/darwin-sierra-x86_64/clang-8.0.0-apple/openblas-0.2.19-7qdxwznmqzlbqtusssagtxkypivod6ct/lib']
    define_macros = [('HAVE_CBLAS', None)]
    language = c
blas_mkl_info:
  NOT AVAILABLE

Ubuntu 16.04 + GCC5.4:

spack install py-numpy+blas+lapack ^openblas

lapack_opt_info:
    libraries = ['openblas', 'openblas']
    library_dirs = ['/home/davydden/spack/opt/spack/linux-ubuntu16-x86_64/gcc-5.4.0/openblas-0.2.19-ehpxgz2j74uc7pbca5lnufmna4is3oqs/lib']
    define_macros = [('HAVE_CBLAS', None)]
    language = c
blas_opt_info:
    libraries = ['openblas', 'openblas']
    library_dirs = ['/home/davydden/spack/opt/spack/linux-ubuntu16-x86_64/gcc-5.4.0/openblas-0.2.19-ehpxgz2j74uc7pbca5lnufmna4is3oqs/lib']
    define_macros = [('HAVE_CBLAS', None)]
    language = c
openblas_info:
    libraries = ['openblas', 'openblas']
    library_dirs = ['/home/davydden/spack/opt/spack/linux-ubuntu16-x86_64/gcc-5.4.0/openblas-0.2.19-ehpxgz2j74uc7pbca5lnufmna4is3oqs/lib']
    define_macros = [('HAVE_CBLAS', None)]
    language = c
openblas_lapack_info:
    libraries = ['openblas', 'openblas']
    library_dirs = ['/home/davydden/spack/opt/spack/linux-ubuntu16-x86_64/gcc-5.4.0/openblas-0.2.19-ehpxgz2j74uc7pbca5lnufmna4is3oqs/lib']
    define_macros = [('HAVE_CBLAS', None)]
    language = c
blas_mkl_info:
  NOT AVAILABLE

spack install py-numpy+blas+lapack ^atlas (mix threaded and non-threaded libs due to upstream)

atlas_3_10_blas_threads_info:
    libraries = ['satlas', 'tatlas']
    library_dirs = ['/home/davydden/spack/opt/spack/linux-ubuntu16-x86_64/gcc-5.4.0/atlas-3.10.2-5jtibobvt7nlwd3zyviionsn7v4z2npd/lib']
    define_macros = [('HAVE_CBLAS', None), ('ATLAS_INFO', '"\\"3.10.2\\""')]
    language = c
lapack_opt_info:
    libraries = ['tatlas', 'satlas', 'tatlas']
    library_dirs = ['/home/davydden/spack/opt/spack/linux-ubuntu16-x86_64/gcc-5.4.0/atlas-3.10.2-5jtibobvt7nlwd3zyviionsn7v4z2npd/lib']
    define_macros = [('ATLAS_INFO', '"\\"3.10.2\\""')]
    language = f77
blas_opt_info:
    libraries = ['satlas', 'tatlas']
    library_dirs = ['/home/davydden/spack/opt/spack/linux-ubuntu16-x86_64/gcc-5.4.0/atlas-3.10.2-5jtibobvt7nlwd3zyviionsn7v4z2npd/lib']
    define_macros = [('HAVE_CBLAS', None), ('ATLAS_INFO', '"\\"3.10.2\\""')]
    language = c
openblas_info:
  NOT AVAILABLE
openblas_lapack_info:
  NOT AVAILABLE
atlas_3_10_threads_info:
    libraries = ['tatlas', 'satlas', 'tatlas']
    library_dirs = ['/home/davydden/spack/opt/spack/linux-ubuntu16-x86_64/gcc-5.4.0/atlas-3.10.2-5jtibobvt7nlwd3zyviionsn7v4z2npd/lib']
    define_macros = [('ATLAS_INFO', '"\\"3.10.2\\""')]
    language = f77
lapack_mkl_info:
  NOT AVAILABLE
blas_mkl_info:
  NOT AVAILABLE

spack install py-numpy+blas+lapack ^mkl (not working due to upstream)

lapack_info:
  NOT AVAILABLE
lapack_opt_info:
  NOT AVAILABLE
openblas_lapack_info:
  NOT AVAILABLE
blas_info:
  NOT AVAILABLE
atlas_3_10_blas_threads_info:
  NOT AVAILABLE
atlas_threads_info:
  NOT AVAILABLE
blas_src_info:
  NOT AVAILABLE
atlas_3_10_threads_info:
  NOT AVAILABLE
atlas_blas_info:
  NOT AVAILABLE
atlas_3_10_blas_info:
  NOT AVAILABLE
lapack_src_info:
  NOT AVAILABLE
atlas_blas_threads_info:
  NOT AVAILABLE
openblas_info:
  NOT AVAILABLE
blas_mkl_info:
  NOT AVAILABLE
blas_opt_info:
  NOT AVAILABLE
atlas_info:
  NOT AVAILABLE
atlas_3_10_info:
  NOT AVAILABLE
lapack_mkl_info:
  NOT AVAILABLE

@glennpj ping.

davydden · 2016-11-21T08:09:57Z

there appears to be a bug in numpy which leads to simultaneous usage of both single-threaded and multi-threaded Atlas (evident from above) numpy/numpy#8294 , but i don't think we can do much about it at this point.

davydden · 2016-11-21T14:09:52Z

@tgamblin @alalazo @adamjstewart ping.

glennpj · 2016-11-29T02:40:17Z

This does not work for MKL so the 'ready' label should be removed.

glennpj · 2016-11-29T03:42:42Z

Here is a snippet from the build log:

blas_opt_info:
blas_mkl_info:
  libraries mkl_intel_lp64,mkl_core,mkl_gnu_thread,pthread,m,dl not found in ['/home/gjohnson/spack/opt/spack/linux-centos7-x86_64/gcc-4.8.5/mkl-11.3.3.210-gkyo32sq4snk3r3z2qdxkac5lgtnlh7p', '/home/gjohnson/spack/opt/spack/linux-centos7-x86_64/gcc-4.8.5/mkl-11.3.3.210-gkyo32sq4snk3r3z2qdxkac5lgtnlh7p/include', '/home/gjohnson/spack/opt/spack/linux-centos7-x86_64/gcc-4.8.5/mkl-11.3.3.210-gkyo32sq4snk3r3z2qdxkac5lgtnlh7p/lib', '/home/gjohnson/spack/opt/spack/linux-centos7-x86_64/gcc-4.8.5/mkl-11.3.3.210-gkyo32sq4snk3r3z2qdxkac5lgtnlh7p/lib/intel64']
  NOT AVAILABLE

davydden · 2016-11-29T06:30:19Z

@glennpj

This does not work for MKL so the 'ready' label should be removed.

I fixed the error you reported, but numpy still does not pick up mkl properly. Frankly, i am not intended to fix it within this PR as I believe it is a problem with numpy's config system. It already fails to work properly even with a single Atlas lib, see numpy/numpy#8294. The authors are considering re-implementing the config:

After going through the whole set of site.cfg issues, I'm almost tempted to leave that as is for backwards compat and create a new .cfg config file with sane behavior. There are so many corner cases and people with workarounds for current behavior that it'll be very hard to fix otherwise.

Thereby, the PR will stay with ready attached. You may want to open issue upstream to help debugging the current or (hopefully) upcoming build system with MKL.

davydden · 2016-11-29T06:31:26Z

@tgamblin @becker33 ping.

glennpj · 2016-11-29T12:44:36Z

@davydden If you do not intend to have MKL working with this PR then you should raise an error if ^mkl is in the spec.

glennpj · 2016-11-29T12:51:24Z

I have to say that I disagree with the way MKL is being handled in Spack. Building numpy with MKL is possible using the mkl_rt library and this is documented by Intel, see PR #2361. One could make an argument that a lot of config/build systems are broken upstream. I thought one of the tenets of Spack was to work with build systems as they are and have spack provide convenience but not impose structure. The path here will have Spack without an MKL linked numpy/scipy for an indefinite amount of time.

tgamblin · 2016-11-29T13:06:18Z

@davydden: I actually agree with @glennpj on this one. If the PR breaks existing support for mkl, I think it's broken. Having numpy work with MKL is important to a lot of users. I haven't looked at the details yet, but is it not possible to get that working?

tgamblin · 2016-11-29T13:09:08Z

FWIW, testing get with %intel ^mkl will probably be one if he first combinatorial cases we add to package testing in CI.

alalazo · 2016-11-29T13:37:22Z

@glennpj @tgamblin

I have to say that I disagree with the way MKL is being handled in Spack. Building numpy with MKL is possible using the mkl_rt library and this is documented by Intel, see PR #2361. One could make an argument that a lot of config/build systems are broken upstream. I thought one of the tenets of Spack was to work with build systems as they are and have spack provide convenience but not impose structure.

I think I removed mkl_rt from intel-parallel-studio in #1682 . As far as I understand mkl_rt is just an handle to defer the choice of the ABI + threading-layer at run-time, and it is potentially dangerous in the sense that the actual libraries that you use depend on having specific environment variables set. This just to give an idea why it was removed there in favor of a set of libraries which are consistent with the variants of intel-parallel-studio.

There's one thing that is not clear to me from the discussion above: is using mkl_rt necessary for py-numpy (because it won't link in any other way) or are we just missing the correct way of passing a consistent linking line? In the former case I'll try to think of a way to re-add it in #1875 while I am polishing the PR.

tgamblin · 2016-11-29T14:38:16Z

@alalazo: thanks!

There's one thing that is not clear to me from the discussion above: is using mkl_rt necessary for py-numpy (because it won't link in any other way) or are we just missing the correct way of passing a consistent linking line?

I actually don't know the answer to this -- @glennpj do you?

davydden · 2016-11-29T16:05:19Z

@glennpj :

Building numpy with MKL is possible using the mkl_rt library and this is documented by Intel

yes, it is possible, but as @alalazo pointed out

mkl_rt is just an handle to defer the choice of the ABI + threading-layer at run-time, and it is potentially dangerous in the sense that the actual libraries that you use depend on having specific environment variables set.

One could make an argument that a lot of config/build systems are broken upstream.

I don't feel like Spack should give away the security of using one-blas-lapack-everywhere only to make numpy work with MKL. That's the whole point of Spack and graph of packages and conretization.

@tgamblin:

If the PR breaks existing support for mkl

it does not, MKL was not working, and still does not work.

@alalazo:

is using mkl_rt necessary for py-numpy (because it won't link in any other way) or are we just missing the correct way of passing a consistent linking line? In the former case I'll try to think of a way to re-add it in #1875 while I am polishing the PR.

i would not add it to MKL anyhow. I think it is dangerous and should be avoided by all means. The fact that some packages are broken (clearly numpy is an example), does not mean that we need to change default library lists for MKL. One can always hard-code mkl_rt here irrespectfully of what is actually returned as Blas/Lapack from MKL, but it is only needed because numpy is broken upstream. Again, there is nothing wrong in NOT using mkl_rt, and packages should support it.
Keep in mind that numpy does not respect input libs for Atlas either (see numpy/numpy#8294), but it does not mean that we should change the list of Blas/Lapack libraries provided by it.

p.s. either way, i do not plan to do anything with this PR, so feel free to merge it and then someone can take it further with numpy+mkl business.

alalazo · 2016-11-29T16:22:01Z

The fact that some packages are broken (clearly numpy is an example), does not mean that we need to change default library lists for MKL.

I don't mean to change the current default behavior. The changes in #1875 allow to say something like:

mkl_rt = spec['mkl:rt'].libs

and you'll get the mkl_rt library directly from the package that provides it and only if you ask explicitly.

Now: I would do it only if necessary to compile numpy with mkl and I still prefer a change in py-numpy to get this working. But I also think that having a numpy that compiles against mkl is a must at least for v1.0.

davydden · 2016-11-29T21:11:25Z

Now: I would do it only if necessary to compile numpy with mkl and I still prefer a change in py-numpy to get this working. But I also think that having a numpy that compiles against mkl is a must at least for v1.0.

Sound good to me!

Ps. Just to expand my answer why mkl_rt should be avoided by all means: imagine you build a DAG of packages where (I) one package need 32bit integers in Blas/Lapack (ii) another needs 64 (iii) yet another needs threading (iv) yet another needs serial version. As soon as you use mkl_rt, Spack would happily conferize the whole graph, whereas users of packages may get hard-to-debug segmentation faults. I think one of the key advantage of Spack is that it guarantees that one and only one version of a package is used in DAG (be it complex/real valued, 32/64 bit, threaded or not, etc) and everything is RPATH'ed.

davydden · 2016-11-30T06:38:36Z

@alalazo : I added mkl_rt here, on a second thought i would say you don't need to do anything in #1875 with it.

davydden · 2016-11-30T06:40:03Z

@glennpj @tgamblin PR updated with mkl_rt.

nrichart · 2016-12-01T10:47:07Z

var/spack/repos/builtin/packages/py-numpy/package.py

+                        '^atlas' in spec):
+                    f.write('libraries=%s\n'    % names)
+                    f.write('library_dirs=%s\n' % dirs)
+
                if not ((platform.system() == "Darwin") and
                        (platform.mac_ver()[0] == '10.12')):
                    f.write('rpath=%s\n' % ':'.join(lapackblas.directories))


His the rpath properly taken into account if it is in the section [all/default] when you use openblas, mkl or atlas.
Or should it be placed in all the sections separately ? It was added for #719 so should be easy to check.

If it works fine in the [all/default] section you can also factorize the library_dirs that is the same for all of them.

library_dirs do NOT work in ALL (although it's advertised it should), i checked that. I will check if rpath needs to be in each section separately...

@nrichart after looking at #719 AFAICT it works fine because I actually do activate numpy and then do import and run a simple test (see OP). In other words, it works as expected.

davydden · 2016-12-07T14:47:45Z

@tgamblin @becker33 ping.

davydden · 2016-12-20T13:12:59Z

@tgamblin @becker33 ping-ping.

davydden · 2017-01-20T08:34:08Z

@tgamblin @becker33 ping-ping-ping.

tgamblin · 2017-01-23T21:57:14Z

@davydden: Sorry for the delay -- can you rebase?

including a workaround for MKL due to limited config system of numpy

davydden · 2017-01-24T09:55:45Z

@tgamblin i rebased, hopefully the tester will be happy.

davydden · 2017-01-26T07:35:28Z

@tgamblin the tester is happy with py-numpy, this can be merged unless you have further comments

* py-numpy: fix blas/lapack section title * py-numpy: rework BLAS/LAPACK including a workaround for MKL due to limited config system of numpy

davydden added the WIP label Nov 18, 2016

davydden force-pushed the pkg/py_numpy_default_all branch 2 times, most recently from 7d215e7 to 290deb4 Compare November 20, 2016 17:49

davydden changed the title ~~py-numpy: fix blas/lapack section title~~ py-numpy: rework blas/lapack Nov 20, 2016

davydden added blas-lapack-scalapack new-package labels Nov 20, 2016

davydden force-pushed the pkg/py_numpy_default_all branch from 290deb4 to 86444bc Compare November 20, 2016 18:21

davydden added ready and removed WIP labels Nov 21, 2016

davydden mentioned this pull request Nov 24, 2016

atlas: multithreaded BLAS is used despite setting site.cfg to use serial one numpy/numpy#8294

Open

nrichart reviewed Dec 1, 2016

View reviewed changes

davydden mentioned this pull request Dec 23, 2016

add package for conduit #2670

Merged

py-numpy: fix blas/lapack section title

cd2c09a

davydden force-pushed the pkg/py_numpy_default_all branch from a382ac1 to 2d412e0 Compare January 24, 2017 09:53

py-numpy: rework BLAS/LAPACK

5845905

including a workaround for MKL due to limited config system of numpy

davydden force-pushed the pkg/py_numpy_default_all branch from 2d412e0 to 5845905 Compare January 24, 2017 09:55

tgamblin merged commit d324d13 into spack:develop Jan 26, 2017

davydden mentioned this pull request Feb 2, 2017

building scipy results in openblas not being found #3007

Closed

This was referenced Feb 21, 2017

Unable to build Numpy with Intel 17 #3204

Closed

Importing the multiarray numpy extension module failed numpy/numpy#8653

Closed

[WIP] Fix build of Numpy with Intel #3207

Closed

diaena pushed a commit to diaena/spack that referenced this pull request May 26, 2017

py-numpy: rework blas/lapack (spack#2365)

9ee5e33

* py-numpy: fix blas/lapack section title * py-numpy: rework BLAS/LAPACK including a workaround for MKL due to limited config system of numpy

davydden deleted the pkg/py_numpy_default_all branch May 28, 2017 20:55

amklinv pushed a commit that referenced this pull request Jul 17, 2017

py-numpy: rework blas/lapack (#2365)

a3b7cd8

* py-numpy: fix blas/lapack section title * py-numpy: rework BLAS/LAPACK including a workaround for MKL due to limited config system of numpy

healther pushed a commit to electronicvisions/spack that referenced this pull request Jul 26, 2017

py-numpy: rework blas/lapack (spack#2365)

ae59870

* py-numpy: fix blas/lapack section title * py-numpy: rework BLAS/LAPACK including a workaround for MKL due to limited config system of numpy

davydden removed the new-package label Sep 4, 2017

davydden mentioned this pull request Sep 19, 2017

py-numpy linking to intel mkl while compiling with gcc and openblas? #5327

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

py-numpy: rework blas/lapack #2365

py-numpy: rework blas/lapack #2365

davydden commented Nov 18, 2016 •

edited

Loading

davydden commented Nov 21, 2016

davydden commented Nov 21, 2016

glennpj commented Nov 29, 2016

glennpj commented Nov 29, 2016

davydden commented Nov 29, 2016

davydden commented Nov 29, 2016

glennpj commented Nov 29, 2016

glennpj commented Nov 29, 2016

tgamblin commented Nov 29, 2016

tgamblin commented Nov 29, 2016

alalazo commented Nov 29, 2016

tgamblin commented Nov 29, 2016 •

edited

Loading

davydden commented Nov 29, 2016

alalazo commented Nov 29, 2016 •

edited

Loading

davydden commented Nov 29, 2016

davydden commented Nov 30, 2016

davydden commented Nov 30, 2016

nrichart Dec 1, 2016

davydden Dec 1, 2016

davydden Dec 2, 2016

davydden commented Dec 7, 2016

davydden commented Dec 20, 2016

davydden commented Jan 20, 2017

tgamblin commented Jan 23, 2017 •

edited

Loading

davydden commented Jan 24, 2017

davydden commented Jan 26, 2017

py-numpy: rework blas/lapack #2365

py-numpy: rework blas/lapack #2365

Conversation

davydden commented Nov 18, 2016 • edited Loading

davydden commented Nov 21, 2016

davydden commented Nov 21, 2016

glennpj commented Nov 29, 2016

glennpj commented Nov 29, 2016

davydden commented Nov 29, 2016

davydden commented Nov 29, 2016

glennpj commented Nov 29, 2016

glennpj commented Nov 29, 2016

tgamblin commented Nov 29, 2016

tgamblin commented Nov 29, 2016

alalazo commented Nov 29, 2016

tgamblin commented Nov 29, 2016 • edited Loading

davydden commented Nov 29, 2016

alalazo commented Nov 29, 2016 • edited Loading

davydden commented Nov 29, 2016

davydden commented Nov 30, 2016

davydden commented Nov 30, 2016

nrichart Dec 1, 2016

Choose a reason for hiding this comment

davydden Dec 1, 2016

Choose a reason for hiding this comment

davydden Dec 2, 2016

Choose a reason for hiding this comment

davydden commented Dec 7, 2016

davydden commented Dec 20, 2016

davydden commented Jan 20, 2017

tgamblin commented Jan 23, 2017 • edited Loading

davydden commented Jan 24, 2017

davydden commented Jan 26, 2017

davydden commented Nov 18, 2016 •

edited

Loading

tgamblin commented Nov 29, 2016 •

edited

Loading

alalazo commented Nov 29, 2016 •

edited

Loading

tgamblin commented Jan 23, 2017 •

edited

Loading