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: sage-7.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 mkoeppe)

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 copy-paste 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 mkoeppe

  • Branch set to u/mkoeppe/fixes_for_the_cvxopt_mip_backend

comment:2 Changed 3 years ago by mkoeppe

  • Cc dimpase ncohen ingolfured added
  • Commit set to 39473c9d7b53329f15d79cf5da2acdad7b1783ff
  • Description modified (diff)
  • Status changed from new to needs_review

New commits:

39473c9Fix and FIXME

comment:3 Changed 3 years ago by mkoeppe

  • Authors set to Matthias Koeppe

comment:4 follow-up: Changed 3 years ago by dimpase

this backend was partially just a warm-up for the CVXOPT SDP interface.

Besides, if you wanted to solve your LPs with an interior-point method, CVXOPT was the only way. Is it still the case?

comment:5 in reply to: ↑ 4 Changed 3 years ago by mkoeppe

Replying to dimpase:

Besides, if you wanted to solve your LPs with an interior-point 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 vdelecroix

  • 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 git

  • Commit changed from 39473c9d7b53329f15d79cf5da2acdad7b1783ff to 183ce259998f1a014054045bcad6499f35ac978c

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

bceca77CVXOPTBackend: Use 'is None' instead of '== None'
728750fCVXOPTBackend.add_variables: Pass arguments to add_variable, correct default for lower_bound
b4a8ed7Copy new CVXOPTBackend.add_variables tests to other backends
b0d89e4GLPKBackend.add_variables: Set column names correctly
183ce25PPLBackend.add_variable, add_variables: Don't silently ignore binary=True, integer=True

comment:8 Changed 3 years ago by mkoeppe

  • 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 git

  • Commit changed from 183ce259998f1a014054045bcad6499f35ac978c to e47b608c20f5bc1157e15fd796e85e8ce0c86833

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

e47b608CVXOPTBackend.add_linear_constraints: Add doctest, simplify code

comment:10 Changed 3 years ago by mkoeppe

  • 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 mkoeppe

  • Description modified (diff)

comment:12 Changed 3 years ago by mkoeppe

  • Cc jdemeyer added

comment:13 Changed 3 years ago by mkoeppe

  • Description modified (diff)

comment:14 Changed 3 years ago by vdelecroix

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 mkoeppe

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 integer-constrained. That's why it's a NotImplementedError. I've created a ticket (#20351).

comment:16 Changed 3 years ago by dimpase

  • 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 vbraun

  • Branch changed from u/mkoeppe/fixes_for_the_cvxopt_mip_backend to e47b608c20f5bc1157e15fd796e85e8ce0c86833
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.