Ticket #6456 (needs_work defect)

Opened 14 months ago

Last modified 5 weeks ago

Upgrade cvxopt in sage from 0.9 to 1.1.2

Reported by: was Owned by: mabshoff
Priority: major Milestone: sage-4.5.3
Component: packages Keywords:
Cc: Author(s): schilly, dimpase
Report Upstream: Completely fixed; Fix reported upstream Reviewer(s):
Merged in: Work issues:

Description

We are shipping an *ancient* version of cvxopt in sage. It's worse than Debian shipping sage-3.0.5!

Note that upgrading cvxopt should be fairly easy, since basically nothing in Sage depends on cvxopt.

Attachments

trac-6456.patch Download (344 bytes) - added by dimpase 7 months ago.
patch for cvxopt-1.1.2.p1/src/src/setup.py
6456-numerical_sage_cvxopt.patch Download (2.0 KB) - added by schilly 5 months ago.
trival changes to the cvxopt chapter in the numerical sage tutorial
6456-error-check-while-installing.patch Download (0.6 KB) - added by drkirkby 7 weeks ago.
Check for errors while installing cvxopt. Note there is still no spkg-check file. That may or may not be useful - it depends on the source code whether it supports tests.
6456-freebsd-spkg-install.patch Download (422 bytes) - added by pjeremy 5 weeks ago.
cvxopt-1.1.2-Linux-tests.log Download (17.8 KB) - added by drkirkby 5 weeks ago.
Results from running cvxopt's self tests on a Linux system (sage.math), but setting SAGE_CHECK=yes.
cvxopt-1.1.2-SAGE_CHECK.log Download (20.8 KB) - added by schilly 5 weeks ago.
ubuntu 10.4, pentium 4

Change History

  Changed 14 months ago by wdj

Fly in the ointment: The recent version has this statement on its license page  http://abel.ee.ucla.edu/cvxopt/copyright.html:

CVXOPT is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software Foundation; either version 3 of the License, or (at your option) any later version.

Older versions apparently did not have that.

  Changed 7 months ago by schilly

  • upstream set to N/A
  • summary changed from Upgrade cvxopt in sage from 0.9 to 1.1.1 to Upgrade cvxopt in sage from 0.9 to 1.1.2

I've recently upgraded it for me to 1.1.2. The problem is, that the sources from them did not compile on my ubuntu 9.10 machine. So I went to the  ubuntu packges db for lucid and grabbed their version. I don't know what they did in their patch, but I guess it's non trivial...

My spkg is  here.

The only remaining modification I had to made to run the examples from the cvxopt website in %python mode was to replace

from random import ...

to

from sage.misc.prandom import ...

in ./src/src/python/__init__.py at several places.

Q:

  1. what's the usual/best mechanism to avoid using Sage's random and switch back to python's random?!
  2. i have no idea what the solaris patches did in the older version, neither do i know how to get it building on another system :(

follow-ups: ↓ 5 ↓ 6   Changed 7 months ago by schilly

I've created an updated p1 spkg.

Using the 1.1.2 sources directly, I get this error site-packages/cvxopt/base.so: undefined symbol: _g95_stop_blank ... I also fiddled around with the setup.py file.

 cvxopt 1.1.2 p1 spkg is here

  Changed 7 months ago by mvngu

Ticket #1620 is a duplicate of this one.

in reply to: ↑ 3   Changed 7 months ago by dimpase

  • status changed from new to needs_work

Replying to schilly:

I've created an updated p1 spkg. Using the 1.1.2 sources directly, I get this error site-packages/cvxopt/base.so: undefined symbol: _g95_stop_blank ... I also fiddled around with the setup.py file.  cvxopt 1.1.2 p1 spkg is here

sage -t "devel/sage-work/sage/numerical/optimize.py"

bumps out essentially due to:

sage: from cvxopt import cholmod
---------------------------------------------------------------------------
ImportError                               Traceback (most recent call last)

/home/dima/sage/devel/sage-work/sage/<ipython console> in <module>()

ImportError: /home/dima/sage/local/lib/python2.6/site-packages/cvxopt/cholmod.so: undefined symbol: _g95_filename
sage: 

Can you reproduce this on a stand-alone build of cvxopt?

Changed 7 months ago by dimpase

patch for cvxopt-1.1.2.p1/src/src/setup.py

in reply to: ↑ 3 ; follow-up: ↓ 7   Changed 7 months ago by dimpase

  • status changed from needs_work to needs_review

Replying to schilly:

I've created an updated p1 spkg. Using the 1.1.2 sources directly, I get this error site-packages/cvxopt/base.so: undefined symbol: _g95_stop_blank ... I also fiddled around with the setup.py file.  cvxopt 1.1.2 p1 spkg is here

please check the patch I just uploaded. It fixes this problem; you just had to link against more dynamic libs...

in reply to: ↑ 6 ; follow-ups: ↓ 8 ↓ 9   Changed 7 months ago by schilly

Replying to dimpase:

please check the patch I just uploaded. It fixes this problem; you just had to link against more dynamic libs...

Thanks, I knew that it is something with that! Works now!!! ;)

I've uploaded  1.1.1.p2 here

Next I'll try if using the debian/ubuntu version of it was really necessary.

For everyone who wants to try this, don't forget that you have to disable the preparser in Sage via preparser(False) ... otherwise there are unknown types when you try to create a matrix with cvxopt's matrix command.

in reply to: ↑ 7 ; follow-up: ↓ 18   Changed 7 months ago by dimpase

  • status changed from needs_review to needs_work

Replying to schilly:

I've uploaded  1.1.1.p2 here Next I'll try if using the debian/ubuntu version of it was really necessary.

please take out f77blas all over in setup.py, for this is apparently obsolete and not needed - and also nukes the installation on Mac OS X (ppc) -- (otherwise it works on the latter platform)

in reply to: ↑ 7   Changed 7 months ago by dimpase

Replying to schilly:

I think that really what remains to be done is to remove dependencies on obsolete fortrans (f77), see my other comment on this. Let's get it done!

Replying to dimpase:

please check the patch I just uploaded. It fixes this problem; you just had to link against more dynamic libs...

Thanks, I knew that it is something with that! Works now!!! ;) I've uploaded  1.1.1.p2 here Next I'll try if using the debian/ubuntu version of it was really necessary.

I don't see a necessity to try non-debian/ubunty version. If you look at the debian patches, you see that all they changed is the source were slight tweaks in setup.py

For everyone who wants to try this, don't forget that you have to disable the preparser in Sage via preparser(False) ... otherwise there are unknown types when you try to create a matrix with cvxopt's matrix command.

Does this mean that we should think of importing cvxopt's matrix into Sage under some other name? I don't know what the usual Sage's way to deal with such things, i.e. name clashes between packages, is.

follow-up: ↓ 11   Changed 7 months ago by drkirkby

  • Have the license issue been resolved?
  • Has this been tested on Solaris?

in reply to: ↑ 10   Changed 7 months ago by dimpase

Replying to drkirkby:

* Have the license issue been resolved?

it is GPL v3 or later. Does it matter? I suppose I can ask the authors to tweak it, if it is really necessary.

* Has this been tested on Solaris?

no, but I can try on one of Skynet's machines (perhaps you can tell me which one is most likely to work :)), or you can try it yourself. (I don't have a ready Solaris install anywhere).

follow-ups: ↓ 13 ↓ 16   Changed 7 months ago by drkirkby

You better check the license issue with William. Code should be GPL 2 or GPL2+, but there are exceptions if a package is optional and some other conditions - I've never fully understood under what conditions code can be GLP 3. But you might find you can only use the latest version which is GPL 2, and not a GPL 3 version.

sage: license()

says all code except jsmath is GPL2, and apparently jsmath is ok, as Sage does not link to it.

The code will not build on skynet, as there are no SPARC machines there. It should build it on 't2' easily though.

I suggest you download sage 4.3.0.1 from one of the mirrors

http://www.sagemath.org/download-solaris.html

Use the following settings.

kirkby@t2:[~] $ echo $PATH
/usr/local/gcc-4.4.1-sun-linker/bin:/usr/local/bin2:/usr/bin:/usr/ccs/bin:/usr/local/bin:/usr/sfw/bin:/bin:/usr/sbin
kirkby@t2:[~] $ echo $LD_LIBRARY_PATH
/usr/local/gcc-4.4.1-sun-linker/lib:=/usr/local/gcc-4.4.1-sun-linker/lib/sparcv9:/usr/local/lib

type make, and build Sage, then try your package.

There is also a binary of Sage on the mirrors. You could download that. I'm not precisely sure what you then need to do to build just your package using the binary as a starting point.

The latest Sage source will not build on Solaris, but 4.3.0.1 will.

Dave

in reply to: ↑ 12 ; follow-up: ↓ 14   Changed 7 months ago by dimpase

Replying to drkirkby:

You better check the license issue with William. Code should be GPL 2 or GPL2+, but there are exceptions if a package is optional and some other conditions - I've never fully understood under what conditions code can be GLP 3. But you might find you can only use the latest version which is GPL 2, and not a GPL 3 version.

Well, cvxopt is an optional package, so it must be in the same boat as jmath, or some gap packages, that are also not GPL 2.

{{{ sage: license() }}} says all code except jsmath is GPL2, and apparently jsmath is ok, as Sage does not link to it. The code will not build on skynet, as there are no SPARC machines there. It should build it on 't2' easily though.

hmm, isn't this a sparc/solaris?

SunOS mark2 5.10 Generic_127111-01 sun4u sparc SUNW,Sun-Blade-2500

Dima

in reply to: ↑ 13 ; follow-up: ↓ 15   Changed 7 months ago by drkirkby

Replying to dimpase:

Replying to drkirkby:

You better check the license issue with William. Code should be GPL 2 or GPL2+, but there are exceptions if a package is optional and some other conditions - I've never fully understood under what conditions code can be GLP 3. But you might find you can only use the latest version which is GPL 2, and not a GPL 3 version.

Well, cvxopt is an optional package, so it must be in the same boat as jmath, or some gap packages, that are also not GPL 2.

Fair enough.

{{{ sage: license() }}} says all code except jsmath is GPL2, and apparently jsmath is ok, as Sage does not link to it. The code will not build on skynet, as there are no SPARC machines there. It should build it on 't2' easily though.

hmm, isn't this a sparc/solaris? SunOS mark2 5.10 Generic_127111-01 sun4u sparc SUNW,Sun-Blade-2500 Dima

Yes, it is. Sorry, I was not aware of the existance of that machine.

However, I do not know how the compilers and paths are configured on that machine. You need to have GNU make & GNU tar in your path before the Sun ones, and you need to have the Sun linker (/usr/ccs/bin/ld) in your path before any GNU ones. There are some general instructions on building Sage on Solaris at  http://wiki.sagemath.org/solaris which you would need to follow.

I've written some somewhat simpler instructions at  http://wiki.sagemath.org/devel/Building-Sage-on-the-T5240-t2 on how to build Sage on 't2'. They are simpler, as I have already put the right tools in the right locations.

The Sun Blade 2500 (mark2) should be quicker than the T5240 (t2) at building Sage. However it would require some setting up of the build environment to build Sage. If you just want an easy solution, 't2' will just work, albeit not as quickly as the Sun Blade 2500.

Dave

in reply to: ↑ 14 ; follow-up: ↓ 17   Changed 7 months ago by dimpase

Replying to drkirkby:

hmm, isn't this a sparc/solaris? SunOS mark2 5.10 Generic_127111-01 sun4u sparc SUNW,Sun-Blade-2500 Dima

