Opened 2 years ago
Closed 2 years ago
#28730 closed defect (fixed)
cvxopt fails its test suite
Reported by:  jhpalmieri  Owned by:  

Priority:  critical  Milestone:  sage9.0 
Component:  packages: standard  Keywords:  
Cc:  Merged in:  
Authors:  John Palmieri  Reviewers:  François Bissey 
Report Upstream:  N/A  Work issues:  
Branch:  cee0cab (Commits, GitHub, GitLab)  Commit:  cee0cabba5b80f811cb8db925e9f02f047a53414 
Dependencies:  Stopgaps: 
Description (last modified by )
With a Python 3 build of Sage, ./sage f c cvxopt
fails.
See also #24657.
Change History (21)
comment:1 Changed 2 years ago by
comment:2 Changed 2 years ago by
Thank you for pointing out that ticket. It looks different to me, though: I see
Optimal solution found. Traceback (most recent call last): File "/Users/jpalmier/Desktop/Sage/sage_builds/TESTING/sage9.0.beta5/local/lib/python3.7/sitepackages/numpy/lib/histograms.py", line 410, in _get_bin_edges n_equal_bins = operator.index(bins) TypeError: 'float' object cannot be interpreted as an integer During handling of the above exception, another exception occurred: Traceback (most recent call last): File "normappr.py", line 27, in <module> pylab.hist(A*x1.value + b, m/5) File "/Users/jpalmier/Desktop/Sage/sage_builds/TESTING/sage9.0.beta5/local/lib/python3.7/sitepackages/matplotlib/pyplot.py", line 3137, in hist stacked=stacked, normed=normed, data=data, **kwargs) File "/Users/jpalmier/Desktop/Sage/sage_builds/TESTING/sage9.0.beta5/local/lib/python3.7/sitepackages/matplotlib/__init__.py", line 1867, in inner return func(ax, *args, **kwargs) File "/Users/jpalmier/Desktop/Sage/sage_builds/TESTING/sage9.0.beta5/local/lib/python3.7/sitepackages/matplotlib/axes/_axes.py", line 6639, in hist m, bins = np.histogram(x[i], bins, weights=w[i], **hist_kwargs) File "/Users/jpalmier/Desktop/Sage/sage_builds/TESTING/sage9.0.beta5/local/lib/python3.7/sitepackages/numpy/lib/histograms.py", line 780, in histogram bin_edges, uniform_bins = _get_bin_edges(a, bins, range, weights) File "/Users/jpalmier/Desktop/Sage/sage_builds/TESTING/sage9.0.beta5/local/lib/python3.7/sitepackages/numpy/lib/histograms.py", line 413, in _get_bin_edges '`bins` must be an integer, a string, or an array') TypeError: `bins` must be an integer, a string, or an array Error: test /Users/jpalmier/Desktop/Sage/sage_builds/TESTING/sage9.0.beta5/local/var/tmp/sage/build/cvxopt1.1.8.p2/src/examples/doc/chap10/normappr.py failed
comment:3 Changed 2 years ago by
 Description modified (diff)
comment:4 followup: ↓ 5 Changed 2 years ago by
I see a change from numpy ultimately. Which file is this from? I'll see if it still happens with cvxopt1.2.3 which is what I am using in sageongentoo. In fact now that I have shipped suitesparse we should be able to update cvxopt, I guess I should have taken a lead on that.
comment:5 in reply to: ↑ 4 Changed 2 years ago by
Replying to fbissey:
I see a change from numpy ultimately. Which file is this from?
Not sure what you mean. The error message is from the cvxopt log file, and it refers to normappr.py
, or in full detail, SAGE_ROOT/local/var/tmp/sage/build/cvxopt1.1.8.p2/src/examples/doc/chap10/normappr.py
comment:6 Changed 2 years ago by
I see, I should have spotted it. Now I see that this is the same file file but a different problem than in #24657. Upgrading cvxopt won't fix the fact that you try to run (outdated) documentation as tests.
Note like I said in #24657 cvxopt' spkgcheck is not running a test suite, it is running documentation examples as if it was a test suite. And then shock horror it is not foolproofed like one. I think we should ditch spkgcheck in cvxopt unless we are to run the real test suite with pytest
.
comment:7 Changed 2 years ago by
That makes sense to me. Unfortunately, I don't think we have a way of including an optional package as a dependency for testing, so should we just remove the cvxopt spkgcheck
file? Then later we can consider adding pytest
as a standard package.
comment:8 Changed 2 years ago by
That would be the sensible thing. At least in my opinion. The other testing dependency I can think of is nose
and it looks like it has been made a standard package.
comment:9 Changed 2 years ago by
Actually, looking at the version of cvxopt
which is in Sage, its INSTALL
file says
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.py
I just downloaded version 1.2.3, and its INSTALL
file says
To test that the installation was successful, run the included tests using python m unittest discover s tests or alternatively, if `nose` is installed, nosetests
So we could update to 1.2.3 and use nose
.
comment:10 Changed 2 years ago by
I guess we can try that. Personally I didn't look at INSTALL
when writing the tests on sageongentoo, I looked at the .travis.yml
file and followed what they are doing in there. There is a chance that INSTALL
is not up to date but we should give it a go.
comment:11 Changed 2 years ago by
I think our current broken spkgcheck
is already doing what they suggest.
I tried upgrading to 1.2.3, but there are too many dependencies for me to deal with right now: SuiteSparse which in turn depends on cmake
. I don't want to convert cmake
to be a standard Sage package. Maybe we can just build part of SuiteSparse without using cmake
.
comment:12 Changed 2 years ago by
I thought part of #22380 had gone in but it was blocked by cygwin :( If suitesparse now depends on cmake that means there is a new release. I should at least look into that.
comment:13 Changed 2 years ago by
#22380 is using suitesparse 5.4.0, upstream is now at 5.6.0 but it should all work as in that ticket. cmake
is only needed for some stuff we don't use. Unless there are new dependencies between packages that I haven't spotted.
comment:14 Changed 2 years ago by
I agree, we don't need cmake
for the crucial parts of SuiteSparse
.
comment:15 Changed 2 years ago by
Now I see I made a patch to avoid calls to cmake. We should finish that ticket.
comment:16 followup: ↓ 17 Changed 2 years ago by
So should we close this ticket, or should we use this to remove spkgcheck
? In either case, #22380 can instate a better version.
comment:17 in reply to: ↑ 16 Changed 2 years ago by
Replying to jhpalmieri:
So should we close this ticket, or should we use this to remove
spkgcheck
? In either case, #22380 can instate a better version.
We should remove it here. Fixing the cygwin problem in #22380 may take a bit of time. I have an alternative packaging option for suitesparse, it just makes me the maintainer for both gentoo and sage until it gets a better build system [which is likely to be cmake].
comment:18 Changed 2 years ago by
 Branch set to u/jhpalmieri/cvxoptnospkgcheck
comment:19 Changed 2 years ago by
 Commit set to cee0cabba5b80f811cb8db925e9f02f047a53414
 Status changed from new to needs_review
New commits:
cee0cab  trac 28730: testsuite for cvxopt is broken, so remove spkgcheck.

comment:20 Changed 2 years ago by
 Reviewers set to François Bissey
 Status changed from needs_review to positive_review
LGTM
comment:21 Changed 2 years ago by
 Branch changed from u/jhpalmieri/cvxoptnospkgcheck to cee0cabba5b80f811cb8db925e9f02f047a53414
 Resolution set to fixed
 Status changed from positive_review to closed
Is it related to #24657? Note my comment at the end of the ticket.