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

configure: Add option --disable-cvxopt #32226

Closed
mkoeppe opened this issue Jul 17, 2021 · 27 comments
Closed

configure: Add option --disable-cvxopt #32226

mkoeppe opened this issue Jul 17, 2021 · 27 comments

Comments

@mkoeppe
Copy link
Contributor

mkoeppe commented Jul 17, 2021

Given the brittle build system of cvxopt (#31905, #32087), and of its dependency suitesparse (#30478), we should give users a recourse when the installation fails.

cvxopt provides our only backend for SemidefiniteProgram, introduced over 5 years ago. Although not yet used anywhere in the Sage library, it provides important functionality for a number of Sage user, e.g.
it's used in https://github.com/bachirelkhadir/sdp_solvers_experiments which is needed for
demonstration of https://epubs.siam.org/doi/pdf/10.1137/19M1287584 in
https://github.com/bachirelkhadir/Convex-Quaternary-Quartics-Are-Sum-of-Squares/blob/master/optimal_constant_generalized_cs_deg_8_a4.pdf

Hence, we keep cvxopt as a standard package, but make it disablable.

Marking doctests that depend on cvxopt with # optional - cvxopt is also preparation for modularization. Thanks to the dummy backend MatrixSDPBackend, the modeling side of the interface can still be doctested even if cvxopt is not installed.

We also deprecate the outdated and unused global function linear_program, which directly uses cvxopt.

CC: @dimpase @tscrim @orlitzky

Component: packages: standard

Author: Matthias Koeppe

Branch/Commit: 2ab2f15

Reviewer: Dima Pasechnik

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

@mkoeppe mkoeppe added this to the sage-9.4 milestone Jul 17, 2021
@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jul 17, 2021

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jul 17, 2021

Commit: 83e309a

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jul 17, 2021

New commits:

83e309abuild/pkgs/{cvxopt,suitesparse}/type: Change to optional

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jul 17, 2021

Author: Matthias Koeppe

@mkoeppe

This comment has been minimized.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 17, 2021

Branch pushed to git repo; I updated commit sha1. New commits:

f77a3f8src/doc/en/thematic_tutorials/numerical_sage/cvxopt.rst: Mark all tests optional - cvxopt

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 17, 2021

Changed commit from 83e309a to f77a3f8

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 17, 2021

Changed commit from f77a3f8 to 88c2291

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 17, 2021

Branch pushed to git repo; I updated commit sha1. New commits:

88c2291src/sage/numerical/backends/cvxopt_backend.pyx: Mark all tests # optional - cvxopt

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 17, 2021

Branch pushed to git repo; I updated commit sha1. New commits:

c3bf30csrc/sage/numerical/backends/cvxopt_sdp_backend.pyx: Mark all tests # optional - cvxopt
198a681src/sage/numerical/optimize.py: Deprecate linear_program
9955c1fsrc/sage/numerical/sdp.pyx, src/sage/numerical/backends/generic_sdp_backend.pyx: Use MatrixSDPBackend as fall back, mark tests using 'solve' # optional - cvxopt

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 17, 2021

Changed commit from 88c2291 to 9955c1f

@mkoeppe

This comment has been minimized.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 17, 2021

Branch pushed to git repo; I updated commit sha1. New commits:

4608b0asrc/setup.cfg.m4: Remove cvxopt

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 17, 2021

Changed commit from 9955c1f to 4608b0a

@dimpase
Copy link
Member

dimpase commented Jul 18, 2021

comment:10

I disagree. Most of Sage functionality is not used by sagelib, it's absolutely not a reason to remove it.
Besides, it's the only SDP backend we have at the moment.

A number of publications I have depend on this functionality, and perhaps more.

@dimpase dimpase removed this from the sage-9.4 milestone Jul 18, 2021
@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jul 18, 2021

comment:11

Note that this ticket does not remove cvxopt, it merely makes it optional - same status as cbc and as many other well maintained software that some people use within sage

The problem is that standard packages really need to build on everyone's system, and right now there are unresolved issues (linked in the ticket description).

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 18, 2021

Branch pushed to git repo; I updated commit sha1. New commits:

2cde511configure.ac: Add option --disable-cvxopt
2ab2f15Revert "build/pkgs/{cvxopt,suitesparse}/type: Change to optional"

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 18, 2021

Changed commit from 4608b0a to 2ab2f15

@mkoeppe

This comment has been minimized.

@mkoeppe mkoeppe changed the title Downgrade suitesparse, cvxopt to "optional" configure: Add option --disable-cvxopt Jul 18, 2021
@mkoeppe mkoeppe added this to the sage-9.4 milestone Jul 18, 2021
@mkoeppe

This comment has been minimized.

@orlitzky
Copy link
Contributor

comment:15

What's the practical difference between standard-but-optional and plain optional? Is it just the default value of --enable-<pkg> (yes vs no)?

It's a tiny bit annoying to have to remember to --disable-cvxopt when I'm building a branch that doesn't use cvxopt (i.e. pretty much all of them), but I guess that's no different than most packages that disable some things but enable others by default.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jul 19, 2021

comment:16

Replying to @orlitzky:

What's the practical difference between standard-but-optional and plain optional? Is it just the default value of --enable-<pkg> (yes vs no)?

Kind of, but in addition to that, also controls what is packaged up in the source distribution

@dimpase
Copy link
Member

dimpase commented Jul 19, 2021

Reviewer: Dima Pasechnik

@dimpase
Copy link
Member

dimpase commented Jul 19, 2021

comment:17

OK, fine by me.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jul 19, 2021

comment:18

Thanks.

@dimpase

This comment has been minimized.

@vbraun
Copy link
Member

vbraun commented Jul 23, 2021

Changed branch from u/mkoeppe/downgrade_suitesparse__cvxopt_to__optional_ to 2ab2f15

@vbraun vbraun closed this as completed in 60dd1ed Jul 23, 2021
mkoeppe added a commit to mkoeppe/sage that referenced this issue Oct 29, 2023
mkoeppe added a commit to mkoeppe/sage that referenced this issue Oct 31, 2023
vbraun pushed a commit to vbraun/sage that referenced this issue Nov 1, 2023
sagemathgh-36567: `sage.numerical`: Update `# needs`
    
<!-- ^^^^^
Please provide a concise, informative and self-explanatory title.
Don't put issue numbers in there, do this in the PR body below.
For example, instead of "Fixes sagemath#1234" use "Introduce new method to
calculate 1+1"
-->
<!-- Describe your changes here in detail -->

Also removing a function deprecated in sagemath#32226

<!-- Why is this change required? What problem does it solve? -->
<!-- If this PR resolves an open issue, please link to it here. For
example "Fixes sagemath#12345". -->
- In part cherry picked from sagemath#35095
- Part of sagemath#29705
<!-- If your change requires a documentation PR, please link it
appropriately. -->

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->
<!-- If your change requires a documentation PR, please link it
appropriately -->
<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
<!-- Feel free to remove irrelevant items. -->

- [x] The title is concise, informative, and self-explanatory.
- [ ] The description explains in detail what this PR is about.
- [x] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation accordingly.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on
- sagemath#12345: short description why this is a dependency
- sagemath#34567: ...
-->

<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
    
URL: sagemath#36567
Reported by: Matthias Köppe
Reviewer(s): David Coudert, Matthias Köppe
vbraun pushed a commit to vbraun/sage that referenced this issue Nov 2, 2023
sagemathgh-36567: `sage.numerical`: Update `# needs`
    
<!-- ^^^^^
Please provide a concise, informative and self-explanatory title.
Don't put issue numbers in there, do this in the PR body below.
For example, instead of "Fixes sagemath#1234" use "Introduce new method to
calculate 1+1"
-->
<!-- Describe your changes here in detail -->

Also removing a function deprecated in sagemath#32226

<!-- Why is this change required? What problem does it solve? -->
<!-- If this PR resolves an open issue, please link to it here. For
example "Fixes sagemath#12345". -->
- In part cherry picked from sagemath#35095
- Part of sagemath#29705
<!-- If your change requires a documentation PR, please link it
appropriately. -->

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->
<!-- If your change requires a documentation PR, please link it
appropriately -->
<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
<!-- Feel free to remove irrelevant items. -->

- [x] The title is concise, informative, and self-explanatory.
- [ ] The description explains in detail what this PR is about.
- [x] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation accordingly.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on
- sagemath#12345: short description why this is a dependency
- sagemath#34567: ...
-->

<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
    
URL: sagemath#36567
Reported by: Matthias Köppe
Reviewer(s): David Coudert, Matthias Köppe
vbraun pushed a commit to vbraun/sage that referenced this issue Nov 5, 2023
sagemathgh-36567: `sage.numerical`: Update `# needs`
    
<!-- ^^^^^
Please provide a concise, informative and self-explanatory title.
Don't put issue numbers in there, do this in the PR body below.
For example, instead of "Fixes sagemath#1234" use "Introduce new method to
calculate 1+1"
-->
<!-- Describe your changes here in detail -->

Also removing a function deprecated in sagemath#32226

<!-- Why is this change required? What problem does it solve? -->
<!-- If this PR resolves an open issue, please link to it here. For
example "Fixes sagemath#12345". -->
- In part cherry picked from sagemath#35095
- Part of sagemath#29705
<!-- If your change requires a documentation PR, please link it
appropriately. -->

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->
<!-- If your change requires a documentation PR, please link it
appropriately -->
<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
<!-- Feel free to remove irrelevant items. -->

- [x] The title is concise, informative, and self-explanatory.
- [ ] The description explains in detail what this PR is about.
- [x] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation accordingly.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on
- sagemath#12345: short description why this is a dependency
- sagemath#34567: ...
-->

<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
    
URL: sagemath#36567
Reported by: Matthias Köppe
Reviewer(s): David Coudert, Matthias Köppe
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

4 participants