Yes, it is. Sorry, I was not aware of the existance of that machine. However, I do not know how the compilers and paths are configured on that machine. You need to have GNU make & GNU tar in your path before the Sun ones, and you need to have the Sun linker (/usr/ccs/bin/ld) in your path before any GNU ones. There are some general instructions on building Sage on Solaris at  http://wiki.sagemath.org/solaris which you would need to follow.

well, on Skynet there is /usr/local/skynet_bash_profile that you can source upon login (from .bashrc, or just manually), and this gives you the ready setup to build Sage.

I don't have an account on t2, it seems to me. By the way, absent-mindedly I started building 4.3.3.alpha on there, and it went till gnutls, where it stopped... I noticed that gnutls is over 2 years old, version 2.2.2, whereas the current one is 2.8.5. Shouldn't one upgrade to this one, before even trying to fix this?

Dima

in reply to: ↑ 12   Changed 7 months ago by dimpase

Replying to drkirkby:

You better check the license issue with William. Code should be GPL 2 or GPL2+, but there are exceptions if a package is optional and some other conditions - I've never fully understood under what conditions code can be GLP 3. But you might find you can only use the latest version which is GPL 2, and not a GPL 3 version.

I emailed William about this, and he said it's OK in this case, it can be v3.

in reply to: ↑ 15   Changed 7 months ago by drkirkby

Replying to dimpase:

Replying to drkirkby:

However, I do not know how the compilers and paths are configured on that machine. You need to have GNU make & GNU tar in your path before the Sun ones, and you need to have the Sun linker (/usr/ccs/bin/ld) in your path before any GNU ones. There are some general instructions on building Sage on Solaris at  http://wiki.sagemath.org/solaris which you would need to follow.

well, on Skynet there is /usr/local/skynet_bash_profile that you can source upon login (from .bashrc, or just manually), and this gives you the ready setup to build Sage.

I'm not however aware of anyone building Sage on there recently, so I don't know if the environment is set up suitably. Quite a few Solaris-specific changes have been made in the last year, and some of them might not be compatible with the build system on there. I don't know. Specifically, if

gcc -v 

shows gcc was configured with the GNU linker, then the GNU linker must be in your path before the Sun linker. (Basically, whatever linker gcc uses, must be in your path first, as some code makes the assumption the first linker in your path is the one gcc uses, which might not be true.) I'm not aware of a foolproof test of this.

There should be something like

--with-ld=/usr/ccs/bin/ld

if the Sun linker was used, or

--with-ld=/path/to/gnu/ld

if the GNU linker was used.

Certainly, I've never had a problem with gnutls failing on 't2'.

I don't have an account on t2, it seems to me.

I've emailed William to ask if you can have an account on t2, as that might be the simplest solution, though that Blade 2500 would be significantly faster than 't2'.

By the way, absent-mindedly I started building 4.3.3.alpha on there, and it went till gnutls, where it stopped... I noticed that gnutls is over 2 years old, version 2.2.2, whereas the current one is 2.8.5. Shouldn't one upgrade to this one, before even trying to fix this? Dima

I can see your point about upgrading, though I'm not aware of any particular issues with the version in Sage. That version will build on Solaris. You could try appending /usr/sfw/lib to your LD_LIBRARY_PATH. I have known of issues with gnutls on OpenSolaris?, but not on Solaris 10.

I see your post about the GPL 3. That bit is sorted out then.

Dave

in reply to: ↑ 8   Changed 6 months ago by dimpase

  • status changed from needs_work to needs_review

Replying to dimpase:

Replying to schilly:

Here is an updated version, that also works on Solaris (this needed copying sun_complex.h from old cvxopt-0.9 and patching cvxopt.h). I also turned on building GSL-extension (by turning on the appropriate option in setup.py, and supplying right include and lib-paths)  http://boxen.math.washington.edu/home/dima/packages/cvxopt-1.1.2.p3.spkg

follow-up: ↓ 20   Changed 6 months ago by schilly

  • status changed from needs_review to needs_work
  • reviewer set to schilly

hi, great work, your 1.1.2.p3.spkg works for me on intel core2 duo with ubuntu 9.04 running 4.3.4.rc0.

It's just that the name shouldn't have the .p3, the SPKG.txt is not correct (your name missing), the patches aren't yours but still mine and there is no mention what you have really done with the *.h files for solaris. where is a good page to read about the spkg policies?!?

in reply to: ↑ 19 ; follow-up: ↓ 21   Changed 5 months ago by dimpase

  • status changed from needs_work to needs_review

Replying to schilly:

hi, great work, your 1.1.2.p3.spkg works for me on intel core2 duo with ubuntu 9.04 running 4.3.4.rc0. It's just that the name shouldn't have the .p3, the SPKG.txt is not correct (your name missing), the patches aren't yours but still mine and there is no mention what you have really done with the *.h files for solaris. where is a good page to read about the spkg policies?!?

OK, finally here comes an update:  http://boxen.math.washington.edu/home/dima/packages/cvxopt-1.1.2.spkg

Fixed SPKG.txt, added all the patches in patches/, further changes as we discussed by email (see SPKG.txt). Tested (with Sage 4.3.4) on Linux x86 and x86_64, Solaris (t2) and on MacOSX 10.5 PPC (G4)

in reply to: ↑ 20 ; follow-up: ↓ 22   Changed 5 months ago by schilly

Replying to dimpase:

Fixed SPKG.txt

spkg works for me on ubuntu 9.04, intel core2 duo and sage 4.3.4. I've just further cleaned up the SPKG.txt file since only the net changes are relevant and now there is also a title plus a description.

final spkg is  here

in reply to: ↑ 21 ; follow-up: ↓ 23   Changed 5 months ago by dimpase

Replying to schilly:

Replying to dimpase:

Fixed SPKG.txt

spkg works for me on ubuntu 9.04, intel core2 duo and sage 4.3.4. I've just further cleaned up the SPKG.txt file since only the net changes are relevant and now there is also a title plus a description. final spkg is  here

OK, great, thanks! How do we finalise this? Should I make myself the owner? Should I make you (and/or myself?) the author?

in reply to: ↑ 22   Changed 5 months ago by schilly

  • reviewer schilly deleted
  • author set to schilly, dimpase

Replying to dimpase:

OK, great, thanks! How do we finalise this? Should I make myself the owner? Should I make you (and/or myself?) the author?

Uhm, we both are the authors and well, we need a thrid party to give us a positive review ;)

follow-up: ↓ 25   Changed 5 months ago by ncohen

  • status changed from needs_review to positive_review

Hello !!!

This patch applies, the spkg compiles fine and is well built, everything runs in the end and the doctest using it passes !!!

The only trouble I could find is the need to disable the preparser (mentionned by Harald earlier) to use regular CVXOPT scripts in Sage, but this is not new and could be adressed later. Clearly, this is an improvement for Sage :-)

Thank you for your work !!!

Nathann

P.S. : what would you think of creating another ticket to add a line about this perparsing bug somewhere there : http://www.sagemath.org/doc/numerical_sage/cvxopt.html ?

in reply to: ↑ 24   Changed 5 months ago by schilly

Replying to ncohen:

P.S. : what would you think of creating another ticket to add a line about this perparsing bug somewhere there : http://www.sagemath.org/doc/numerical_sage/cvxopt.html ?

That's not a bug, the examples in that tutorial redefine Integer as int and similar workarounds. I prefer disabling the preparser or switching to pure python mode. However, maybe this could be done automatically somehow (as with numpy arrays?) but that's not the scope of this ticket.

Changed 5 months ago by schilly

trival changes to the cvxopt chapter in the numerical sage tutorial

follow-up: ↓ 27   Changed 5 months ago by jhpalmieri

Any ideas why the new spkg is so much smaller than the old one?

-rw-r--r-- 1 palmieri palmieri 2463336 2010-02-11 08:56 spkg/standard/cvxopt-0.9.p8.spkg
-rw-r--r-- 1 palmieri palmieri  733213 2010-03-24 09:49 spkg/standard/cvxopt-1.1.2.spkg

in reply to: ↑ 26 ; follow-up: ↓ 46   Changed 5 months ago by schilly

Replying to jhpalmieri:

Any ideas why the new spkg is so much smaller than the old one?

iirc we have removed documentation and examples. They are not exposed in any way or not used at all (tex sources).

follow-ups: ↓ 29 ↓ 35   Changed 4 months ago by mhansen

  • status changed from positive_review to needs_work

I don't think this is ready to go in. Some issues:

1. I don't think there should be a patches-old directory. If people need them for historical reasons, then they should get them from the hg repo since that's what it is there for.

2. Files are modified in place in the src/ directory. That should be as close to clean as vanilla upstream as possible. The modified files should be copied over from patches/

3. In the patches directory, the patches should be unified diffs (diff -Naur).

4. In spkg-install, you should just remove the old, unnecessary code instead of just commenting it out. Also, I don't think the SAGE_LOCAL check is necessary.

in reply to: ↑ 28 ; follow-up: ↓ 30   Changed 4 months ago by dimpase

Replying to mhansen:

I don't think this is ready to go in. Some issues: 1. I don't think there should be a patches-old directory. If people need them for historical reasons, then they should get them from the hg repo since that's what it is there for.

OK, this is clear. We need to check if we didn't nuke the old .hg/, and put it back, if necessary.

2. Files are modified in place in the src/ directory. That should be as close to clean as vanilla upstream as possible. The modified files should be copied over from patches/

OK, this is clear too (although this seems to be against the historic way cvxopt spkg was created)

3. In the patches directory, the patches should be unified diffs (diff -Naur).

I don't get this. Are you saying that patches cannot be just files that are ready to be copied, that they must be diffs, and the spkg-install must be patching rather than copying?

4. In spkg-install, you should just remove the old, unnecessary code instead of just commenting it out. Also, I don't think the SAGE_LOCAL check is necessary.

we'll see.

in reply to: ↑ 29 ; follow-up: ↓ 31   Changed 4 months ago by mhansen

Replying to dimpase:

I don't get this. Are you saying that patches cannot be just files that are ready to be copied, that they must be diffs, and the spkg-install must be patching rather than copying?

You should have both the patched file and the diff in the patches/ directory. The diff is useful when upgrading the spkg.

in reply to: ↑ 30 ; follow-up: ↓ 32   Changed 4 months ago by dimpase

Replying to mhansen:

Replying to dimpase:

I don't get this. Are you saying that patches cannot be just files that are ready to be copied, that they must be diffs, and the spkg-install must be patching rather than copying?

You should have both the patched file and the diff in the patches/ directory. The diff is useful when upgrading the spkg.

This seems to be an unnecessary duplication of information. The diffs can be trivially created, if needed.

in reply to: ↑ 31 ; follow-up: ↓ 33   Changed 3 months ago by mhansen

Replying to dimpase:

This seems to be an unnecessary duplication of information. The diffs can be trivially created, if needed.

They can be recreated if you have the correct version of the source from which the patched file was created. It's easier if the actual diff is in the version control.

in reply to: ↑ 32   Changed 7 weeks ago by dimpase

Replying to mhansen:

Replying to dimpase:

This seems to be an unnecessary duplication of information. The diffs can be trivially created, if needed.

They can be recreated if you have the correct version of the source from which the patched file was created. It's easier if the actual diff is in the version control.

I should have time next week to fix this, finally... Dima

  Changed 7 weeks ago by mhansen

There is also the issue at #9525 which needs to be taken care of.

Changed 7 weeks ago by drkirkby

Check for errors while installing cvxopt. Note there is still no spkg-check file. That may or may not be useful - it depends on the source code whether it supports tests.

in reply to: ↑ 28   Changed 5 weeks ago by dimpase

  • status changed from needs_work to needs_review

Replying to mhansen:

I don't think this is ready to go in. Some issues: 1. I don't think there should be a patches-old directory. If people need them for historical reasons, then they should get them from the hg repo since that's what it is there for. 2. Files are modified in place in the src/ directory. That should be as close to clean as vanilla upstream as possible. The modified files should be copied over from patches/ 3. In the patches directory, the patches should be unified diffs (diff -Naur). 4. In spkg-install, you should just remove the old, unnecessary code instead of just commenting it out. Also, I don't think the SAGE_LOCAL check is necessary.

the update, that takes your comments into account, is here  http://boxen.math.washington.edu/home/dima/packages/cvxopt-1.1.2.spkg

Please have a look, hopefully it is OK now. Dima

follow-up: ↓ 42   Changed 5 weeks ago by dunfield

This ticket should be coordinated with #9598, which adds GLPK support to cvxopt.

follow-up: ↓ 38   Changed 5 weeks ago by schilly

spkg looks fine for me, builds and also solves some randomly choosen example from the user guide. additionally, i tested if my patch for the documentation 6456-numerical_sage_cvxopt.patch still works and yes, it does.

regarding glpk, Dima are you able to enable the glpk flag and try building it? if it works i can test it again and see if that's fine - if it doesn't build and you run into bigger problems, we should finally update this spkg anyways and postpone glpk support in cvxopt.

in reply to: ↑ 37   Changed 5 weeks ago by dunfield

Replying to schilly:

regarding glpk, Dima are you able to enable the glpk flag and try building it? if it works i can test it again and see if that's fine - if it doesn't build and you run into bigger problems, we should finally update this spkg anyways and postpone glpk support in cvxopt.

There's a patch for glpk for the old cvxopt 0.9 at #9598. The changes there should work find for cvxopt 1.1 as well.

follow-up: ↓ 41   Changed 5 weeks ago by drkirkby

Looking in

cvxopt-1.1.2/src

I see

drwxr-xr-x   3 drkirkby staff          5 Mar 24 11:40 cvxopt-1.1.2
lrwxrwxrwx   1 drkirkby staff         16 Jul 26 16:10 src -> cvxopt-1.1.2/src

It is certainly unusual to do this. Normally the top level src directory contains the source, not another directory with a link like this. I know of no other package like this.

I'm also puzzled why we have this:

# Solaris-specific patches
cp -p patches/sun_complex.h src/src/C/
cp -p patches/cvxopt.h src/src/C/

There is no file src/src/C/sun_complex.h so the first line just creates copies the file /usr/include/complex.h from Solaris to the patches directory. That file is from Sun, and its doubtful we can legally distribute /usr/include/complex.h

The second patch, patches/cvxopt.h differs from src/src/C/cvxopt.h by very little. A diff shows:

drkirkby@hawk:~/sage-4.5.2.alpha0/spkg/optional/cvxopt-1.1.2$ diff -u src/src/C/cvxopt.h patches/cvxopt.h
--- src/src/C/cvxopt.h	Mon Jul 26 11:16:09 2010
+++ patches/cvxopt.h	Mon Jul 26 10:58:48 2010
@@ -26,7 +26,14 @@
 /* ANSI99 complex is disabled during build of CHOLMOD */
 
 #ifndef NO_ANSI99_COMPLEX
+
+/* work around Solaris 10 specific problem in complex.h */
+#if defined (__sun)
+#include "sun_complex.h"
+#else
 #include "complex.h"
+#endif
+
 #define MAT_BUFZ(O)  ((complex *)((matrix *)O)->buffer)
 #endif

If I'm not mistaken, all these two patches achieve is to

  • Add a file /usr/include/complex.h to Sage taken from Solaris, that it is doubtful we can legally include, though I doubt Sun (now Oracle) will complain.
  • Change a file patches/cvxopt.h to include the file we have just illegally copied.

It would to me at least be a lot easier to just change the second file so it had

#ifdef if defined __sun /* Need to check if that's the best one */ 
#include <complex.h>
#endif

Why are we bothering to copy a system file from Solaris, rather than just not use #include <complex.h>?

Dave

follow-up: ↓ 55   Changed 5 weeks ago by drkirkby

I should add, /usr/include/complex.h is included in the first release of Solaris 10 (released in March 2005) and also in the latest OpenSolaris build. There seems little point in doing what we are doing, espeically given it is doubtful if this is legal - the header is not released under the GPL.

Dave

in reply to: ↑ 39   Changed 5 weeks ago by schilly

Replying to drkirkby:

Why are we bothering to copy a system file from Solaris, rather than just not use #include <complex.h>?

Because that's the way it was done in 0.9 and nobody of us knows about this in such a detail.

in reply to: ↑ 36 ; follow-ups: ↓ 43 ↓ 44   Changed 5 weeks ago by dimpase

Replying to dunfield:

This ticket should be coordinated with #9598, which adds GLPK support to cvxopt.

please see

 http://boxen.math.washington.edu/home/dima/packages/cvxopt-1.1.2.p1.spkg

This incorporates the #9598 in 1.1.2. Only tested on Linux Debian 32bit. I won't be having much internet until after tomorrow.

Dima

in reply to: ↑ 42   Changed 5 weeks ago by schilly

Replying to dimpase:

This incorporates the #9598 in 1.1.2.

I tested it again and also tried to use glpk as an lp solver and it worked:

sage: from cvxopt import matrix, solvers
sage: c = matrix([-4., -5.])
sage: G = matrix([[2., 1., -1., 0.], [1., 2., 0., -1.]])
sage: h = matrix([3., 3., 0., 0.])
sage: sol = solvers.lp(c, G, h, solver='glpk')
GLPK Simplex Optimizer, v4.44
4 rows, 2 columns, 6 non-zeros
Preprocessing...
2 rows, 2 columns, 4 non-zeros
Scaling...
 A: min|aij| =  1.000e+00  max|aij| =  2.000e+00  ratio =  2.000e+00
Problem data seem to be well scaled
Constructing initial basis...
Size of triangular part = 2
*     0: obj =   0.000000000e+00  infeas =  0.000e+00 (0)
*     2: obj =  -9.000000000e+00  infeas =  0.000e+00 (0)
OPTIMAL SOLUTION FOUND

dear release manager, please don't forget to include GLPL as a dependency for cvxopt in the spkg/standard/deps file according to  this comment.

in reply to: ↑ 42   Changed 5 weeks ago by dunfield

This incorporates the #9598 in 1.1.2. Only tested on Linux Debian 32bit.

Works fine on OS X Leopard, used same test as shilly above.

follow-up: ↓ 48   Changed 5 weeks ago by drkirkby

  • status changed from needs_review to needs_info

Are you sure its a good idea to merge the changes from #9598? IMHO, it would be better to make these tickets separate, as:

  • #9598 has not been tested properly.
  • There is no documentation for the updates, so nothing to indicate that cvxopt can be used with the glpk solver.
  • There are no additional doc tests which show the output of using cvxopt with the glpk solver.
  • The author does not know if it's platform dependent or not, and says he has only checked on OS X.

I've marked #9598 as needs work, as based on what I deduce, it does need work before being what I personally consider acceptable.

Dave

in reply to: ↑ 27   Changed 5 weeks ago by drkirkby

Replying to schilly:

Replying to jhpalmieri:

Any ideas why the new spkg is so much smaller than the old one?

iirc we have removed documentation and examples. They are not exposed in any way or not used at all (tex sources).

I don't think that was such a good idea, as the documentation and the examples can be used to test the package. To quote from the INSTALL file.

Test it:
--------
To test that the installation was successful, go to the examples
directory and try one of the examples, for example,

    $ cd examples/doc/chap8
    $ python lp

So if a spkg-check file was created, whilst leaving the documentation and examples in place, it would be possible to check this.

Given there is only one .spkg which will be merged in 4.5.2, would it not be better to work on this a bit more? I can see some obvious things that could be improved.

  • Put back the examples and documentation.
  • Add an spkg-check file and test the examples - this will only happen if SAGE_CHECK=yes
  • Try removing all the Solaris specific patches. I just did a very quick test on an OpenSolaris machine, and found this built without those patches.
  • Change the directory structure to what is standard in Sage, and not as it is now.
  • Remove the patches from #9598, which IMHO have not been checked carefully.

I'll leave it up to you guys how you resolve it. If you want me to make a package based on that above, I'd be willing to do it.

Dave

  Changed 5 weeks ago by dunfield

Over at #9598, I just uploaded a change in the Sage docs of cvxopt which includes shilly's test of the glpk side. I also changed the URL for the original cvxopt docs; the current link is broken.

in reply to: ↑ 45 ; follow-up: ↓ 49   Changed 5 weeks ago by dimpase

Replying to drkirkby:

Are you sure its a good idea to merge the changes from #9598? IMHO, it would be better to make these tickets separate, as:

Dave, it's just the question of turning a particular interface on. There should be no problems --- most of all cause there is 0 exposure of this to any Sage code at the moment.

The next step would be to have a proper test for this somewhere...

* #9598 has not been tested properly. * There is no documentation for the updates, so nothing to indicate that cvxopt can be used with the glpk solver. * There are no additional doc tests which show the output of using cvxopt with the glpk solver. * The author does not know if it's platform dependent or not, and says he has only checked on OS X. I've marked #9598 as needs work, as based on what I deduce, it does need work before being what I personally consider acceptable. Dave

in reply to: ↑ 48   Changed 5 weeks ago by drkirkby

Replying to dimpase:

Replying to drkirkby:

Are you sure its a good idea to merge the changes from #9598? IMHO, it would be better to make these tickets separate, as:

Dave, it's just the question of turning a particular interface on. There should be no problems --- most of all cause there is 0 exposure of this to any Sage code at the moment. The next step would be to have a proper test for this somewhere...

But my understanding is that there should be a test, so code like that in #9598 can't be committed until there is a test and documentation for it - I note that some documentation has now been added, though I'm not sure about test code. It does not seem right to me to link to a library when

* Whether the linking on some platforms is untested. * The is no documentation to cvxopt to show how to use this library. * There is no test code.

(That was the situation at the time I marked it as needing work - that may have changed now).

Note also that cvxopt does have test code, which is not executed. Since that was not before, I'm not suggesting that should be made conditional on getting a positive review. But given this ticket will not be merged in 4.5.2 (as only one .spkg file will be), it would seem wise to sort out that too, and run the package's self-tests. That would mean restoring the documentation and examples, as that is how this code gets tested.

Sorry if I appear too pedantic, but I'm just trying to ensure that what we have works on all platforms, is tested on all platforms, and is documented properly.

Dave

Changed 5 weeks ago by pjeremy

follow-ups: ↓ 54 ↓ 80   Changed 5 weeks ago by pjeremy

6456-freebsd-spkg-install.patch adds support for FreeBSD (this is a port of the patch in #9601). I have compile-tested this but not yet tried to use the resultant module.

Note that further changes are necessary for cvxopt-1.1.2.p1.spkg to work on most 64-bit OSs. Compiling in FreeBSD/amd64 (with or without the above patch) gives:

gcc -fno-strict-aliasing -g -O2 -DNDEBUG -g -O3 -Wall -Wstrict-prototypes -I/mnt/sage-4.5/local/include -fPIC -DDLONG= -I/mnt/sage-4.5/local/include/python2.6 -c C/misc_solvers.c -o build/temp.freebsd-8.1-PRERELEASE-amd64-2.6/C/misc_solvers.o
C/misc_solvers.c: In function 'scale':
C/misc_solvers.c:152:13: warning: passing argument 1 of 'dscal_' from incompatible pointer type
C/misc_solvers.c:32:13: note: expected 'int *' but argument is of type 'Py_ssize_t *'
C/misc_solvers.c:152:13: warning: passing argument 4 of 'dscal_' from incompatible pointer type
C/misc_solvers.c:32:13: note: expected 'int *' but argument is of type 'Py_ssize_t *'
C/misc_solvers.c:155:13: warning: passing argument 3 of 'dgemv_' from incompatible pointer type
C/misc_solvers.c:39:13: note: expected 'int *' but argument is of type 'Py_ssize_t *'
C/misc_solvers.c:156:9: warning: passing argument 1 of 'dscal_' from incompatible pointer type
C/misc_solvers.c:32:13: note: expected 'int *' but argument is of type 'Py_ssize_t *'
C/misc_solvers.c:156:9: warning: passing argument 4 of 'dscal_' from incompatible pointer type
C/misc_solvers.c:32:13: note: expected 'int *' but argument is of type 'Py_ssize_t *'
C/misc_solvers.c:158:13: warning: passing argument 2 of 'dger_' from incompatible pointer type
C/misc_solvers.c:41:13: note: expected 'int *' but argument is of type 'Py_ssize_t *'
C/misc_solvers.c:160:13: warning: passing argument 1 of 'dscal_' from incompatible pointer type
C/misc_solvers.c:32:13: note: expected 'int *' but argument is of type 'Py_ssize_t *'
C/misc_solvers.c:160:13: warning: passing argument 4 of 'dscal_' from incompatible pointer type
C/misc_solvers.c:32:13: note: expected 'int *' but argument is of type 'Py_ssize_t *'
C/misc_solvers.c: In function 'pack2':
C/misc_solvers.c:459:17: warning: passing argument 3 of 'dlacpy_' from incompatible pointer type
C/misc_solvers.c:49:13: note: expected 'int *' but argument is of type 'Py_ssize_t *'
C/misc_solvers.c:459:17: warning: passing argument 5 of 'dlacpy_' from incompatible pointer type
C/misc_solvers.c:49:13: note: expected 'int *' but argument is of type 'Py_ssize_t *'
C/misc_solvers.c:461:17: warning: passing argument 1 of 'dscal_' from incompatible pointer type
C/misc_solvers.c:32:13: note: expected 'int *' but argument is of type 'Py_ssize_t *'
C/misc_solvers.c:463:17: warning: passing argument 3 of 'dlacpy_' from incompatible pointer type
C/misc_solvers.c:49:13: note: expected 'int *' but argument is of type 'Py_ssize_t *'
C/misc_solvers.c:463:17: warning: passing argument 7 of 'dlacpy_' from incompatible pointer type
C/misc_solvers.c:49:13: note: expected 'int *' but argument is of type 'Py_ssize_t *'

Py_ssize_t is typedef'd from ssize_t, which is long on at least FreeBSD, Linux and Solaris. I believe this is a blocking issue.

follow-up: ↓ 52   Changed 5 weeks ago by jhpalmieri

While this builds on t2.math, it fails to build on mark (a skynet solaris machine). Of course, the old version of cvxopt doesn't build on mark, either...

With either version, I get this, right at the start of the build:

running build_ext
building 'glpk' extension
creating build/temp.solaris-2.10-sun4u-2.6
creating build/temp.solaris-2.10-sun4u-2.6/C
gcc -fno-strict-aliasing -g -O2 -DNDEBUG -g -fwrapv -O3 -Wall -Wstrict-prototypes -fPIC -I/home/pa\
lmieri/mark/sage-4.5.2.alpha1/local/include -I/home/palmieri/mark/sage-4.5.2.alpha1/local/include/\
python2.6 -c C/glpk.c -o build/temp.solaris-2.10-sun4u-2.6/C/glpk.o
In file included from C/cvxopt.h:32:0,
                 from C/glpk.c:20:
C/sun_complex.h:33:32: error: expected identifier or '(' before '_Imaginary'
error: command 'gcc' failed with exit status 1
Error building/installing cvxopt

in reply to: ↑ 51   Changed 5 weeks ago by drkirkby

Replying to jhpalmieri:

While this builds on t2.math, it fails to build on mark (a skynet solaris machine). Of course, the old version of cvxopt doesn't build on mark, either... With either version, I get this, right at the start of the build: {{{ running build_ext building 'glpk' extension creating build/temp.solaris-2.10-sun4u-2.6 creating build/temp.solaris-2.10-sun4u-2.6/C gcc -fno-strict-aliasing -g -O2 -DNDEBUG -g -fwrapv -O3 -Wall -Wstrict-prototypes -fPIC -I/home/pa\ lmieri/mark/sage-4.5.2.alpha1/local/include -I/home/palmieri/mark/sage-4.5.2.alpha1/local/include/\ python2.6 -c C/glpk.c -o build/temp.solaris-2.10-sun4u-2.6/C/glpk.o In file included from C/cvxopt.h:32:0, from C/glpk.c:20: C/sun_complex.h:33:32: error: expected identifier or '(' before '_Imaginary' error: command 'gcc' failed with exit status 1 Error building/installing cvxopt }}}

John, what is the md5 checksum of the package you used? Where did you get it from? I took Dima's package

 http://boxen.math.washington.edu/home/dima/packages/cvxopt-1.1.2.p1.spkg

(md5 = 38681db8e19f69b0e7972c5278e8e183)

but get no such error message on t2.math (Solaris 10). I do notice a lot of unused variables, and other compiler warnings, which always make me a bit weary of code.

I think in light of what Peter Jeremy has found, it would be foolish to not add an spkg-check file and execute the self-tests for cvxopt.

Dave

  Changed 5 weeks ago by jhpalmieri

I misspoke earlier. I haven't tried to build this on t2, so I don't know if it builds there. I have of course built the old version on t2 with no problem.

On the skynet machine mark, I can't build either the old version or the new one (= Dima's package), despite using the new gcc compiler with the sun linker which successfully built lots of other packages, making more progress than I'd seen before on that machine. (I get the same md5sum as you posted.)

in reply to: ↑ 50 ; follow-up: ↓ 67   Changed 5 weeks ago by dimpase

Replying to pjeremy:

6456-freebsd-spkg-install.patch adds support for FreeBSD (this is a port of the patch in #9601). I have compile-tested this but not yet tried to use the resultant module. Note that further changes are necessary for cvxopt-1.1.2.p1.spkg to work on most 64-bit OSs.

could it simply be that some -m64 or whatever flag settings to be added to spkg-install?

It can very well be that the standalone cvxopt does not work on that fancy 64-bit systems anyway. If this is the case, I am not willing to do anything on this at this ticket.

Last but not least, I would object in strongest possible terms to call a blocker an issue that is present in the current cvxopt (0.9) spkg. We must upgrade, and then try to improve, and not sit endlessly here...

in reply to: ↑ 40 ; follow-up: ↓ 56   Changed 5 weeks ago by dimpase

Replying to drkirkby:

I should add, /usr/include/complex.h is included in the first release of Solaris 10 (released in March 2005) and also in the latest OpenSolaris build. There seems little point in doing what we are doing, espeically given it is doubtful if this is legal - the header is not released under the GPL.

sun_complex.h is in the current cvxopt spkg. I added the sun_complex.h inclusion to 1.1.2 cause I was not able to make it work on t2 otherwise. So I just went with the solution that is known to work.

Note that <complex.h> is included in cvxopt.h by default, albeit under some condition, etc.

If you have a better solution that works on t2, please post a patch here, and I make cvxopt-1.1.2.p2.spkg that incorporates it. I am also willing to put back examples and docs.

Dima

in reply to: ↑ 55   Changed 5 weeks ago by drkirkby

Replying to dimpase:

Replying to drkirkby:

I should add, /usr/include/complex.h is included in the first release of Solaris 10 (released in March 2005) and also in the latest OpenSolaris build. There seems little point in doing what we are doing, espeically given it is doubtful if this is legal - the header is not released under the GPL.

sun_complex.h is in the current cvxopt spkg. I added the sun_complex.h inclusion to 1.1.2 cause I was not able to make it work on t2 otherwise. So I just went with the solution that is known to work. Note that <complex.h> is included in cvxopt.h by default, albeit under some condition, etc. If you have a better solution that works on t2, please post a patch here, and I make cvxopt-1.1.2.p2.spkg that incorporates it. I am also willing to put back examples and docs. Dima

The current patch for Solaris does not work at all with gcc 4.5, as there's a syntax error in the code I wrote above, which you copied.

This line is not legal C

#ifdef if defined __sun /* Need to check if that's the best one */

and gives a warning from the compiler

In file included from C/glpk.c:20:0:
C/cvxopt.h:30:11: warning: extra tokens at end of #ifdef directive

It then goes on to give an error message, as I suspect the header file <complex.h> does not get included. I suspect gcc is interpreting this as

#ifdef if

I think we need the documentation and examples back, add an spkg-check file, and run all the self-tests. Unfortunately, I don't have time to do any of that now, though I can do it later today. (My wife is waiting on me to do some things in the house!)

Note Peter's problem is new to this upgrade, and the warning looks to me that it could cause a problem on any 64-bit system - irrespective of whether -m64 is added as a compiler flag. In fact, I doubt Peter ever uses -m64 in his builds on FreeBSD.

Dave

  Changed 5 weeks ago by drkirkby

I'm working on a version now which runs the self-tests and hopefully builds on all platforms. I'm still concerned about what Peter found though. Give me an hour or two, and I'll post a new package.

Dave

  Changed 5 weeks ago by drkirkby

  • status changed from needs_info to needs_work

I've spent as long on this as I am able to for a while, so I thought I'd post what I have got.

Note, that since 1.12 has never been merged into sage, this should be called 1.12 and not 1.12.p0, 1.12.p1 or similar. There's noting wrong with people adding a temporary letter or patch level if they want, but when its finally merged, it should not have all these temporary builds.

I've created a package which builds on Solaris, but it fails all self-tests on Solaris.

 http://boxen.math.washington.edu/home/kirkby/patches/cvxopt-1.1.2.spkg

It builds on Linux, but fails about half the self-tests on Linux.

To run the self-tests, type

$ export SAGE_CHECK=yes
$ ./sage -i http://boxen.math.washington.edu/home/kirkby/patches/cvxopt-1.1.2.spkg

I've not committed any changes.

I'm not sure what to make of this. I don't feel this is ready, given the high failure rate of the self-tests.

Dave

follow-up: ↓ 60   Changed 5 weeks ago by jhpalmieri

Two quick comments: both Dima's and Dave's packages seem to contain a file .SPKG.txt.swp, which should not be there. Also, I can't build these on the skynet machine mark (sparc solaris), but I can if (following Dave's suggestion), I comment out these lines in spkg-install:

cp -p patches/sun_complex.h src/src/C/
cp -p patches/cvxopt.h src/src/C/

I haven't tried building on t2 with this modification.

in reply to: ↑ 59 ; follow-up: ↓ 68   Changed 5 weeks ago by drkirkby

Replying to jhpalmieri:

Two quick comments: both Dima's and Dave's packages seem to contain a file .SPKG.txt.swp, which should not be there.

Well spotted.

Also, I can't build these on the skynet machine mark (sparc solaris), but I can if (following Dave's suggestion), I comment out these lines in spkg-install: {{{ cp -p patches/sun_complex.h src/src/C/ cp -p patches/cvxopt.h src/src/C/ }}}

It's odd, since if I comment those lines out on OpenSolaris, it fails to build:

copying python/coneprog.py -> build/lib.solaris-2.11-i86pc-2.6/cvxopt
running build_ext
building 'base' extension
creating build/temp.solaris-2.11-i86pc-2.6
creating build/temp.solaris-2.11-i86pc-2.6/C
gcc -DNDEBUG -g -O3 -m64 -Wall -Wstrict-prototypes -m64 -fPIC -I/export/home/drkirkby/sage-4.5.2.alpha1/local/include/python2.6 -c C/base.c -o build/temp.solaris-2.11-i86pc-2.6/C/base.o
C/base.c: In function 'convert_znum':
C/base.c:156: error: '_Imaginary_I' undeclared (first use in this function)
C/base.c:156: error: (Each undeclared identifier is reported only once
C/base.c:156: error: for each function it appears in.)
C/base.c: In function 'initbase':
C/base.c:1727: warning: dereferencing type-punned pointer will break strict-aliasing rules
C/base.c:1736: warning: dereferencing type-punned pointer will break strict-aliasing rules
error: command 'gcc' failed with exit status 1
Error building/installing cvxopt

I think Dima's package

 http://boxen.math.washington.edu/home/dima/packages/cvxopt-1.1.2.p1.spkg

has a syntax error in the patch, so that will fail on Solaris. But my own package

 http://boxen.math.washington.edu/home/kirkby/patches/cvxopt-1.1.2.spkg

lacks that syntax error. But it seems to be necessary to add patches on some version of Solaris and messes up the build on another.

I haven't tried building on t2 with this modification.

Do not be too surprised if you need to change it again!

I will need to double-check if the old version of cvxopt was building on OpenSolaris. I think it was, but #9525 shows that the package would report it had built, even if it failed.

I've deleted my older builds on OpenSolaris but since I use a ZFS file system, with snapshots enabled every 15 minutes, so I can easily get back to an older build. Or I could just rebuild an older Sage on here, as it takes less than half an hour. I do need to check what's happening here.

Unfortunately, the last few weeks I seem to be spending a lot of time trying to test things on Solaris for people. I've been involved in the cvxopt, Pari, Singular and some other package updates.

Dave

follow-up: ↓ 73   Changed 5 weeks ago by drkirkby

I'm attaching a log of the test results on Linux. Either there is a big problem here, or my code for the testing is wrong. The cvxopt does not have a simple make check as on most packages, so there is a bigger chance of an error on my part.

Changed 5 weeks ago by drkirkby

Results from running cvxopt's self tests on a Linux system (sage.math), but setting SAGE_CHECK=yes.

follow-up: ↓ 66   Changed 5 weeks ago by schilly

Replying to drkirkby:

I'm not sure what to make of this. I don't feel this is ready, given the high failure rate of the self-tests.

I've run them locally and many open a plot window. Do you have installed all the appropriate libraries for plotting? For me, this smells like it is clearly not designed for automated testing because I had to close all windows manually. Besides that, your script also tests /chap7/covsel.bin which is not a python file and also not found by the appropriate /chap7/covsel script due to relative paths.

I appreciate to try to include testing - and in general it makes sense to do it - but my point of view is that it is much more important to update this years old library and get compiling working on all platforms and resort fixing the library later in separate tickets. 0.9 is not useful at all.

  Changed 5 weeks ago by schilly

ok, i have to take that back, with spkg-check there are no popup windows and with the exception of this covsel/covsel.bin file everything passes. The error in your log is nearly always this ZZ thing, I don't know where this comes from ... it doesn't happen for me.

Changed 5 weeks ago by schilly

ubuntu 10.4, pentium 4

  Changed 5 weeks ago by mhansen

There are still currently some problems with the spkg.

- All of the .patch files are made in the wrong direction (i.e. removing Sage-specific code and adding generic code).

- On OSX, the package links against libgslcblas which is not what we want to do. See #3435. The blas libraries for OSX are found in /usr/lib. We also don't build ATLAS on OS X.

- This fails on Cygwin as well since we don't build ATLAS there. There are BLAS libraries in /usr/lib as well.

  Changed 5 weeks ago by schilly

and btw, covsel also works and looks like this:

harri@stdbox:~/Downloads/cvxopt-1.1.2/src/examples/doc/chap7$ sage -python covsel
500 rows/columns, 1741 nonzeros

Newton decrement squared: 5.01869e+08
Newton decrement squared: 1.29139e+08
Newton decrement squared: 3.26344e+07
Newton decrement squared: 1.14508e+02
Newton decrement squared: 2.68329e+01
Newton decrement squared: 1.52504e+00
Newton decrement squared: 5.25935e-03
Newton decrement squared: 6.89978e-08
Newton decrement squared: 1.34440e-17
number of iterations:  9
harri@stdbox:~/Downloads/cvxopt-1.1.2/src/examples/doc/chap7$ echo "$?"
0

in reply to: ↑ 62 ; follow-up: ↓ 78   Changed 5 weeks ago by drkirkby

Replying to schilly:

Replying to drkirkby:

I'm not sure what to make of this. I don't feel this is ready, given the high failure rate of the self-tests.

I've run them locally and many open a plot window. Do you have installed all the appropriate libraries for plotting?

Probably not.

For me, this smells like it is clearly not designed for automated testing because I had to close all windows manually.

That may be true. I don't know exactly what exists on sage.math. But many of the messages do not seem to indicate such a problem to me. Things like:

   from sage.misc.prandom import seed
   ImportError: cannot import name seed

does not look like a plotting problem or library to me. Would you agree?

Besides that, your script also tests /chap7/covsel.bin which is not a python file and also not found by the appropriate /chap7/covsel script due to relative paths.

Yes, I had noticed that, but given the large number of problems, it was only one in around 20 tests. I can easily change that, but it seems to be in the noise at the minute.

I appreciate to try to include testing - and in general it makes sense to do it - but my point of view is that it is much more important to update this years old library and get compiling working on all platforms and resort fixing the library later in separate tickets. 0.9 is not useful at all.

At the minute, this is certainly worst than the previous version on Solaris.

The warning messages noticed by Peter look pretty serious to me and are new to this version. Those occur on any platform.

I personally feel these issues should be resolved, otherwise we could end up making matters worst. If the upgrade does not get done properly now, it probably never will.

Dave

in reply to: ↑ 54 ; follow-ups: ↓ 70 ↓ 75   Changed 5 weeks ago by pjeremy

Replying to dimpase:

Replying to pjeremy:

