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

GenericBackend: Add default implementation of add_variables and add_linear_constraints #20325

Closed
mkoeppe opened this issue Mar 30, 2016 · 18 comments

Comments

@mkoeppe
Copy link
Member

mkoeppe commented Mar 30, 2016

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.

Depends on #20323

CC: @dimpase @videlec @jdemeyer @fchapoton @nbruin

Component: numerical

Author: Matthias Koeppe

Branch/Commit: 40876ee

Reviewer: Dima Pasechnik

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

@mkoeppe mkoeppe added this to the sage-7.2 milestone Mar 30, 2016
@mkoeppe
Copy link
Member Author

mkoeppe commented Apr 11, 2016

Dependencies: #20323

@mkoeppe

This comment has been minimized.

@mkoeppe
Copy link
Member Author

mkoeppe commented May 24, 2016

@mkoeppe
Copy link
Member Author

mkoeppe commented May 24, 2016

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

@mkoeppe
Copy link
Member Author

mkoeppe commented May 24, 2016

Commit: 987f609

@mkoeppe
Copy link
Member Author

mkoeppe commented May 24, 2016

Author: Matthias Koeppe

@vbraun
Copy link
Member

vbraun commented May 25, 2016

comment:5

Falilures in the patch bot

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 26, 2016

Changed commit from 987f609 to 40876ee

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 26, 2016

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

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

@mkoeppe
Copy link
Member Author

mkoeppe commented Jun 3, 2016

comment:9

Needs review.

@mkoeppe

This comment has been minimized.

@mkoeppe
Copy link
Member Author

mkoeppe commented Jun 11, 2016

comment:10

ping?

@dimpase
Copy link
Member

dimpase commented Jun 11, 2016

comment:11

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

@mkoeppe
Copy link
Member Author

mkoeppe commented Jun 11, 2016

comment:12

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

@dimpase
Copy link
Member

dimpase commented Jun 12, 2016

comment:13

ok, good.

@dimpase
Copy link
Member

dimpase commented Jun 12, 2016

Reviewer: Dima Pasechnik

@dimpase dimpase modified the milestones: sage-7.2, sage-7.3 Jun 12, 2016
@mkoeppe
Copy link
Member Author

mkoeppe commented Jun 13, 2016

comment:14

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

@vbraun
Copy link
Member

vbraun commented Jun 14, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants