Opened 8 years ago

Last modified 8 years ago

#14699 closed defect

Install ATLAS header files and static libraries — at Version 34

Reported by: jhpalmieri Owned by: jdemeyer
Priority: blocker Milestone: sage-5.10
Component: packages: standard Keywords: atlas IML testsuite check cblas.h
Cc: leif, jpflori, jdemeyer, vbraun Merged in:
Authors: Jean-Pierre Flori Reviewers:
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Change History (34)

comment:1 Changed 8 years ago by jpflori

Surely an ATLAS problem, see http://trac.sagemath.org/sage_trac/ticket/10508#comment:526 for a solution.

I suggest we call at least install_inc and potentially install_lib from ATLAS build system to install headers and static libs.

comment:2 Changed 8 years ago by jpflori

Or just install which should call both of them.

comment:3 follow-up: Changed 8 years ago by leif

John, U sure about sage.math?

128.208.160.191	sage.math.washington.edu sage sage.math

comment:4 in reply to: ↑ 3 Changed 8 years ago by leif

Replying to leif:

John, U sure about sage.math?

128.208.160.191	sage.math.washington.edu sage sage.math

(As opposed to

;; ANSWER SECTION:
sage.math.washington.edu. 600	IN	A	128.208.160.197

which is boxen.)

comment:5 Changed 8 years ago by jpflori

It would be nice to fix that on top of #14605 which I've tested quite extensively and is finished since some time.

comment:6 Changed 8 years ago by leif

+1 to also again install static libs.

comment:7 Changed 8 years ago by jpflori

  • Cc jeroen vbraun added
  • Keywords atlas added
  • Priority changed from major to critical
  • Summary changed from Get iml to install correctly and pass its self-tests on sage.math to Install ATLAS headers file and static libraries

In fact disregard my comment about #14605. We should fix the issue here quickly, hopefully before 5.10 official release if it's not too late. Although as it does not seem to prevent anything from working except for IML testsuite, 5.11 should be ok as well.

comment:8 Changed 8 years ago by leif

  • Cc jdemeyer added; jeroen removed
  • Keywords IML testsuite check cblas.h added
  • Summary changed from Install ATLAS headers file and static libraries to Install ATLAS header files and static libraries

comment:9 Changed 8 years ago by Koen

A side-effect of this ticket is probably that numpy gets built without ATLAS acceleration.

comment:10 Changed 8 years ago by leif

For the record, I did get

ATLAS_CFLAGS=''
ATLAS_LIBS='-lcblas -latlas'

-L${SAGE_LOCAL}/lib "incidentally" came from / for GMP, although after -lcblas -latlas, but we meanwhile add ${SAGE_LOCAL}/lib to LIBRARY_PATH (in sage-env), so that it worked.

Last edited 8 years ago by leif (previous) (diff)

comment:11 follow-up: Changed 8 years ago by leif

P.S.: Although IIRC according to POSIX, -Ldir1 -lfoo -Ldir2 -lbar is the same as -Ldir1 -Ldir2 -lfoo -lbar.

comment:12 Changed 8 years ago by jpflori

  • Authors set to Jean-Pierre Flori
  • Description modified (diff)
  • Status changed from new to needs_review

comment:13 in reply to: ↑ 11 Changed 8 years ago by leif

Replying to leif:

P.S.: Although IIRC according to POSIX, -Ldir1 -lfoo -Ldir2 -lbar is the same as -Ldir1 -Ldir2 -lfoo -lbar.

At least GNU ld docs agree with this:

All -L options apply to all -l options, regardless of the order in which the options appear.

comment:14 follow-ups: Changed 8 years ago by Koen

You'd still need a line

    cmd += ' --incdir=' + os.path.join(conf['SAGE_LOCAL'],'include') 

in configure_atlas_library() for the header files to go the correct place?

And currently numpy is built without Atlas (it uses another blas): local/lib/python2.7/site-packages/numpy/core/_dotblas.so normally contains ATL_* symbols (for an old Atlas 3.8.3 version, at least).

comment:15 in reply to: ↑ 14 Changed 8 years ago by jpflori

Replying to Koen:

You'd still need a line

    cmd += ' --incdir=' + os.path.join(conf['SAGE_LOCAL'],'include') 

in configure_atlas_library() for the header files to go the correct place?

I don't think so. We use prefix and that should be enough from what I've seen in ATLAS configure script.

comment:16 in reply to: ↑ 14 Changed 8 years ago by jpflori

Replying to Koen:

And currently numpy is built without Atlas (it uses another blas): local/lib/python2.7/site-packages/numpy/core/_dotblas.so normally contains ATL_* symbols (for an old Atlas 3.8.3 version, at least).

On my gcc110 install (where IML testsuite fails), numpy uses ATLAS correctly. There is no ATLAS symbols in the file you pointed because we now use a shared version of ATLAS. But if I run ldd on the file it points to the ATLAS libs.

And I seem to remember we had lots of troubles correctly biulding numpy and scipy with the new ATLAS so hopefully it was actually used.

comment:17 follow-up: Changed 8 years ago by leif

s/inlcude/include/

and IMHO (=WTF?) also s/serial/single-threaded/g s/parallel/multi-threaded/g ...