6456-freebsd-spkg-install.patch adds support for FreeBSD (this is a port of the patch in #9601). I have compile-tested this but not yet tried to use the resultant module. Note that further changes are necessary for cvxopt-1.1.2.p1.spkg to work on most 64-bit OSs.

could it simply be that some -m64 or whatever flag settings to be added to spkg-install?

No. The code is wrong/buggy/broken. The breakage is probably hidden in 32-bit builds.

It can very well be that the standalone cvxopt does not work on that fancy 64-bit systems anyway. If this is the case, I am not willing to do anything on this at this ticket.

I do not consider an amd64/x86_64 system to be "fancy". I suspect that anyone wanting to do serious work with Sage will be using a 64-bit system.

Last but not least, I would object in strongest possible terms to call a blocker an issue that is present in the current cvxopt (0.9) spkg. We must upgrade, and then try to improve, and not sit endlessly here...

There is little point in upgrading to a package that is known to be broken. This particular bug does not appear to be present in cvxopt-0.9 (at least I can't find the "incompatible pointer type" warnings in either my own or boxen builds) so by upgrading, we would be introducing a regression into Sage. I am very concerned at this "release it now, we'll make it work later" mentality. If Sage is going to be a viable alternative to the M's, it needs to be trustworthy - complaints of "feature X is missing" are easily rectified, claims of "Sage gave me wrong answers" can quickly turn into "you can't trust the output from Sage" and are far more difficult to refute.

in reply to: ↑ 60 ; follow-up: ↓ 69   Changed 5 weeks ago by jhpalmieri

Replying to drkirkby:

But my own package lacks that syntax error. But it seems to be necessary to add patches on some version of Solaris and messes up the build on another.

It still fails to build on mark:

building 'glpk' extension
creating build/temp.solaris-2.10-sun4u-2.6
creating build/temp.solaris-2.10-sun4u-2.6/C
gcc -fno-strict-aliasing -g -O2 -DNDEBUG -g -fwrapv -O3 -Wall -Wstrict-prototypes -fPIC -I/home/palmieri/mark/sage-4.5.2.alpha1/local/include -I/home/palmieri/mark/sage-4.5.2.alpha1/local/include/python2.6 -c C/glpk.c -o build/temp.solaris-2.10-sun4u-2.6/C/glpk.o
In file included from C/cvxopt.h:32:0,
                 from C/glpk.c:20:
C/sun_complex.h:33:32: error: expected identifier or '(' before '_Imaginary'
error: command 'gcc' failed with exit status 1
Error building/installing cvxopt

I'm not sure what syntax error you're referring to, but I don't see a difference in any of the files in the "patches" directory between your spkg and Dima's.

in reply to: ↑ 68   Changed 5 weeks ago by drkirkby

Replying to jhpalmieri:

Replying to drkirkby:

But my own package lacks that syntax error. But it seems to be necessary to add patches on some version of Solaris and messes up the build on another.

It still fails to build on mark:

Did the previous version build on the Solaris host 'mark' on Skynet?

I'm not sure what syntax error you're referring to, but I don't see a difference in any of the files in the "patches" directory between your spkg and Dima's.

In a build log I see of yours on mark, there was a warning reported about extra items on line 30 (IIRC) of a header file. That came from the code

#ifdef if defined __sun /* Need to check if that's the best one */ 

which I typed above in this ticket, but appears to have been copied into one of the versions of this package. That code was incorrectly typed by me - it was put as a comment in this ticket, and I'd not checked it with a C compiler. It certainly does not what do what I had intended. I assumed that was in Dima's package, but I appolise if I was mistaken. Either way, the code is wrong.

Dave

in reply to: ↑ 67   Changed 5 weeks ago by drkirkby

Replying to pjeremy:

There is little point in upgrading to a package that is known to be broken.

Agreed.

I don't think some of the people commenting on the ticket realise what that warning is about. In fact, I personally feel gcc should consider that an error and not just issue a warning. I doubt the Sun compiler would permit that code.

This particular bug does not appear to be present in cvxopt-0.9 (at least I can't find the "incompatible pointer type" warnings in either my own or boxen builds)

Me neither. That bug is a regression.

so by upgrading, we would be introducing a regression into Sage.

Yes.

I am very concerned at this "release it now, we'll make it work later" mentality.

Me too. There is far too much emphasis in Sage of adding features and far too little in controlling quality. This ticket seems to be a prime example of that.

If Sage is going to be a viable alternative to the M's, it needs to be trustworthy -

Yes.

complaints of "feature X is missing" are easily rectified, claims of "Sage gave me wrong answers" can quickly turn into "you can't trust the output from Sage" and are far more difficult to refute.

Yes. Likewise complaints of Sage crashed damages Sage's reputation. The bug you found could certainly cause a crash.

Dave

follow-up: ↓ 72   Changed 5 weeks ago by drkirkby

I see three possible ways forward with this ticket - there might be others, but these two seem the most likely to get a positive result.

  • Try an earlier release (i.e newer than 0.9, but not the latest)
  • Contact the author, making him aware of the bug Peter found - i.e. passing 64-bit pointers when the code is expecting 32-bit ones. Also point out there are a lot of variables declared, which are never used.
  • Try without the GLPK linking. It's possible the error is only seen when linking to that, though I don't think that's the case.

Dave

in reply to: ↑ 71 ; follow-up: ↓ 74   Changed 5 weeks ago by pjeremy

Replying to drkirkby:

* Try an earlier release (i.e newer than 0.9, but not the latest)

I don't see any benefit in this. The only justification I can find mentioned in this thread for updating cvxopt is that the current version is "ancient". Unless someone comes up with a more compelling reason for updating, I would suggest sticking to the current spkg until problems with cvxopt-1.1.2 are resolved.

* Contact the author, making him aware of the bug Peter found - i.e. passing 64-bit pointers when the code is expecting 32-bit ones. Also point out there are a lot of variables declared, which are never used.

To be precise, the code is passing pointers to 64-bit objects to functions expecting pointers to 32-bit objects.

I started to look at how difficult it would be to fix this morning but ran out of train journey before I got very far.

Also, whilst reading through the revision history for cvxopt, I notice that there have been a couple of changes that don't appear to be backward compatible: The cvxopt.random module was deleted in 0.9.2 and the definition of bool(A) (where A is a matrix) was changed in 1.1. Are these changes likely to impact other components of Sage?

in reply to: ↑ 61   Changed 5 weeks ago by dimpase

Replying to drkirkby:

I'm attaching a log of the test results on Linux. Either there is a big problem here, or my code for the testing is wrong. The cvxopt does not have a simple make check as on most packages, so there is a bigger chance of an error on my part.

I am presently using Sage+cvxopt-1.1.2 in my research computations, and results I get make sense, so I cannot expect the problem you report being too hard to fix. I'll have a look now.

Dima

in reply to: ↑ 72 ; follow-up: ↓ 76   Changed 5 weeks ago by dimpase

Replying to pjeremy:

Replying to drkirkby:

* Try an earlier release (i.e newer than 0.9, but not the latest)

I don't see any benefit in this. The only justification I can find mentioned in this thread for updating cvxopt is that the current version is "ancient". Unless someone comes up with a more compelling reason for updating, I would suggest sticking to the current spkg until problems with cvxopt-1.1.2 are resolved.

* Contact the author, making him aware of the bug Peter found - i.e. passing 64-bit pointers when the code is expecting 32-bit ones. Also point out there are a lot of variables declared, which are never used.

To be precise, the code is passing pointers to 64-bit objects to functions expecting pointers to 32-bit objects. I started to look at how difficult it would be to fix this morning but ran out of train journey before I got very far. Also, whilst reading through the revision history for cvxopt, I notice that there have been a couple of changes that don't appear to be backward compatible: The cvxopt.random module was deleted in 0.9.2 and the definition of bool(A) (where A is a matrix) was changed in 1.1. Are these changes likely to impact other components of Sage?

I am not aware of any Sage component that uses cvxopt. Surely, upgrading from 0.9 to 1.1 will break some code using cvxopt, but this is to be expected. The structure of the library has changed somewhat, so some imports might need to be fixed.

Regarding random, cvxopt has switched to using external random sources before 0.9.8, which is the currently used in Sage 4.5.1 spkg version. So I do not see why this is relevant at this point at all.

in reply to: ↑ 67 ; follow-up: ↓ 77   Changed 5 weeks ago by dimpase

Replying to pjeremy:

Replying to dimpase:

Replying to pjeremy:

6456-freebsd-spkg-install.patch adds support for FreeBSD (this is a port of the patch in #9601). I have compile-tested this but not yet tried to use the resultant module. Note that further changes are necessary for cvxopt-1.1.2.p1.spkg to work on most 64-bit OSs.

could it simply be that some -m64 or whatever flag settings to be added to spkg-install?

No. The code is wrong/buggy/broken. The breakage is probably hidden in 32-bit builds.

It can very well be that the standalone cvxopt does not work on that fancy 64-bit systems anyway. If this is the case, I am not willing to do anything on this at this ticket.

I do not consider an amd64/x86_64 system to be "fancy". I suspect that anyone wanting to do serious work with Sage will be using a 64-bit system.

Last but not least, I would object in strongest possible terms to call a blocker an issue that is present in the current cvxopt (0.9) spkg. We must upgrade, and then try to improve, and not sit endlessly here...

There is little point in upgrading to a package that is known to be broken.

There is also little point in keeping 0.9.8 in Sage! It's next to impossible to use 0.9.8 for anything serious, as it's not supported, it is known to be buggy in one or another way (one can dig this up in cvxopt archives, if needed), there are no easy to find examples to look at for 0.9, etc etc etc.

Sticking to 0.9.8 is the same as having no cvxopt in Sage at all...

in reply to: ↑ 74   Changed 5 weeks ago by drkirkby

Replying to dimpase:

Replying to pjeremy:

Replying to drkirkby:

* Try an earlier release (i.e newer than 0.9, but not the latest)

I don't see any benefit in this. The only justification I can find mentioned in this thread for updating cvxopt is that the current version is "ancient". Unless someone comes up with a more compelling reason for updating, I would suggest sticking to the current spkg until problems with cvxopt-1.1.2 are resolved.

* Contact the author, making him aware of the bug Peter found - i.e. passing 64-bit pointers when the code is expecting 32-bit ones. Also point out there are a lot of variables declared, which are never used.

To be precise, the code is passing pointers to 64-bit objects to functions expecting pointers to 32-bit objects. I started to look at how difficult it would be to fix this morning but ran out of train journey before I got very far. Also, whilst reading through the revision history for cvxopt, I notice that there have been a couple of changes that don't appear to be backward compatible: The cvxopt.random module was deleted in 0.9.2 and the definition of bool(A) (where A is a matrix) was changed in 1.1. Are these changes likely to impact other components of Sage?

I am not aware of any Sage component that uses cvxopt. Surely, upgrading from 0.9 to 1.1 will break some code using cvxopt, but this is to be expected. The structure of the library has changed somewhat, so some imports might need to be fixed. Regarding random, cvxopt has switched to using external random sources before 0.9.8, which is the currently used in Sage 4.5.1 spkg version. So I do not see why this is relevant at this point at all.

The pointer problem could potentially cause segfaults and data corruption. That's my single biggest concern. Not a single test passed on my Solaris build.

There appears to be a split view on this ticket

  • Those that want it updated for the functionality.
  • Those that don't want the current code updated because they feel it's a regression.

How about making the updated version an optional or experimental package? (Personally I feel the latter is more appropriate). Those that feel they need the update can use it, whilst those that consider it makes Sage less stable will simply not bother installing it.

I think Peter's comments about the:

"release it now, we'll make it work later" mentality

is very true here.

Dave

in reply to: ↑ 75   Changed 5 weeks ago by drkirkby

Replying to dimpase:

Sticking to 0.9.8 is the same as having no cvxopt in Sage at all...

Why was 0.9.8 ever put in Sage if its so useless?

Your comment about 0.9.8 being buggy is interesting.

Version 1.1.2 is most obviously buggy.

Dave

in reply to: ↑ 66 ; follow-up: ↓ 79   Changed 5 weeks ago by dimpase

Replying to drkirkby:

Replying to schilly:

Replying to drkirkby:

I'm not sure what to make of this. I don't feel this is ready, given the high failure rate of the self-tests.

I've run them locally and many open a plot window. Do you have installed all the appropriate libraries for plotting?

Probably not.

For me, this smells like it is clearly not designed for automated testing because I had to close all windows manually.

That may be true. I don't know exactly what exists on sage.math. But many of the messages do not seem to indicate such a problem to me. Things like: {{{ from sage.misc.prandom import seed ImportError?: cannot import name seed }}} does not look like a plotting problem or library to me. Would you agree?

Besides that, your script also tests /chap7/covsel.bin which is not a python file and also not found by the appropriate /chap7/covsel script due to relative paths.

Yes, I had noticed that, but given the large number of problems, it was only one in around 20 tests. I can easily change that, but it seems to be in the noise at the minute.

I appreciate to try to include testing - and in general it makes sense to do it - but my point of view is that it is much more important to update this years old library and get compiling working on all platforms and resort fixing the library later in separate tickets. 0.9 is not useful at all.

At the minute, this is certainly worst than the previous version on Solaris. The warning messages noticed by Peter look pretty serious to me and are new to this version. Those occur on any platform. I personally feel these issues should be resolved, otherwise we could end up making matters worst. If the upgrade does not get done properly now, it probably never will.

Dave,

your spkg-check is buggy. You probably end up calling a very wrong python to run the examples. E.g. if I cd to cvxopt-1.1.2/src/examples/doc/chap8 make a symbolic link mcsdp.py to mcsdp, fire up sage, and do

sage: load('mcsdp.py')

it happily runs without any errors etc. And this is one of examples your spkg-check fails on with weird error messages. So the problem does not seem to be in cvxopt here, but rather in your script... Dima

in reply to: ↑ 78   Changed 5 weeks ago by drkirkby

Replying to dimpase:

Dave, your spkg-check is buggy. You probably end up calling a very wrong python to run the examples. E.g. if I cd to cvxopt-1.1.2/src/examples/doc/chap8 make a symbolic link mcsdp.py to mcsdp, fire up sage, and do sage: load('mcsdp.py') it happily runs without any errors etc. And this is one of examples your spkg-check fails on with weird error messages. So the problem does not seem to be in cvxopt here, but rather in your script... Dima

Dima,

I made it very clear that "I've spent as long on this as I am able to for a while, so I thought I'd post what I have got." I also made it clear I'd not committed the changes - a clear reflection I was not confident of them all.

 http://trac.sagemath.org/sage_trac/ticket/6456#comment:58

If the wrong python is being called, that's a bug, as the first in the path, which is the one in Sage, should be called. One could work around that with "$SAGE_LOCAL/nib/python"

I'm aware of the issue with calling the .bin file, but as I remarked above, that's in the noise compared to the most significant issues of warnings from the compiler.

Feel free to improve the test suite. Rather than execute them all in a loop, perhaps its better to cd to the directory and run them from there. But those changes are not going to get around the more serious issue, which is seen well before the test suite is run.

I'm involved in many things at Sage at the moment.

  • This ticket
  • #9343 - the upgrade of Pari, which is a non-trivial issue
  • #9281 - trying to get more self-tests into Sage.
  • #8059 - update Singular SPKG to newest upstream release. Another big package

In addition, I've involved in same way with other less time consuming tickets

  • #9533: Update GSL to the latest upstream release (1.14) & permit parallel building.
  • #9568: Update IML to the newest upstream release, and improve spkg-install
  • #9603: Force iconv to build + install on HP-UX. Currently it is only installed on Solaris and Cygwin.

I really don't have a huge amount of time to devote to this one. I'm less inclined to devote time to improving the self-tests on this package, as its clear the package is more seriously broken when its compiled.

I seem to get cc'ed on a number of tickets where people can't be bothered to build on Solaris, so they think I will do the work for them. I'm tending to do that less now. There should be a much larger disk on t2 soon, so there will be even less excuse for people to pester me do do the Solaris checking. I never pester others other check code on Linux or OS X. I just do it myself.

To be fair Dima, this is not aimed at you, as you have checked code on Solaris many times.

With all the comments above on this ticket, how many people have actually made an effort to test cvxopt on a few computers with a few different operating systems. Compare that with

#9533: Update GSL to the latest upstream release (1.14) & permit parallel building.

where a lot of people have tested the code on multiple operating systems, with multiple compilers and run all the self-tests and run all the doctests on popular platforms. That's been done on

Dave

in reply to: ↑ 50 ; follow-up: ↓ 82   Changed 5 weeks ago by dimpase

  • upstream changed from N/A to Reported upstream. Little or no feedback.

Replying to pjeremy:

6456-freebsd-spkg-install.patch adds support for FreeBSD (this is a port of the patch in #9601). I have compile-tested this but not yet tried to use the resultant module. Note that further changes are necessary for cvxopt-1.1.2.p1.spkg to work on most 64-bit OSs. Compiling in FreeBSD/amd64 (with or without the above patch) gives: {{{ gcc -fno-strict-aliasing -g -O2 -DNDEBUG -g -O3 -Wall -Wstrict-prototypes -I/mnt/sage-4.5/local/include -fPIC -DDLONG= -I/mnt/sage-4.5/local/include/python2.6 -c C/misc_solvers.c -o build/temp.freebsd-8.1-PRERELEASE-amd64-2.6/C/misc_solvers.o C/misc_solvers.c: In function 'scale': C/misc_solvers.c:152:13: warning: passing argument 1 of 'dscal_' from incompatible pointer type C/misc_solvers.c:32:13: note: expected 'int *' but argument is of type 'Py_ssize_t *' C/misc_solvers.c:152:13: warning: passing argument 4 of 'dscal_' from incompatible pointer type C/misc_solvers.c:32:13: note: expected 'int *' but argument is of type 'Py_ssize_t *' C/misc_solvers.c:155:13: warning: passing argument 3 of 'dgemv_' from incompatible pointer type C/misc_solvers.c:39:13: note: expected 'int *' but argument is of type 'Py_ssize_t *' C/misc_solvers.c:156:9: warning: passing argument 1 of 'dscal_' from incompatible pointer type C/misc_solvers.c:32:13: note: expected 'int *' but argument is of type 'Py_ssize_t *' C/misc_solvers.c:156:9: warning: passing argument 4 of 'dscal_' from incompatible pointer type C/misc_solvers.c:32:13: note: expected 'int *' but argument is of type 'Py_ssize_t *' C/misc_solvers.c:158:13: warning: passing argument 2 of 'dger_' from incompatible pointer type C/misc_solvers.c:41:13: note: expected 'int *' but argument is of type 'Py_ssize_t *' C/misc_solvers.c:160:13: warning: passing argument 1 of 'dscal_' from incompatible pointer type C/misc_solvers.c:32:13: note: expected 'int *' but argument is of type 'Py_ssize_t *' C/misc_solvers.c:160:13: warning: passing argument 4 of 'dscal_' from incompatible pointer type C/misc_solvers.c:32:13: note: expected 'int *' but argument is of type 'Py_ssize_t *' C/misc_solvers.c: In function 'pack2': C/misc_solvers.c:459:17: warning: passing argument 3 of 'dlacpy_' from incompatible pointer type C/misc_solvers.c:49:13: note: expected 'int *' but argument is of type 'Py_ssize_t *' C/misc_solvers.c:459:17: warning: passing argument 5 of 'dlacpy_' from incompatible pointer type C/misc_solvers.c:49:13: note: expected 'int *' but argument is of type 'Py_ssize_t *' C/misc_solvers.c:461:17: warning: passing argument 1 of 'dscal_' from incompatible pointer type C/misc_solvers.c:32:13: note: expected 'int *' but argument is of type 'Py_ssize_t *' C/misc_solvers.c:463:17: warning: passing argument 3 of 'dlacpy_' from incompatible pointer type C/misc_solvers.c:49:13: note: expected 'int *' but argument is of type 'Py_ssize_t *' C/misc_solvers.c:463:17: warning: passing argument 7 of 'dlacpy_' from incompatible pointer type C/misc_solvers.c:49:13: note: expected 'int *' but argument is of type 'Py_ssize_t *' }}}

the following 2-byte change appears to cure the problem. Semantically, this restricts the dense matrices that are dealt with to the sizes 231 by 231, but this is OK for all the practical purposes. Hopefully CVXOPT people will come up with a better fix, but for the time being this should suffice and cure this particular headache.

--- a/patches/cvxopt.h  Mon Jul 26 18:45:42 2010 +0300
+++ b/patches/cvxopt.h  Thu Jul 29 09:12:21 2010 -0700
@@ -61,7 +61,7 @@
 typedef struct {
   PyObject_HEAD
   void *buffer;          /* in column-major-mode array of type 'id' */
-  int_t nrows, ncols;    /* number of rows and columns */
+  int nrows, ncols;    /* number of rows and columns -- was int_t */
   int   id;              /* DOUBLE, INT, COMPLEX */
 } matrix;

Py_ssize_t is typedef'd from ssize_t, which is long on at least FreeBSD, Linux and Solaris. I believe this is a blocking issue.

It remains to sort out the complex.h stuff on Solaris...

  Changed 5 weeks ago by dimpase

  • upstream changed from Reported upstream. Little or no feedback. to Reported upstream. Developers acknowledge bug.

in reply to: ↑ 80 ; follow-ups: ↓ 83 ↓ 84   Changed 5 weeks ago by dimpase

  • upstream changed from Reported upstream. Developers acknowledge bug. to Completely fixed; Fix reported upstream

Replying to dimpase:

Replying to pjeremy:

[...]

the following 2-byte change appears to cure the problem. Semantically, this restricts the dense matrices that are dealt with to the sizes 231 by 231, but this is OK for all the practical purposes. Hopefully CVXOPT people will come up with a better fix, but for the time being this should suffice and cure this particular headache.

 --- a/patches/cvxopt.h  Mon Jul 26 18:45:42 2010 +0300
 +++ b/patches/cvxopt.h  Thu Jul 29 09:12:21 2010 -0700
 @@ -61,7 +61,7 @@
  typedef struct {
    PyObject_HEAD
    void *buffer;          /* in column-major-mode array of type 'id' */
 -  int_t nrows, ncols;    /* number of rows and columns */
 +  int nrows, ncols;    /* number of rows and columns -- was int_t */
    int   id;              /* DOUBLE, INT, COMPLEX */
  } matrix;

One of cvxopt developers has acknowledged this as a valid fix. Further, he says that it will get into a new cvxopt release, 1.1.3, due shortly.

I am inclined to wait for 1.1.3, while preparing an spkg-check starting off from Dave's version, and eventually sorting out OSX and Cygwin. (and eventually hooking up the cvxopt's documentation to Sage's documentation)

Further, the Sun complex.h related issue, I looked at the /usr/include/complex.h over on t2 and mark. It just does not make sense how _Imaginary_I is (not) defined there, something like

#define _Imaginary_I _Imaginary_I

---no wonder it does not work. While it does make sense in the supplied patches/sun_complex.h, which differs from the former at this and few other places.

Dima

in reply to: ↑ 82   Changed 5 weeks ago by drkirkby

Replying to dimpase:

Replying to dimpase:

Replying to pjeremy:

[...] One of cvxopt developers has acknowledged this as a valid fix. Further, he says that it will get into a new cvxopt release, 1.1.3, due shortly.

Good.

I am inclined to wait for 1.1.3, while preparing an spkg-check starting off from Dave's version, and eventually sorting out OSX and Cygwin. (and eventually hooking up the cvxopt's documentation to Sage's documentation)

I think having those self-tests will be very useful. It might need a change of method, to change to a directory before running them, rather than run them from a higher level directory as I did.

Further, the Sun complex.h related issue,

Don't waste any time on that. I've spent most of the day looking at this, and will summarise my findings later.

in reply to: ↑ 82 ; follow-up: ↓ 91   Changed 5 weeks ago by pjeremy

Replying to dimpase:

Replying to dimpase:

the following 2-byte change appears to cure the problem. Semantically, this

...

One of cvxopt developers has acknowledged this as a valid fix. Further, he says that it will get into a new cvxopt release, 1.1.3, due shortly.

That's good. I'm sorry that Real Life intervened and I wasn't able to complete the investigation of this problem myself. I've checked and it gets rid of the warnings on FreeBSD as well. That leaves only just over 3000 warnings in a Sage build that need investigating.

I am inclined to wait for 1.1.3, while preparing an spkg-check starting off from Dave's version, and eventually sorting out OSX and Cygwin.

Depending on the cvxopt project's definition of "shortly", that sounds reasonable. I would appreciate the new SPKG including my fix to support FreeBSD (and something similar may be needed to support Cygwin)

  Changed 5 weeks ago by drkirkby

I set about trying to resolve why cvxopt would build on some Solaris systems but not on others. I believe I have finally got to the bottom of this.

If one looks in the current cvxopt SPKG.txt file, there is a reference to this bug, and why the sun_complex.h was added.

 http://bugs.opensolaris.org/bugdatabase/view_bug.do?bug_id=6549313

which does not appear to have any activity for more than 3 years. There's a little test program there, which I modified a bit to print good or bad:

#include <stdio.h>
#include <complex.h>

/*
 * "volatile" is meant to prevent gcc from calculating the sqrt as a
 * constant, we want to test libc.
 */
volatile complex double z = -_Complex_I;
int
main(void)
{
        z = csqrt(z);
        if (creal(z) > 0.0)
                printf("good\n");       /* good */
        else
                printf("bad\n");       /* bad */
}

After saving that to a file test.c I then tried to compile this using

gcc -lm -std=c99 test.c  # Or in some cases using the Sun compiler.

in each case noting if the file compiled, or whether it gave an error like:

$ gcc -lm -std=c99 test.c
test.c:8: error: '_Complex_I' undeclared here (not in a function)

Assuming it did compile (which was the minority of cases), if that test program printed 'good' or 'bad' when it was run.

Comparing different 12 systems I have access to, sorted in order of the release date of the operating system, this is what I found:

ComputerCPUhostnameOSRelease of OSCompilerCompileResult
Sun Blade 2000SPARCswanSolaris 1010/2009gcc 4.5.0YesGood
Sun Blade 2000SPARCswanSolaris 1010/2009SunStudio 12.1YesBad
Sun Blade 2000SPARCswanSolaris 1010/2009gcc 4.4.4No-
Sun Blade 2000SPARCswanSolaris 1010/2009gcc 3.4.3No-
unknown x86orcusSolaris 1010/2009gcc 4.3.4No-
unknown x86orcusSolaris 1010/2009gcc 3.4.3No-
Sun Ultra 27x86hawkOpenSolaris06/2009SunStudio 12.1YesGood
Sun Ultra 27x86hawkOpenSolaris06/2009gcc 4.5.0YesGood
Sun Ultra 27x86hawkOpenSolaris06/2009gcc 4.4.4No-
Sun Ultra 27x86hawkOpenSolaris06/2009gcc 3.4.3No-
Sun Blade 2500SPARCmercurySolaris 1005/2009gcc 4.3.4No-
Sun Blade 2500SPARCmercurySolaris 1005/2009gcc 3.4.5No-
Sun Blade 2500SPARCmercurySolaris 1005/2009gcc 3.4.3No-
Sun T5240SPARCt2Solaris 1005/2009gcc 4.4.1No-
Sun T5240SPARCt2Solaris 1005/2009gcc 3.4.3No-
Sun Fire X4540x86diskOpenSolaris11/2008gcc 3.4.3No-
Dell OptiPlex 755x86fulviaSolaris 1005/2008gcc 4.5.0YesGood
Dell OptiPlex 755x86fulviaSolaris 1005/2008gcc 3.4.3No-
Sun Blade 2500SPARCmarkSolaris 1001/2006gcc 4.5.0YesGood
Sun Blade 2500SPARCmarkSolaris 1001/2006gcc 3.4.3No-
Sun Blade 2500SPARCmarkSolaris 1001/2006SunStudio 12YesBad
Sun Blade 1000SPARCredstartSolaris 1003/20054.5.0YesGood
Sun Blade 1000SPARCredstartSolaris 1003/20054.4.3No-
Sun Blade 1000SPARCredstartSolaris 1003/20053.4.3No-
Sun Netra T1SPARCkestrelSolaris 1003/20054.4.2No-
Sun Netra T1SPARCkestrelSolaris 1003/20053.4.3No-
Sun Fire 480RSPARCraSolaris 802/2004gcc 4.3.4No-

Notes:

  • swan , redstart , hawk and kestrel are my own personal machines.
  • fulvia, mark and mark2 are hosts on Skynet.
  • disk and t2 are hosts on the *.math.washington.edu network.
  • ra and mercury are hosts on blastwave.org. (Dennis Clark of Blastwave has given me access to their network, which is useful, as their Sun Blade 2500 has faster CPUs than the Sun Blade 2500 on Skynet).
  • orcus is a Solaris zone running on some x86 hardware on the blastwave.org network. I don't know what that hardware is - since it's in a Solaris zone, this is hidden.
  • ra on the blastwave.org network runs a version of Solaris older than what Sage aims to support. IMHO, we should aim to support all Solaris 10 releases, the first of which was released in March 2005, but not bother with older releases, where there are C99 related issues.

Looking at those results, some patterns can be seen.

  • Every time gcc 4.5.0 was used, test.c compiled and run OK. (gcc 4.5.0 was available on all the skynet machines and I built it on all my own machines. I did not bother building it on t2.math, disk.math or any of the machines on Blastwave.)
  • If an older version of gcc was used, test.c would never compile.
  • Whether the system had SPARC or x86 processors did change the behavior.
  • The age of the operating system did not matter. (Although I have some doubts if this would have worked with gcc 4.5 on the old Solaris 8 machine, but as noted above, this is too old to bother supporting.)

I then realised that this is the same bug that hit us with the Sage library before (#7932). I reported that to the gcc bug database as two bugs - one for SPARC, one for x86. The GCC developers then closed one as a duplicate of the other.

If you read  gcc bug 42753, it basically boils down to the fact that the gcc developers claim it's a bug in the Sun header file, but that their fixincludes is lacking the facility to deal with this. However, their fixincludes was updated in gcc 4.5, so this problem was fixed in gcc 4.5.

So I suspect we have two options.

  • Force people to use gcc 4.5 on Solaris, then remove the sun_complex.h hack completely. That would be rather annoying, since that's the latest version of gcc, and I'm not aware of anywhere where one can download a pre-compiled gcc 4.5 binary. People would have to build it themselves, which is a non-trivial process on Solaris.
  • We apply that patch on gcc versions older than 4.5, but omit the patch with gcc version 4.5 or later. There's an example of how to do this  here using the gcc macros __GNUC__, __GNUC_MINOR__ and __GNUC_PATCHLEVEL__. It's easy to see what macros gcc defines, by creating an empty file foobar.c, and using gcc -c -E -dM foobar.c

So in summary, I think the solution to the Solaris problem is that the code in cvxopt needs changing so that the patched header only gets included when building with gcc less than 4.5.0. I suspect the easiest solution is to always apply a patched header file, but arrange for the patch to just include <complex.h> on gcc 4.5.0 or later, but have its current behavior on earlier gcc series.

Although I've not yet modified sun_complex.h to have this behavior, I suspect it will allow the code to compile on any gcc >= 4.0.1, which is the earliest Sage supports.

Dave

  Changed 5 weeks ago by drkirkby

I just noticed the current SPKG.txt file has a couple of issues, which means it does not meet the guidelines of the Sage Developers Guide

  • There is no == License == section. I just looked at the  Copyright and license section on the cvxopt web site, and see there are multiple licenses (GPL2, GPL3, GNU Lesser General Public License, version 2.1), but all are compatible with Sage.
  • The section marked == Download Source == in SPKG.txt should be marked == Upstream Contact ==
  • There's no == Dependencies == section.
  • There's no == Special Update/Build Instructions == section.
  • The == Releases == section should be renamed to == Changelog ==

These are all trivial changes, which will only take a few minutes to implement.

Dave

follow-up: ↓ 88   Changed 5 weeks ago by drkirkby

This problem with cvxopt on Solaris has become critical now, as there are no longer any compilers except gcc 4.5.0 on any of the Solaris machines.

I've created a very small patch to fix the current version of cvxopt so it builds on Solaris. The #9657. I'd like to get that merged asap, as it is really a pain now we can't build on skynet,

Dave

in reply to: ↑ 87 ; follow-up: ↓ 89   Changed 5 weeks ago by drkirkby

Replying to drkirkby:

This problem with cvxopt on Solaris has become critical now, as there are no longer any compilers except gcc 4.5.0 on any of the Solaris machines.

I meant to add there are no compilers able to build Sage on Solaris on and of the Skynet machines.

I've created a very small patch to fix the current version of cvxopt so it builds on Solaris.

We should get that patch upstream if possible. As you can see, it is very trivial. All we need to add to the upstream source is something like this.

#include <complex.h>
#if defined(__sun__) && defined(__GNUC__)
   #if __GNUC__ < 4  || ( __GNUC__ == 4 && __GNUC_MINOR__ < 5   )

      #undef  _Complex_I
      #define _Complex_I (__extension__ 1.0iF)

      #undef I
      #define I _Complex_I

   #endif
#endif

There's no need to have a huge great header file as there is at the minute. It is totally unnecessary. One two items are missing from the header file, so only two need to be added. The patch currently add a very large proprietary header file when only a few lines of code are needed. }}}

in reply to: ↑ 88 ; follow-up: ↓ 90   Changed 5 weeks ago by dimpase

Replying to drkirkby:

Replying to drkirkby:

This problem with cvxopt on Solaris has become critical now, as there are no longer any compilers except gcc 4.5.0 on any of the Solaris machines.

I meant to add there are no compilers able to build Sage on Solaris on and of the Skynet machines.

I've created a very small patch to fix the current version of cvxopt so it builds on Solaris.

We should get that patch upstream if possible. As you can see, it is very trivial. All we need to add to the upstream source is something like this.

 #include <complex.h>
 #if defined(__sun__) && defined(__GNUC__)
    #if __GNUC__ < 4  || ( __GNUC__ == 4 && __GNUC_MINOR__ < 5   )
 
       #undef  _Complex_I
       #define _Complex_I (__extension__ 1.0iF)
 
       #undef I
       #define I _Complex_I
 
    #endif
 #endif

Why do you check for gcc here? I imagine with other compilers the problem would be the same, too. (for the sake of reporting upstream...) Is SunStudio? 12 providing its own fix for this?

Otherwise I agree that this looks more pleasant than the current sun_complex.h hack; I'll put it in the updated cvxopt 1.1.2 now.

Dima

There's no need to have a huge great header file as there is at the minute. It is totally unnecessary. One two items are missing from the header file, so only two need to be added. The patch currently add a very large proprietary header file when only a few lines of code are needed. }}}

in reply to: ↑ 89   Changed 5 weeks ago by drkirkby

Replying to dimpase:

Why do you check for gcc here? I imagine with other compilers the problem would be the same, too. (for the sake of reporting upstream...) Is "SunStudio? 12 providing its own fix for this?

The fix is not needed with the SunStudio compiler. I can compile a test program that uses _Complex_I with the Sun compiler without resorting to any hacks - see the big table above I produced.  http://trac.sagemath.org/sage_trac/ticket/6456#comment:85

In any case, I've not done much testing with SunStudio on this, so it would be unwise to start applying patches without testing them.

Otherwise I agree that this looks more pleasant than the current sun_complex.h hack; I'll put it in the updated cvxopt 1.1.2 now. Dima

Yes, I think so too. gcc 4.5.0 creates a file with the fixed header, which I inspected. That is huge, looking similar to that the current patch in Sage. There's no need to repeat them all. And as I noted before, it is doubtful if its 100% legal to do so. All I did was copied the necessary parts.

I did email the cvxopt developers about this, so hopefully it can be fixed upstream. You might not actually need the patch at all when 1.13 is released.

Dave

in reply to: ↑ 84 ; follow-up: ↓ 92   Changed 5 weeks ago by dimpase

Replying to pjeremy:

Replying to dimpase:

Replying to dimpase:

the following 2-byte change appears to cure the problem. Semantically, this

...

One of cvxopt developers has acknowledged this as a valid fix. Further, he says that it will get into a new cvxopt release, 1.1.3, due shortly.

That's good. I'm sorry that Real Life intervened and I wasn't able to complete the investigation of this problem myself. I've checked and it gets rid of the warnings on FreeBSD as well. That leaves only just over 3000 warnings in a Sage build that need investigating.

I am inclined to wait for 1.1.3, while preparing an spkg-check starting off from Dave's version, and eventually sorting out OSX and Cygwin.

Depending on the cvxopt project's definition of "shortly", that sounds reasonable. I would appreciate the new SPKG including my fix to support FreeBSD (and something similar may be needed to support Cygwin)

New spkg is here:

 http://boxen.math.washington.edu/home/dima/packages/cvxopt-1.1.2.spkg

here is the list of goodies that got added/updated/fixed:

  • applied P.Jeremy's FreeBSD patch
  • corrected the 64-bit specific int* bug reported by pjeremy
  • turned on GSL extension (this is the default mode for CVXOPT, and GSL is a standard Sage spkg, so this makes perfect sense); this in particular allowed to get rid of strange random seed-related import bugs uncovered by David Kirkby's spkg-check TODO(?): one might want to enchance the code to allow other Sage random sources,

at the moment only GSL is used in CVXOPT-1.1.2 spkg, apparently it will need an unclear to me "with seed(..)" construct.

TODO: We will need to make sure that CVXOPT is built after GSL

  • modified spkg-check to report test names, cd to appropriate subdirs, and skip .bin files. TODO: add more tests.
  • corrected the .patch files in patches/ to be in right order --- just run the makepatchfiles script to re-create these files!
  • removed html doc files in src/doc; the .rst doc files are there, so it's a question of rebuilding them (e.g. one can do sage -sh; cd src/doc; make html) TODO: incorporate docs buiding into spkg-install, and/or merge into Sage documentation
  • included David's shortened and gcc-version targeted Sun-specific patch (comment 88); removed sun_complex.h
  • took care of SPKG.txt sections, as mentioned in comment 87

MAJOR change: it now depends on GSL--- this gets rid of sage.prandom related problems

MAJOR TODOs: MacOSX fixes, and testing with gcc-4.5 on as many platforms as possible (otherwise it looks good to go !)

Dima

in reply to: ↑ 91 ; follow-up: ↓ 93   Changed 5 weeks ago by dimpase

Replying to dimpase: forgot to add that I tested (also spkg-check) on Linux 32bit and 64bit (sage.math), as well as on Sparc Solaris (t2.math) with gcc 4.x, with x<=4.

Would appreciate hearing how it does with gcc 4.5 (Linux and Solaris)

in reply to: ↑ 92   Changed 5 weeks ago by drkirkby

Replying to dimpase:

Would appreciate hearing how it does with gcc 4.5 (Linux and Solaris)

I've been to the pub tonight and have several beers. I think it's best if I don't start building anything now!

But I'll look at that when I have sobered up a bit.

Be aware, there is a ticket to update gsl #9533. However, IIRC, an inspection of the GSL ChangeLog? shows that all the changes were related to way GSL is built, and nothing to do with actual changes in the algorithms. But please check that! As I said, I have had a few (well a few too many) beers tonight!

Dave

Note: See TracTickets for help on using tickets.