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

upgrade cvxopt to 1.1.5 #13160

Closed
kiwifb opened this issue Jun 25, 2012 · 39 comments
Closed

upgrade cvxopt to 1.1.5 #13160

kiwifb opened this issue Jun 25, 2012 · 39 comments

Comments

@kiwifb
Copy link
Member

kiwifb commented Jun 25, 2012

Upgrading cvxopt to 1.1.5 we can get rid of a patch to correct a wrong behavior that is obvious on OS X. Also I want to simplify the setup.py patch which give rise to overlinking (and in fact potential linking of multiple cblas libraries - yuk!). I am also attempting to install the documentation.

Updated spkg at:

Component: packages: standard

Author: François Bissey

Reviewer: Volker Braun

Merged: sage-5.3.beta1

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

@kiwifb kiwifb added this to the sage-5.3 milestone Jun 25, 2012
@kiwifb
Copy link
Member Author

kiwifb commented Jun 25, 2012

comment:1

I have a spkg almost ready to upload. Changes are uncommitted yet. There are some kinks to check in the installation of the documentation.

@kiwifb
Copy link
Member Author

kiwifb commented Jun 26, 2012

comment:2

Experimental spkg now posted and linked in the description. The changes in the spkg have not been committed yet so I can still fiddle with it as necessary.

@kiwifb

This comment has been minimized.

@dimpase
Copy link
Member

dimpase commented Jul 20, 2012

comment:3

Looks good on OSX 10.6.8 and Sage 5.2.beta1 with #10508 applied.
I tried both lapack/blas, the ones from Apple, and the ones from Atlas 3.10 from #10508, with SAGE_CHECK on.

By the way, shouldn't #10508 be a dependence of this ticket? (I added it here, tentatively.)

Also, I wonder if there is anything to report upstream here.

@dimpase
Copy link
Member

dimpase commented Jul 20, 2012

Dependencies: #10508

@vbraun
Copy link
Member

vbraun commented Jul 21, 2012

comment:4

#10508 doesn't change the layout of the ATLAS libraries, the current atlas-3.8.4 already provides the cblas library. If the cvxopt spkg were to link to the correct libraries only, it would work with both atlas-3.8.4 and atlas-3.10.0 spkgs.

@vbraun
Copy link
Member

vbraun commented Jul 21, 2012

Changed dependencies from #10508 to none

@dimpase
Copy link
Member

dimpase commented Jul 22, 2012

comment:5

Replying to @vbraun:

#10508 doesn't change the layout of the ATLAS libraries, the current atlas-3.8.4 already provides the cblas library. If the cvxopt spkg were to link to the correct libraries only, it would work with both atlas-3.8.4 and atlas-3.10.0 spkgs.

When you closed #10509 you said it is fixed by atlas-3.10.0 spkg. And #10508 is the one the provides atlas-3.10.0 spkg.

@kiwifb
Copy link
Member Author

kiwifb commented Jul 22, 2012

comment:6

The only thing really to check in this ticket is the installation of the documentation. Technically it needs sphinx to build but because up to date build documentation is shipped nothing is done. So the question is whether to require sphinx or just copy the already built documentation. Once I make an informed decision on that I'll finalize the spkg.

@kiwifb
Copy link
Member Author

kiwifb commented Jul 22, 2012

comment:7

OK I pushed a new spkg on google code it install the already built documentation and has a section commented out on how to build it in spkg-install.

@dimpase
Copy link
Member

dimpase commented Jul 23, 2012

comment:9

Replying to @kiwifb:

OK I pushed a new spkg on google code it install the already built documentation and has a section commented out on how to build it in spkg-install.

Shouldn't it install docs by default? (Currently, you need to set SAGE_SPKG_INSTALL_DOCS)

Also, I don't understand what you mean by "dependence" on sphinx. Isn't sphinx included in Sage?

@kiwifb
Copy link
Member Author

kiwifb commented Jul 23, 2012

comment:10

Replying to @dimpase:

Replying to @kiwifb:

OK I pushed a new spkg on google code it install the already built documentation and has a section commented out on how to build it in spkg-install.

Shouldn't it install docs by default? (Currently, you need to set SAGE_SPKG_INSTALL_DOCS)

Also, I don't understand what you mean by "dependence" on sphinx. Isn't sphinx included in Sage?

As far as I know, you only install documentation for spkg if that variable is set. I may be wrong.

Yes sphinx is shipped with sage but if this spkg uses sphinx it has to be listed as one of its dependencies in spkg/standard/deps. In this particular case it is probably ok because cvxopt can be build after sage I think. But in the alternative there are problems as far as I remember from a previous upgrade of numpy way back.

@dimpase
Copy link
Member

