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

Add optional package cvxpy, update cylp, add CVXPY MIP backend #34251

Closed
mkoeppe opened this issue Jul 30, 2022 · 84 comments · Fixed by #35120
Closed

Add optional package cvxpy, update cylp, add CVXPY MIP backend #34251

mkoeppe opened this issue Jul 30, 2022 · 84 comments · Fixed by #35120

Comments

@mkoeppe
Copy link
Contributor

mkoeppe commented Jul 30, 2022

(split out from #31962)

We add cvxpy for disciplined convex programming and extensions as an optional package.

We add a new MIP backend solver="CVXPY". Using solver="CVXPY/ECOS", solver="CVXPY/GLPK", etc., CVXPY can be asked to use a specific solver.

When also cylp is installed (from #33487, upgraded here), the new solver backend "CVXPY/CBC" becomes the default (unless Gurobi or CPLEX are available), thus replacing the similar functionality of the CBC backend provided by sage_numerical_backend_coin.

Details about dependencies: Requirements listed in
cvxpy's setup.py:

python_requires='>=3.6',
install_requires=["osqp >= 0.4.1",
                  "ecos >= 2",
                  "scs >= 1.1.6",
                  "numpy >= 1.15",
                  "scipy >= 1.1.0"],

So the new dependencies are:

  • osqp (operator splitting quadratic program) (home · repo · PyPI)
  • ecos (embedded conic solver) (repo · PyPI)
  • SCS (splitting conic solver) (repo · PyPI)

In addition, osqp's requirements.txt lists:
numpy >= 1.7, scipy >= 0.13.2, qdldl

which adds:

  • qldl-python (interface to QDLDL, to factor quasi-definite linear systems) (repo · PyPI)

Finally, qldl-python depends on pybind11 (already an SPKG)

See also:

Follow-ups:

Depends on #34228

CC: @dimpase @yuan-zhou @sheerluck @dcoudert @seblabbe @maxale @kiwifb @isuruf @tkralphs

Component: packages: optional

Author: Matthias Koeppe, Andrey Belgorodski

Reviewer: Martin Pépin

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

@mkoeppe mkoeppe added this to the sage-9.7 milestone Jul 30, 2022
@mkoeppe mkoeppe changed the title Add optional package cvxpy Add optional package cvxpy, update cylp Jul 30, 2022
@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jul 30, 2022

Branch: u/mkoeppe/add_optional_package_cvxpy

@mkoeppe

This comment has been minimized.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jul 30, 2022

Commit: 76d1208

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jul 30, 2022

Last 10 new commits:

cff440eadding some code from ppl_backend.pyx to cvxpy_backend.{pyx,pxd}
8c6f323CVXPYBackend.is_variable_{boolean,integer,continuous}: Implement
b497afcsrc/sage/numerical/backends/cvxpy_backend.pyx: Fix up some doctests
942d2edCVXPYBackend.objective_coefficient: Fix up
5a5ab69build/pkgs/cylp: Add patch from https://github.com/coin-or/CyLP/pull/150
75f867dbuild/pkgs/cvxpy: Use https://github.com/cvxpy/cvxpy/pull/1707
1fb2926build/pkgs/cylp: Add another commit from https://github.com/coin-or/CyLP/pull/150
4fd763asrc/sage/numerical/backends/cvxpy_backend_test.py: New
8e482bebuild/pkgs/cylp: Update to 0.91.5, remove patches
76d1208build/pkgs/cvxpy/requirements.txt: Update git ref

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jul 30, 2022

Changed author from Matthias Koeppe to Matthias Koeppe, Andrey Belgorodski

@mkoeppe mkoeppe changed the title Add optional package cvxpy, update cylp Add optional package cvxpy, update cylp, add CVXPY MIP backend Jul 30, 2022
@mkoeppe

This comment has been minimized.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 30, 2022

Changed commit from 76d1208 to 7d4dccb

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 30, 2022

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

7d4dccbbuild/pkgs/cvxpy: Make it a normal package, add dependencies qdldl_python, osqp, scs, ecos

@mkoeppe

This comment has been minimized.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 30, 2022

Changed commit from 7d4dccb to aab359f

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 30, 2022

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

aab359fbuild/pkgs/{ecos,osqp}_python: Renamed from ecos, osqp

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 30, 2022

Changed commit from aab359f to 6d039ae

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 30, 2022

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

468b2c5build/pkgs/ecos_python/dependencies: Add suitesparse
6d039aebuild/pkgs/{ecos,osqp}_python: Use --no-build-isolation

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 30, 2022

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

763985dsrc/sage/numerical/backends/cvxpy_backend.pyx: Make doctests pass

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 30, 2022

Changed commit from 6d039ae to 763985d

@mkoeppe

This comment has been minimized.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 30, 2022

Changed commit from 763985d to a079e6e

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 30, 2022

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

a079e6ebuild/pkgs/cylp/spkg-install.in: Use --no-build-isolation

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 30, 2022

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

9f9207asrc/sage/numerical/backends/cvxpy_backend.pyx: Store constraint names
7b1ee3fsrc/sage/numerical/backends/cvxpy_backend.pyx: Use our column names when no name is given
cbbc6absrc/sage/numerical/backends/cvxpy_backend.pyx: Convert lower, upper variable bounds to float
9f9b346src/sage/combinat/matrices/dancing_links.pyx: Make doctest more flexible

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 30, 2022

Changed commit from a079e6e to 9f9b346

@sheerluck
Copy link
Contributor

comment:16

I am not sure, but it feels like if default_sdp_solver() is 'Cvxopt' then the tests in cvxpy_sdp_backend.py are broken. And if default_sdp_solver() is 'Cvxpy' then the tests in cvxopt_backend.pyx are broken. Sorry if that is irrelevant to this ticket.


New commits:

9f9207asrc/sage/numerical/backends/cvxpy_backend.pyx: Store constraint names
7b1ee3fsrc/sage/numerical/backends/cvxpy_backend.pyx: Use our column names when no name is given
cbbc6absrc/sage/numerical/backends/cvxpy_backend.pyx: Convert lower, upper variable bounds to float
9f9b346src/sage/combinat/matrices/dancing_links.pyx: Make doctest more flexible

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jul 30, 2022

comment:17

Here on this ticket, there's no SDP solver backend

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jul 30, 2022

comment:18

We had that on #31962, but I have split out the present ticket

@Kerl13
Copy link

Kerl13 commented Oct 2, 2022

Attachment: cvxpy.log

@Kerl13
Copy link

Kerl13 commented Oct 2, 2022

comment:58

After rebasing on develop, I now get the attached errors when running sage -t --long --random-seed=262306895609334379547192037378099413502 src/sage/numerical/backends/cvxpy_backend.pyx

Don't know why the segfault occurred before but the new errors look minor now

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 2, 2022

Changed commit from c9f0b39 to 539c970

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 2, 2022

Branch pushed to git repo; I updated commit sha1. This was a forced push. Last 10 new commits:

55fc4fbbuild/pkgs/{cvxpy,scs,osqp_python,ecos_python,qdldl_python}: Add distros/conda.txt
18e7434build/pkgs/osqp_python/dependencies: Add cmake
11e4de8build/pkgs/ecos_python/dependencies: Remove suitesparse
9a1eeecbuild/pkgs/cylp/SPKG.rst: Add more detailed license info
b9c92aesrc/sage/numerical/backends/cvxpy_backend.pyx: Add doc for init args
5b840bdsrc/sage/numerical/backends: Remove unnecessary imports, muffle unused variable warnings
7132302src/sage/numerical/backends: Fix copy-pasted typos in documentation (binary/continuous/integer)
b5cbc5asrc/sage/numerical/backends/cvxpy_backend.pyx: Add to doc
83ce468build/pkgs/scs/spkg-install.in: Use --no-build-isolation
539c970src/sage/numerical/backends/cvxpy_backend.pyx: Update doctest output for bounds methods

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 15, 2022

Changed commit from 539c970 to 7346fd1

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 15, 2022

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

98c38c1Merge tag '9.8.beta3' into t/34251/add_optional_package_cvxpy
7211f42build/pkgs/cvxpy: Update to 1.2.2
7346fd1build/pkgs/scs: Update to 3.2.2

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Nov 16, 2022

@mkoeppe mkoeppe modified the milestones: sage-9.8, sage-9.9 Dec 31, 2022
@sheerluck
Copy link
Contributor

comment:63

we need this ticket for #31962

we need #31962 for #32053

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jan 17, 2023

comment:64

We'll have to merge this with #21003, which touches the same files and will likely be in the next beta

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 19, 2023

Changed commit from 7346fd1 to 666e973

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 19, 2023

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

666e973Merge tag '9.8.beta7' into t/34251/add_optional_package_cvxpy

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 19, 2023

Changed commit from 666e973 to 7ff5766

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 19, 2023

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

ba9fcbcbuild/pkgs/cvxpy: Update to 1.3.0
7515564build/pkgs/osqp_python: Update to 0.6.2.post8
91cbc21build/pkgs/ecos_python: Update to 2.0.12
7ff5766build/pkgs/qdldl_python: Update to 0.1.5.post3

@sheerluck
Copy link
Contributor

comment:68

without this ticket I can do this

sage: from sage.combinat.designs.database import OA_10_469
sage: OA = OA_10_469()
sage: print(len(OA))
219961

this ticket just hangs (probably O(n2) while p.set_objective(p.sum(b[fe] for fe in L)), looks like every b[fe] creates new cvxpy.Problem with 1, 2, 3 ... 900 constraints)

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jan 20, 2023

comment:69

Yes, we should definitely improve this

@dimpase
Copy link
Member

dimpase commented Jan 30, 2023

comment:70

Replying to @sheerluck:

without this ticket I can do this

sage: from sage.combinat.designs.database import OA_10_469
sage: OA = OA_10_469()
sage: print(len(OA))
219961

this ticket just hangs (probably O(n2) while p.set_objective(p.sum(b[fe] for fe in L)), looks like every b[fe] creates new cvxpy.Problem with 1, 2, 3 ... 900 constraints)

I don't see where in OA_10_469() any optimisation problem is set up. It seems to be a simple
combinatorial construction, no solvers involved. OA is just a list.

@sheerluck
Copy link
Contributor

comment:71

Replying to Dima Pasechnik:

I don't see where in OA_10_469() any optimisation problem is set up.

database.py:1693 -> 
    matrix = _reorder_matrix(symmetric_design)

orthogonal_arrays_build_recursive.py:1189 ->
    matching = g.matching(algorithm="LP")

graph.py:4210 ->
    p = MixedIntegerLinearProgram(maximization=True, solver=solver)

@sheerluck
Copy link
Contributor

and now I can link a range of lines

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Feb 13, 2023

Removed branch from issue description; replaced by PR #35120

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Feb 13, 2023

this ticket just hangs (probably O(n2) while p.set_objective(p.sum(b[fe] for fe in L)), looks like every b[fe] creates new cvxpy.Problem with 1, 2, 3 ... 900 constraints)

I'd suggest that we improve performance in follow-up PRs

vbraun pushed a commit that referenced this issue Mar 26, 2023
    
<!-- ^^^^^
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 #1234" use "Introduce new method to
calculate 1+1"
-->
### 📚 Description
Fixes #34251

<!-- Describe your changes here in detail -->
<!-- Why is this change required? What problem does it solve? -->
<!-- If it resolves an open issue, please link to the issue here. For
example "Closes #1337" -->

### 📝 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! -->

- [x] I have made sure that the title is self-explanatory and the
description concisely explains the PR.
- [x] I have linked an issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation accordingly.

### ⌛ Dependencies
<!-- List all open pull requests that this PR logically depends on -->
<!--
- #xyz: short description why this is a dependency
- #abc: ...
-->
    
URL: #35120
Reported by: Matthias Köppe
Reviewer(s): Dima Pasechnik
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants