Opened 5 years ago
Closed 4 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:  sage7.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 )
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 copypasted 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 5 years ago by
 Dependencies set to #20323
 Description modified (diff)
comment:2 Changed 4 years ago by
 Branch set to u/mkoeppe/genericbackend__add_default_implementation_of__add_variables__and__add_linear_constraints_
comment:3 Changed 4 years ago by
 Commit set to 987f6093b9821038074801f8790ceaa406ce4c56
 Status changed from new to needs_review
comment:4 Changed 4 years ago by
comment:5 Changed 4 years ago by
 Status changed from needs_review to needs_work
Falilures in the patch bot
comment:6 Changed 4 years ago by
 Commit changed from 987f6093b9821038074801f8790ceaa406ce4c56 to 40876eed69fae9ba7d2ac0440a2fd3b131eef85f
Branch pushed to git repo; I updated commit sha1. New commits:
40876ee  PPLBackend.add_linear_constraints: Fix handling of 'names' argument

comment:7 Changed 4 years ago by
 Status changed from needs_work to needs_review
comment:8 Changed 4 years ago by
 Cc jdemeyer chapoton nbruin added; ncohen removed
comment:9 Changed 4 years ago by
 Description modified (diff)
 Type changed from enhancement to defect
Needs review.
comment:10 Changed 4 years ago by
ping?
comment:11 Changed 4 years ago by
So you moved these implementations into the generic backend, basically?
comment:12 Changed 4 years ago by
Yes, and added a _test method and fixed a bug in the PPL method (which was tested by a wrong test).
comment:13 Changed 4 years ago by
 Milestone changed from sage7.2 to sage7.3
 Reviewers set to Dima Pasechnik
 Status changed from needs_review to positive_review
ok, good.
comment:14 Changed 4 years ago by
Thanks for reviewing, Dima. When you have a moment, could you look at #20600?
comment:15 Changed 4 years ago by
 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
Last 10 new commits:
CPLEXBackend._test_add_variables: Make test suitable for InteractiveLPBackend
GenericBackend._test_add_linear_constraint_vector: Make test suitable for InteractiveLPBackend
GenericBackend._test_solve: Remove again for now; too many failures
CVXOPTBackend, InteractiveLPBackend: Remove add_variables implementations, inherit them from GenericBackend
GenericBackend.add_linear_constraints: New
CoinBackend.add_linear_constraints: Remove, inherit from GenericBackend
CVXOPTBackend.add_linear_constraints: Remove, inherit from GenericBackend
InteractiveLPBackend.add_linear_constraints: Remove, inherit from GenericBackend
GurobiBackend.add_variables: Remove, inherit from GenericBackend
GenericBackend._test_add_linear_constraints: Add tests from COINBackend, CVXOPTBackend