Opened 3 years ago
Closed 3 years ago
#20303 closed defect (fixed)
Fixes for add_variables in CVXOPT, PPL, GLPK MIP backends and add_linear_constraints in CVXOPT
Reported by:  mkoeppe  Owned by:  

Priority:  major  Milestone:  sage7.2 
Component:  numerical  Keywords:  lp, CVXOPT, PPL, GLPK 
Cc:  dimpase, ncohen, ingolfured, vdelecroix, jdemeyer  Merged in:  
Authors:  Matthias Koeppe  Reviewers:  Vincent Delecroix, Dima Pasechnik 
Report Upstream:  N/A  Work issues:  
Branch:  e47b608 (Commits)  Commit:  e47b608c20f5bc1157e15fd796e85e8ce0c86833 
Dependencies:  Stopgaps: 
Description (last modified by )
The add_variables
method had defects for the CVXOPT, PPL, GLPK.
Also, add_linear_constraints
had a defect for CVXOPT.
See new tests added in this ticket.
I first noticed it with the CVXOPT backend. Then, by copying the same doctests to the other backends, I found more errors in the PPL, GLPK backends. (See #20302 for a discussion regarding doctests for the backends  there should be a better way than relying on copypaste discipline.)
Note, I have copied the doctests to the CPLEX and Gurobi backends but am currently not able to run them.
Change History (17)
comment:1 Changed 3 years ago by
 Branch set to u/mkoeppe/fixes_for_the_cvxopt_mip_backend
comment:2 Changed 3 years ago by
 Cc dimpase ncohen ingolfured added
 Commit set to 39473c9d7b53329f15d79cf5da2acdad7b1783ff
 Description modified (diff)
 Status changed from new to needs_review
comment:3 Changed 3 years ago by
comment:4 followup: ↓ 5 Changed 3 years ago by
this backend was partially just a warmup for the CVXOPT SDP interface.
Besides, if you wanted to solve your LPs with an interiorpoint method, CVXOPT was the only way. Is it still the case?
comment:5 in reply to: ↑ 4 Changed 3 years ago by
Replying to dimpase:
Besides, if you wanted to solve your LPs with an interiorpoint method, CVXOPT was the only way. Is it still the case?
I think the answer is yes; neither the GLPK backend nor the CPLEX backend seem to have support for these solvers' interior point algorithms.
comment:6 Changed 3 years ago by
 Status changed from needs_review to needs_work
Hello,
You should explain in the ticket description what is not working... and also provide doctests to show that it works now!
comment:7 Changed 3 years ago by
 Commit changed from 39473c9d7b53329f15d79cf5da2acdad7b1783ff to 183ce259998f1a014054045bcad6499f35ac978c
Branch pushed to git repo; I updated commit sha1. New commits:
bceca77  CVXOPTBackend: Use 'is None' instead of '== None'

728750f  CVXOPTBackend.add_variables: Pass arguments to add_variable, correct default for lower_bound

b4a8ed7  Copy new CVXOPTBackend.add_variables tests to other backends

b0d89e4  GLPKBackend.add_variables: Set column names correctly

183ce25  PPLBackend.add_variable, add_variables: Don't silently ignore binary=True, integer=True

comment:8 Changed 3 years ago by
 Cc vdelecroix added
 Description modified (diff)
 Keywords PPL GLPK added
 Status changed from needs_work to needs_review
 Summary changed from Fixes for the CVXOPT MIP Backend to Fixes for add_variables in CVXOPT, PPL, GLPK MIP backends
comment:9 Changed 3 years ago by
 Commit changed from 183ce259998f1a014054045bcad6499f35ac978c to e47b608c20f5bc1157e15fd796e85e8ce0c86833
Branch pushed to git repo; I updated commit sha1. New commits:
e47b608  CVXOPTBackend.add_linear_constraints: Add doctest, simplify code

comment:10 Changed 3 years ago by
 Description modified (diff)
 Summary changed from Fixes for add_variables in CVXOPT, PPL, GLPK MIP backends to Fixes for add_variables in CVXOPT, PPL, GLPK MIP backends and add_linear_constraints in CVXOPT
comment:11 Changed 3 years ago by
 Description modified (diff)
comment:12 Changed 3 years ago by
 Cc jdemeyer added
comment:13 Changed 3 years ago by
 Description modified (diff)
comment:14 Changed 3 years ago by
I am not sure that NotImplementedError
is the right error for PPL add_variable
method. PPL is precisely intended to solve problems over rationals number. I would rather use a ValueError
.
comment:15 Changed 3 years ago by
PPL is a rational MIP solver, not just an LP solver. It's only the Sage backend that does not support setting variables to be integerconstrained.
That's why it's a NotImplementedError
. I've created a ticket (#20351).
comment:16 Changed 3 years ago by
 Reviewers set to Vincent Delecroix, Dima Pasechnik
 Status changed from needs_review to positive_review
looks good. Do not forget to remove NotImplementedError
from PPL backend once it can do integer/binary vars from #20354.
I tested CBC, but no CPLEX or GUROBI.
comment:17 Changed 3 years ago by
 Branch changed from u/mkoeppe/fixes_for_the_cvxopt_mip_backend to e47b608c20f5bc1157e15fd796e85e8ce0c86833
 Resolution set to fixed
 Status changed from positive_review to closed
New commits:
Fix and FIXME