dimpase commented Jul 23, 2012

comment:11

Replying to @kiwifb:

OK, I asked on sage-devel

@kcrisman
Copy link
Member

comment:12

Replying to @dimpase:

Replying to @kiwifb:

OK I pushed a new spkg on google code it install the already built documentation and has a section commented out on how to build it in spkg-install.

Shouldn't it install docs by default? (Currently, you need to set SAGE_SPKG_INSTALL_DOCS)

?? #11197 has sort of delayed most packages actually implementing #10823. I believe that we currently don't want docs installed by default outside of wherever the spkg lives. Or maybe I'm misunderstanding the point of this discussion, my apologies if that's so.

@kiwifb
Copy link
Member Author

kiwifb commented Jul 23, 2012

comment:13

I didn't remember how the discussion had ended. Probably because I wasn't on the follow up ticket where things were discussed. Should I remove the section about documentation until one day we sort this out?

We should note that some packages already put their doc in $SAGE_LOCAL/share/doc:

frb15@p2n14-c /hpc/scratch/frb15/sandbox/sage-5.1.beta5/local/share/doc :ll   
total 1792
drwxr-xr-x 2 frb15 z001 131072 Jun 29 13:08 NTL
drwxr-xr-x 5 frb15 z001 131072 Jun 29 15:05 ipython
drwxr-xr-x 3 frb15 z001 131072 Jun 29 13:13 mpfr
drwxr-xr-x 3 frb15 z001 131072 Jun 29 15:12 networkx-1.2
drwxr-xr-x 3 frb15 z001 131072 Jun 29 13:53 ppl
drwxr-xr-x 3 frb15 z001 131072 Jun 29 13:53 pwl
drwxr-xr-x 2 frb15 z001 131072 Jun 29 15:49 sagetex

I am not even sure those obey SAGE_SPKG_INSTALL_DOCS and some are html. The cvxopt spkg linked in this ticket doesn't need sphinx because the doc is prebuilt in the tarball.

@dimpase
Copy link
Member

dimpase commented Jul 24, 2012

comment:14

Replying to @kiwifb:

I didn't remember how the discussion had ended. Probably because I wasn't on the follow up ticket where things were discussed. Should I remove the section about documentation until one day we sort this out?

no, why, documentation is great to have. But why not remove the pre-built one and make it using sphynx?
(the latter will need a trivial change in SAGE_ROOT/spkg/standard/deps).

@kiwifb
Copy link
Member Author

kiwifb commented Jul 24, 2012

comment:15

Can do that but I don't think it will be much different from the one shipped. The first thing that sphinx does when it is run is check whether what is already built is up to date. Rebuilding it again is just a waste of cpu cycles.

I don't really care about it that much myself. So if people are quite happy about rebuilding the doc I'll just remove the pre build from the spkg to save space and produce the other needed patch.

@dimpase
Copy link
Member

dimpase commented Jul 27, 2012

comment:16

There are leftovers of debugging in src/src/C/blas.c, starting from line 295, which reads

    printf("scal\n");

This is very strange.

EDIT: I checked the upstream, and they don't have these printfs...

@dimpase
Copy link
Member

dimpase commented Jul 27, 2012

comment:17

Replying to @dimpase:

There are leftovers of debugging in src/src/C/blas.c, starting from line 295, which reads

    printf("scal\n");

This is very strange.

EDIT: I checked the upstream, and they don't have these printfs...

Moreover, the upstream files base.c, blas.c, and sparse.c in src/src/C have earlier dates (and different sizes) than the ones in spkg.
And replacing the spkg files by upstream files makes the package seemingly work on OSX10.6.8. I didn't test much, but the file src/examples/doc/chap10/l1svc.py which gives me crashes with the spkg, works OK with the upstream replacement.

@kiwifb
Copy link
Member Author

kiwifb commented Jul 29, 2012

comment:18

Sorry wrong spkg I was tracking something on power7 I will correct the source today - hopefully.

@kiwifb
Copy link
Member Author

kiwifb commented Jul 30, 2012

comment:19

Corrected the source. Sorry for the inconvenience.

@vbraun

This comment has been minimized.

@vbraun
Copy link
Member

vbraun commented Aug 2, 2012

comment:20

On Fedora 17 x86_64:

building 'gsl' extension
creating build/temp.linux-x86_64-2.7
creating build/temp.linux-x86_64-2.7/C
gcc -fno-strict-aliasing -fwrapv -DNDEBUG -g -fwrapv -O3 -Wall -Wstrict-prototypes -fPIC -I/home/vbraun/opt/sage-atlas/sage-5.2/local/include/gsl -I/home/vbraun/opt/sage-atlas/sage-5.2/local/include/python2.7 -c C/gsl.c -o build/temp.linux-x86_64-2.7/C/gsl.o
C/gsl.c:28:25: fatal error: gsl/gsl_rng.h: No such file or directory
compilation terminated.

The header is in Sage but the include path is wrong:

[vbraun@laptop sage-5.2]$ find | grep gsl_rng
./local/include/gsl/gsl_rng.h

The fix is to set If I edit GSL_INC_DIR = SAGE_INCLUDE in setup.py, and not SAGE_INCLUDE/gsl.

I've updated the spkg (see description, only change is the GSL_INC_DIR) and it builds for me now.

@kiwifb
Copy link
Member Author

kiwifb commented Aug 2, 2012

comment:21

OK you beat me to it. I was going to post about it. I am guessing the problem is not visible if gsl is installed system wide.

@vbraun
Copy link
Member

vbraun commented Aug 2, 2012

comment:22

Just saw that we discovered the same bug within ~5mins. Coincidence? ;-)

@kiwifb
Copy link
Member Author

kiwifb commented Aug 2, 2012

comment:23

John was the pointer for me really. Didn't notice it before - all the system I tested it with had gsl as standard. I need a box with nothing interesting on it really.

@jhpalmieri
Copy link
Member

comment:24

On hawk, this builds if SAGE_ATLAS_LIB points to the system ATLAS, but not if I use the ATLAS spkg from #10508. Log here.

@kiwifb
Copy link
Member Author

kiwifb commented Aug 2, 2012

comment:25

shouldn't be -lblas but -lf77blas I guess.

@kiwifb
Copy link
Member Author

kiwifb commented Aug 2, 2012

comment:26
+if os.environ['UNAME'] == 'CYGWIN':
+  BLAS_LIB =['blas', 'gfortran']
+elif os.environ['UNAME'] == 'Darwin':
+  BLAS_LIB =['blas','gfortran','m']
+else:
+  BLAS_LIB =['blas','cblas','gfortran','atlas','m']
+

should really be

+if os.environ['UNAME'] == 'CYGWIN':
+  BLAS_LIB =['blas', 'gfortran']
+elif os.environ['UNAME'] == 'Darwin':
+  BLAS_LIB =['blas','gfortran','m']
+else:
+  BLAS_LIB =['f77blas','cblas','gfortran','atlas','m']
+

I think it may have escaped because we had libblas before #10508

@vbraun
Copy link
Member

vbraun commented Aug 2, 2012

comment:27

I agree and just replaced the spkg (new md5sum = b155f7e4624c2869b9a0e5aa21803435)

Builds on Fedora 17

@jhpalmieri
Copy link
Member

comment:28

Builds and passes self-tests on hawk, both with the system ATLAS and the ATLAS spkg from #10508.

@vbraun
Copy link
Member

vbraun commented Aug 2, 2012

comment:29

Looks good to me!

@vbraun
Copy link
Member

vbraun commented Aug 2, 2012

Reviewer: Volker Braun

@vbraun
Copy link
Member

vbraun commented Aug 2, 2012

Author: François Bissey

@kiwifb
Copy link
Member Author

kiwifb commented Aug 2, 2012

comment:30

Thanks Volker for the last mile. I also just thought that we will need a follow up ticket if we allow people to use ATLAS on OS X.

@jhpalmieri
Copy link
Member

comment:31

So on OS X, if people use the ATLAS spkg, then is this using a mix of Sage's ATLAS and the system's blas? (I'm in the process of building on OS X, including ATLAS from #10508, and cvxopt has built and passes its self-tests. The build hasn't completed, so Sage's test suite hasn't run yet.)

@dimpase
Copy link
Member

dimpase commented Aug 3, 2012

comment:32

Replying to @jhpalmieri:

So on OS X, if people use the ATLAS spkg, then is this using a mix of Sage's ATLAS and the system's blas? (I'm in the process of building on OS X, including ATLAS from #10508, and cvxopt has built and passes its self-tests. The build hasn't completed, so Sage's test suite hasn't run yet.)

I hope so. It would be a big speed digression to use GSL.

@kiwifb
Copy link
Member Author

kiwifb commented Aug 3, 2012

comment:33

From what I put in the spkg cvxopt will use the OS X accelerate framework instead of ATLAS. You should note that it is not the only spkg for which it will be the case. linbox will do that as well for example. iml also comes to mind as well and numpy and scipy should be checked.
From the sage shell can you check the output of

otool -L local/lib/python/site-packages/cvxopt/blas.so

It should tell you against which library it was linked in some form.

@jdemeyer
Copy link
Contributor

Merged: sage-5.3.beta1

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

6 participants