Opened 3 years ago

Closed 3 years ago

#20325 closed defect (fixed)

GenericBackend: Add default implementation of `add_variables` and `add_linear_constraints`

Reported by: mkoeppe Owned by:
Priority: major Milestone: sage-7.3
Component: numerical Keywords: lp
Cc: dimpase, vdelecroix, jdemeyer, chapoton, nbruin Merged in:
Authors: Matthias Koeppe Reviewers: Dima Pasechnik
Report Upstream: N/A Work issues:
Branch: 40876ee (Commits) Commit: 40876eed69fae9ba7d2ac0440a2fd3b131eef85f
Dependencies: #20323 Stopgaps:

Description (last modified by mkoeppe)

The backend methods add_variables and add_linear_constraints should have a default implementation in GenericBackend, like add_linear_constraint_vector.

add_variables can be taken from Gurobi and also removed from CVXOPT and InteractiveLP. add_linear_constraints can be taken from COIN and also removed from CVXOPT and InteractiveLP. (The other backends have specific implementations of these methods; one supposes that they are faster, though this probably has not been tested.)

Since the removal of the copy-pasted functions from the backends would remove doctests, I mark this ticket as dependent on #20323.

  • * *

The new tests revealed a bug in the PPL backend, which has been fixed. Also, the CPLEX backend used to add variables in reverse order for no good reason; changed that.

Change History (15)

comment:1 Changed 3 years ago by mkoeppe

  • Dependencies set to #20323
  • Description modified (diff)

comment:2 Changed 3 years ago by mkoeppe

  • Branch set to u/mkoeppe/genericbackend__add_default_implementation_of__add_variables__and__add_linear_constraints_

comment:3 Changed 3 years ago by mkoeppe

  • Commit set to 987f6093b9821038074801f8790ceaa406ce4c56
  • Status changed from new to needs_review

Last 10 new commits:

f2d52f5CPLEXBackend._test_add_variables: Make test suitable for InteractiveLPBackend
6604442GenericBackend._test_add_linear_constraint_vector: Make test suitable for InteractiveLPBackend
e6ba997GenericBackend._test_solve: Remove again for now; too many failures
c3d2959CVXOPTBackend, InteractiveLPBackend: Remove add_variables implementations, inherit them from GenericBackend
d061abfGenericBackend.add_linear_constraints: New
8b73e2dCoinBackend.add_linear_constraints: Remove, inherit from GenericBackend
16ed7e9CVXOPTBackend.add_linear_constraints: Remove, inherit from GenericBackend
afad991InteractiveLPBackend.add_linear_constraints: Remove, inherit from GenericBackend
941d30bGurobiBackend.add_variables: Remove, inherit from GenericBackend
987f609GenericBackend._test_add_linear_constraints: Add tests from COINBackend, CVXOPTBackend

comment:4 Changed 3 years ago by mkoeppe

  • Authors set to Matthias Koeppe

comment:5 Changed 3 years ago by vbraun

  • Status changed from needs_review to needs_work

Falilures in the patch bot

comment:6 Changed 3 years ago by git

  • Commit changed from 987f6093b9821038074801f8790ceaa406ce4c56 to 40876eed69fae9ba7d2ac0440a2fd3b131eef85f

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

40876eePPLBackend.add_linear_constraints: Fix handling of 'names' argument

comment:7 Changed 3 years ago by mkoeppe

  • Status changed from needs_work to needs_review

comment:8 Changed 3 years ago by mkoeppe

  • Cc jdemeyer chapoton nbruin added; ncohen removed

comment:9 Changed 3 years ago by mkoeppe

  • Description modified (diff)
  • Type changed from enhancement to defect

Needs review.

comment:10 Changed 3 years ago by mkoeppe

ping?

comment:11 Changed 3 years ago by dimpase

So you moved these implementations into the generic backend, basically?

comment:12 Changed 3 years ago by mkoeppe

Yes, and added a _test method and fixed a bug in the PPL method (which was tested by a wrong test).

comment:13 Changed 3 years ago by dimpase

  • Milestone changed from sage-7.2 to sage-7.3
  • Reviewers set to Dima Pasechnik
  • Status changed from needs_review to positive_review

ok, good.

comment:14 Changed 3 years ago by mkoeppe

Thanks for reviewing, Dima. When you have a moment, could you look at #20600?

comment:15 Changed 3 years ago by vbraun

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