(but the latter is not JP's invention I guess)

comment:18 in reply to: ↑ 17 Changed 8 years ago by jpflori

Replying to leif:

s/inlcude/include/

Done.

and IMHO (=WTF?) also s/serial/single-threaded/g s/parallel/multi-threaded/g ...

(but the latter is not JP's invention I guess)

Not mine indeed.

comment:19 Changed 8 years ago by leif

Replying to jpflori:

Replying to Koen:

And currently numpy is built without Atlas (it uses another blas): local/lib/python2.7/site-packages/numpy/core/_dotblas.so normally contains ATL_* symbols (for an old Atlas 3.8.3 version, at least).

On my gcc110 install (where IML testsuite fails), numpy uses ATLAS correctly. There is no ATLAS symbols in the file you pointed because we now use a shared version of ATLAS. But if I run ldd on the file it points to the ATLAS libs.

Yes.

$ readelf -d local/lib/python2.7/site-packages/numpy/core/_dotblas.so

Dynamic section at offset 0x4dc0 contains 26 entries:
  Tag        Type                         Name/Value
 0x0000000000000001 (NEEDED)             Shared library: [libptf77blas.so.2]
 0x0000000000000001 (NEEDED)             Shared library: [libptcblas.so.2]
 0x0000000000000001 (NEEDED)             Shared library: [libatlas.so.2]
 0x0000000000000001 (NEEDED)             Shared library: [libpython2.7.so.1.0]
 0x0000000000000001 (NEEDED)             Shared library: [libpthread.so.0]
 0x0000000000000001 (NEEDED)             Shared library: [libc.so.6]
 ...

comment:20 Changed 8 years ago by jpflori

  • Status changed from needs_review to needs_work

make_atlas() takes no argument.

comment:21 Changed 8 years ago by jpflori

  • Status changed from needs_work to needs_review

comment:22 Changed 8 years ago by jhpalmieri

I saw the error on boxen. I thought that sage.math was dead? In any case, it's the machine I get to by typing ssh sage.math.washington.edu, and when I log off it says Connection to sage.math.washington.edu closed. The $HOSTNAME is boxen, though.

comment:23 Changed 8 years ago by jhpalmieri

On boxen, after installing the new ATLAS spkg, the current iml spkg installs and passes its tests. Same for an iml spkg which doesn't copy over the patched version of configure.

comment:24 Changed 8 years ago by leif

P.S.: I guess the odd "patch" to IML's configure dates back to the times when ATLAS was somehow optional, or at least did not get properly detected on all systems. Running / building IML's test suite on such systems was always broken then.

(In other parts of Sage using IML, BLAS libraries get explicitly added by us when linking.)

comment:25 Changed 8 years ago by jdemeyer

  • Priority changed from critical to blocker

comment:26 follow-up: Changed 8 years ago by vbraun

Do not install static libraries, its a feature that we don't.

Also, nobody in his right mind expects blas headers. This is why the iml testsuite, which the authors probably didn't expect to be distributed, is the only place that trips over it. For starters, there is no universal naming standard (blas.h vs. cblas.h vs. foo_cblas.h vs. foo/cblas.h). There are multiple blas implementations and if you all install them at the same place then they would overwrite each other.

comment:27 Changed 8 years ago by vbraun

And if you need a reason for not installing static libraries you just have to look at this ticket where you got confused about whether something links against atlas or not. Shared libraries are the only way that make it clear which particular atlas version you are linking against and that you didn't accidentally pick up some system atlas.

comment:28 Changed 8 years ago by Koen

I disagree that nobody expects the blas headers. If I try to install a package on top of Sage, e.g. the Shogun machine learning toolbox, then it will only compile in certain features if it can find a blas - and for that it needs blas headers. With older sage versions this works just fine.

comment:29 Changed 8 years ago by vbraun

Shogun just makes it unnecessarily complicated for themselves, you need to define preprocessor constants to tell it which blas header to look for...

I agree that we should have blas/cblas/f77blas headers since its the right thing to do. But when we fix the IML testsuite we shouldn't introduce that as a dependency, because very often the headers are either not installed or installed with a funky name.

comment:30 Changed 8 years ago by jdemeyer

  • Milestone changed from sage-5.11 to sage-5.10

comment:31 in reply to: ↑ 26 ; follow-up: Changed 8 years ago by leif

Replying to vbraun:

Do not install static libraries, its a feature that we don't.

At least they shouldn't get discarded, which is simply stupid.

comment:32 Changed 8 years ago by leif

Btw., the new spkg and afterwards IML installed without problems, and both passed their test suites on Linux x86_64 (Ubuntu 10.04.4, Sage 5.10.rc0).

comment:33 in reply to: ↑ 31 Changed 8 years ago by vbraun

Replying to leif:

At least they shouldn't get discarded, which is simply stupid.

Do you also object to discarding the *.o files? We delete what we don't want around in the install, thats the way it works. Ideally upstream would have an --disable-static switch, but they don't.

comment:34 Changed 8 years ago by vbraun

  • Description modified (diff)

Removed the static library install bit from the spkg

Note: See TracTickets for help on using tickets.