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

Install ATLAS header files and cleanup IML spkg #14699

Closed
jhpalmieri opened this issue Jun 6, 2013 · 87 comments
Closed

Install ATLAS header files and cleanup IML spkg #14699

jhpalmieri opened this issue Jun 6, 2013 · 87 comments

Comments

@jhpalmieri
Copy link
Member

With the new ATLAS spkg from #10508, iml fails its self tests. See [#10508 comment:518] and later comments.

Use spkgs at:

Apply attachment: 14699_env_version_bump.patch to SAGE_ROOT

Depends on #10508

CC: @nexttime @jpflori @jdemeyer @vbraun

Component: packages: standard

Keywords: atlas IML testsuite check cblas.h

Author: Jean-Pierre Flori, Volker Braun, Jeroen Demeyer

Reviewer: Jean-Pierre Flori

Merged: sage-5.10.rc2

Issue created by migration from https://trac.sagemath.org/ticket/14699

@jpflori
Copy link

jpflori commented Jun 6, 2013

comment:1

Surely an ATLAS problem, see #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.

@jpflori
Copy link

jpflori commented Jun 6, 2013

comment:2

Or just install which should call both of them.

@nexttime
Copy link
Mannequin

nexttime mannequin commented Jun 6, 2013

comment:3

John, U sure about sage.math?

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

@nexttime
Copy link
Mannequin

nexttime mannequin commented Jun 6, 2013

comment:4

Replying to @nexttime:

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

@jpflori
Copy link

jpflori commented Jun 6, 2013

comment:5

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

@nexttime
Copy link
Mannequin

nexttime mannequin commented Jun 6, 2013

comment:6

+1 to also again install static libs.

@jpflori
Copy link

jpflori commented Jun 6, 2013

Changed keywords from none to atlas

@jpflori
Copy link

jpflori commented Jun 6, 2013

comment:7

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.

@jpflori jpflori changed the title Get iml to install correctly and pass its self-tests on sage.math Install ATLAS headers file and static libraries Jun 6, 2013
@nexttime
Copy link
Mannequin

nexttime mannequin commented Jun 6, 2013

Changed keywords from atlas to atlas IML testsuite check cblas.h

@nexttime nexttime mannequin changed the title Install ATLAS headers file and static libraries Install ATLAS header files and static libraries Jun 6, 2013
@sagetrac-Koen
Copy link
Mannequin

sagetrac-Koen mannequin commented Jun 6, 2013

comment:9

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

@nexttime
Copy link
Mannequin

nexttime mannequin commented Jun 6, 2013

comment:10

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.

@nexttime
Copy link
Mannequin

nexttime mannequin commented Jun 6, 2013

comment:11

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

@jpflori

This comment has been minimized.

@jpflori
Copy link

jpflori commented Jun 6, 2013

Author: Jean-Pierre Flori

@nexttime
Copy link
Mannequin

nexttime mannequin commented Jun 6, 2013

comment:13

Replying to @nexttime:

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.

@sagetrac-Koen
Copy link
Mannequin

sagetrac-Koen mannequin commented Jun 6, 2013

comment:14

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

@jpflori
Copy link

jpflori commented Jun 6, 2013

comment:15

Replying to @sagetrac-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.

@jpflori
Copy link

jpflori commented Jun 6, 2013

comment:16

Replying to @sagetrac-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.

@nexttime
Copy link
Mannequin

nexttime mannequin commented Jun 6, 2013

comment:17

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)

@jpflori
Copy link

jpflori commented Jun 6, 2013

comment:18

Replying to @nexttime:

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.

@jpflori
Copy link

jpflori commented Jun 11, 2013

comment:69

In the IMl spkg, you don't reexport CFLAGS after checking for SAGE64 (both in spkg-check and spkg-install).

And when looking through trac there is a strange .patch part about renaming sage3_memleak, but that may just be a trac issue.

@jdemeyer
Copy link

comment:70

Replying to @jpflori:

In the IMl spkg, you don't reexport CFLAGS after checking for SAGE64 (both in spkg-check and spkg-install).

It's already exported before. No need to "reexport" it.

And when looking through trac there is a strange .patch part about renaming sage3_memleak, but that may just be a trac issue.

Indeed, that's a Trac bug.

@jdemeyer
Copy link

Changed work issues from SAGE64, to none

@jdemeyer
Copy link

comment:71

I should also note that the only new patch in IML is patches/remove_repl.patch, all the others were already (pre-patched) in the spkg.

@jpflori
Copy link

jpflori commented Jun 12, 2013

comment:72

Replying to @jdemeyer:

Replying to @jpflori:

In the IMl spkg, you don't reexport CFLAGS after checking for SAGE64 (both in spkg-check and spkg-install).

It's already exported before. No need to "reexport" it.

Damn, I was not aware of that!

And when looking through trac there is a strange .patch part about renaming sage3_memleak, but that may just be a trac issue.

Indeed, that's a Trac bug.

@jpflori
Copy link

jpflori commented Jun 14, 2013

comment:73

The spkg looks good, seems to work on my systems.

@jdemeyer
Copy link

Merged: sage-5.10.rc2

@jdemeyer
Copy link

comment:75

Thanks for the review, do you by chance feel like reviewing the IML upgrade at #748?

@jdemeyer
Copy link

Changed merged from sage-5.10.rc2 to none

@jdemeyer
Copy link

comment:76

Reverting the ATLAS package: #14753.

@jdemeyer jdemeyer reopened this Jun 17, 2013
@jdemeyer jdemeyer modified the milestones: sage-5.10, sage-5.11 Jun 17, 2013
@jdemeyer
Copy link

Dependencies: #10508

@jdemeyer
Copy link

Merged: sage-5.10.rc2

@jdemeyer jdemeyer modified the milestones: sage-5.11, sage-5.10 Jun 17, 2013
@jdemeyer
Copy link

comment:79

Keeping "merged in" information for future reference, we should not change this ticket further.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants