Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cvxopt fails its test suite #28730

Closed
jhpalmieri opened this issue Nov 13, 2019 · 24 comments
Closed

cvxopt fails its test suite #28730

jhpalmieri opened this issue Nov 13, 2019 · 24 comments

Comments

@jhpalmieri
Copy link
Member

With a Python 3 build of Sage, ./sage -f -c cvxopt fails.

See also #24657.

Component: packages: standard

Author: John Palmieri

Branch/Commit: cee0cab

Reviewer: François Bissey

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

@kiwifb
Copy link
Member

kiwifb commented Nov 13, 2019

comment:1

Is it related to #24657? Note my comment at the end of the ticket.

@jhpalmieri
Copy link
Member Author

comment:2

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/sage-9.0.beta5/local/lib/python3.7/site-packages/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/sage-9.0.beta5/local/lib/python3.7/site-packages/matplotlib/pyplot.py", line 3137, in hist
    stacked=stacked, normed=normed, data=data, **kwargs)
  File "/Users/jpalmier/Desktop/Sage/sage_builds/TESTING/sage-9.0.beta5/local/lib/python3.7/site-packages/matplotlib/__init__.py", line 1867, in inner
    return func(ax, *args, **kwargs)
  File "/Users/jpalmier/Desktop/Sage/sage_builds/TESTING/sage-9.0.beta5/local/lib/python3.7/site-packages/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/sage-9.0.beta5/local/lib/python3.7/site-packages/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/sage-9.0.beta5/local/lib/python3.7/site-packages/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/sage-9.0.beta5/local/var/tmp/sage/build/cvxopt-1.1.8.p2/src/examples/doc/chap10/normappr.py failed

@jhpalmieri

This comment has been minimized.

@kiwifb
Copy link
Member

kiwifb commented Nov 13, 2019

comment:4

I see a change from numpy ultimately. Which file is this from? I'll see if it still happens with cvxopt-1.2.3 which is what I am using in sage-on-gentoo. 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.

@jhpalmieri
Copy link
Member Author

comment:5

Replying to @kiwifb:

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/cvxopt-1.1.8.p2/src/examples/doc/chap10/normappr.py

@kiwifb
Copy link
Member

kiwifb commented Nov 13, 2019

comment:6

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' spkg-check 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 spkg-check in cvxopt unless we are to run the real test suite with pytest.

@jhpalmieri
Copy link
Member Author

comment:7

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 spkg-check file? Then later we can consider adding pytest as a standard package.

@kiwifb
Copy link
Member

kiwifb commented Nov 13, 2019

comment:8

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.

@jhpalmieri
Copy link
Member Author

comment:9

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.

@kiwifb
Copy link
Member

kiwifb commented Nov 13, 2019

comment:10

I guess we can try that. Personally I didn't look at INSTALL when writing the tests on sage-on-gentoo, 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.

@jhpalmieri
Copy link
Member Author

comment:11

I think our current broken spkg-check 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.

@kiwifb
Copy link
Member

kiwifb commented Nov 13, 2019

comment:12

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.

@kiwifb
Copy link
Member

kiwifb commented Nov 13, 2019

comment:13

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

@jhpalmieri
Copy link
Member Author

comment:14

I agree, we don't need cmake for the crucial parts of SuiteSparse.

@kiwifb
Copy link
Member

kiwifb commented Nov 13, 2019

comment:15

Now I see I made a patch to avoid calls to cmake. We should finish that ticket.

@jhpalmieri
Copy link
Member Author

comment:16

So should we close this ticket, or should we use this to remove spkg-check? In either case, #22380 can instate a better version.

@kiwifb
Copy link
Member

kiwifb commented Nov 14, 2019

comment:17

Replying to @jhpalmieri:

So should we close this ticket, or should we use this to remove spkg-check? 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].

@jhpalmieri
Copy link
Member Author

Branch: u/jhpalmieri/cvxopt-no-spkg-check

@jhpalmieri
Copy link
Member Author

New commits:

cee0cabtrac 28730: testsuite for cvxopt is broken, so remove spkg-check.

@jhpalmieri
Copy link
Member Author

Commit: cee0cab

@jhpalmieri
Copy link
Member Author

Author: John Palmieri

@kiwifb
Copy link
Member

kiwifb commented Nov 14, 2019

comment:20

LGTM

@kiwifb
Copy link
Member

kiwifb commented Nov 14, 2019

Reviewer: François Bissey

@vbraun
Copy link
Member

vbraun commented Nov 16, 2019

Changed branch from u/jhpalmieri/cvxopt-no-spkg-check to cee0cab

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

No branches or pull requests

3 participants