-
-
Notifications
You must be signed in to change notification settings - Fork 567
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Upgrade cvxopt to 1.1.6 #12832
Comments
comment:1
I think they may be both upstream problems. I have tried to run test on a stock scipy-0.10.0 on power7 and several results indicate that at least some of the problems are in scipy itself
There are more error linked to failures to import modules which may be more related to the way test were run. As in the doctest a number of improper parameters or invalid data and negative dimensions problems. |
comment:2
I investigated the first error in
The first numerical difference comes from the call
and the expected output is:
whereas we get:
Does anybody know where the Paul |
comment:3
I could reduce the issue to the following:
The expected output is twice the same matrix F:
On silius with sage-5.0.rc1 I get instead:
thus it seems the product is accumulated to F. Paul |
comment:4
That's unexpected, It could be a cvxopt problem rather than a scipy problem. I may be able to check on Monday when I go back to work for real if there are more problems with cvxopt. |
comment:5
Yes I can confirm that with 5.1beta5 (and cvxopt-1.1.5 from #13160). Will look at the code. |
comment:6
If after that we do (http://abel.ee.ucla.edu/cvxopt/userguide/blas.html#level-3-blas)
we get
Clearly the beta factor is always passed as "1" rather than the default zero or the value you want. I checked that passing a value for alpha works. |
comment:7
I am not completely sure of what is happening there. I need to have a control build locally to be sure. The evaluation eventually goes to sp_dgemm in C/sparse.c, more specifically this section
Notice that in this particular case "partial" is ignored. The issue here is that we don't seem to go through scal[DOUBLE] (part of C/blas.c) at all. That particular call should replace our F input by beta.d*F where beta.d is 0 by default. The fact that this call seem completely bypassed explains why it is useless to give a value for beta. |
comment:8
I cannot see it going in scal[DOUBLE] on my x86_64 box either. I will have to dig it a bit more to find how to crack what is happening there. |
comment:9
cvxopt.base is not linked to cvxopt.blas which contains the interface to scal (it is working fine). So in the previous code this should be a direct call to dscal. I am guessing there is something about the declarations in that bit of code that I don't understand fully. But then why does it work on anything but power7? |
comment:10
Finally found out it is supposed to be a direct call to blas dscal_ but the declaration are all wrong for that I think. I may have to copy the declaration from base.c and see if it works. |
comment:11
I have no clue why the call to dscal_ either doesn't happen or return the wrong thing. |
comment:12
maybe this is due to the fact that the power7 is big endian, and some code is not endian-safe? Paul |
comment:13
Replying to @zimmermann6:
No. We have some other big endian platforms: OS X ppc, arm and sparc. It would have shown up somewhere before. Francois |
This comment has been minimized.
This comment has been minimized.
comment:15
Building cvxopt with SAGE_CHECK=yes on OS X
|
comment:16
And on power7:
|
comment:17
OK from there we get that
That's an interesting starting point. |
Upstream: Reported upstream. No feedback yet. |
comment:18
I am pinging upstream to see if they have a clue on where things go wrong. |
comment:19
Very similar results if I use openblas instead of atlas. There are some slight differences after 100 iterations - possibly a test of iterative precision of a sort. But still fails to converge. |
comment:20
I am currently in contact to figure out where things go wrong. I doesn't quite fit any of the given options. |
comment:21
I have an unstable release to test to see if it fixes the problem. |
Changed upstream from Reported upstream. No feedback yet. to Reported upstream. Developers acknowledge bug. |
comment:22
Francois, who is "upstream" here? Scipy? Paul |
comment:23
No cvxopt: http://abel.ee.ucla.edu/cvxopt/ in particular Lieven Vandenberghe has been in contact with me regularly to find the problem. |
This comment has been minimized.
This comment has been minimized.
comment:29
I'm including the fix from #10508. |
This comment has been minimized.
This comment has been minimized.
spkg diff |
comment:32
Attachment: cvxopt-1.1.6.p0.diff.gz I'll review that ticket on silius. Now downloading Sage 5.9 source... Paul |
comment:33
Isn't that silly, building stuff manually so we don't have to trouble the buildbot? I thought skynet is there to serve us, not the other way 'round ;-) |
comment:34
Volker, what is wrong in testing a patch with the latest Sage release? Paul |
comment:35
Does spkg-check works? I thought it would need work when I checked the rc sent to me by upstream. |
comment:36
Also from my point of view it works on power7, it needs checking against #10508 and other platforms. |
comment:37
Replying to @kiwifb:
I works for me, I tested it on silius and also on OS X 10.6. |
comment:38
OK there may have been special testing code in the rc I was sent. spkg-check definitely didn't work with it. I had to run various tests manually after install. |
comment:39
Works with #10508 and fixes the power7 issue. So I guess its good to go. |
Reviewer: Volker Braun |
Merged: sage-5.10.beta4 |
comment:41
As reported on sage-release by Steven Trogdon, this fails to build if |
comment:42
P.S.: Some more environment variables ( if [ "x$SAGE_SPKG_INSTALL_DOCS" = xyes ] ; then
cd doc
# This part would be used to build the documentation with sphinx.
# cvxopt would then have to depend on sphinx.
# in 1.1.5 the documentation is shipped already built and up to date.
# ${MAKE} -B html
# if [ $? -ne 0 ]; then
# echo "Error building the documentation"
# exit 1
# fi
# checking to see if there is previously installed documentation.
if [ -d $SAGE_LOCAL/share/doc/cvxopt/html ] ; then
rm -rf $SAGE_LOCAL/share/doc/cvxopt/html
fi
mkdir -p $SAGE_LOCAL/share/doc/cvxopt/html
cp -r build/html/* $SAGE_LOCAL/share/doc/cvxopt/html/
fi And some (more) error messages should get redirected to |
comment:43
P.P.S.: Perhaps also And add error checks w.r.t. doc installation. |
comment:44
Follow-up is #14645, new spkg on the way... |
There are two doctests related to cvxopt failing on power7
and
This is now solved in cvxopt 1.1.6. The updated cvxopt spkg also fixes a blas path issue on Darwin and Cygwin.
spkg: http://boxen.math.washington.edu/home/jdemeyer/spkg/cvxopt-1.1.6.p0.spkg (spkg diff)
Upstream: Fixed upstream, in a later stable release.
Component: packages: standard
Author: Jeroen Demeyer
Reviewer: Volker Braun
Merged: sage-5.10.beta4
Issue created by migration from https://trac.sagemath.org/ticket/12832
The text was updated successfully, but these errors were encountered: