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

More tests for common MIP TestSuite: add_col, solve; some fixes for backends #20424

Closed
mkoeppe opened this issue Apr 11, 2016 · 32 comments
Closed

Comments

@mkoeppe
Copy link
Contributor

mkoeppe commented Apr 11, 2016

Split out from #20323.

This patch adds new _test methods for add_col, solve, and for #18572 (but disabled for CVXOPT, where it fails).
To make the new test methods happy,

  • implement add_col for Gurobi;
  • fix the implementation of add_col for COIN;
  • fix Gurobi's treatment of unbounded variables;
  • fix CPLEX unboundedness detection.

If anyone at all cares about the CVXOPT backend, perhaps they could fix it?

From dimpase:
I recall asking how one deals with different backends producing different, albeit equivalent, outputs. E.g. some of them would even introduce extra variables for some constraints (see e.g. ​ #13148 comment:2). Some backends assign names to constraints automatically.

Depends on #20323
Depends on #20600
Depends on #20325

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

Component: numerical

Author: Matthias Koeppe

Branch: 97c4542

Reviewer: Dima Pasechnik

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

@mkoeppe mkoeppe added this to the sage-7.2 milestone Apr 11, 2016
@mkoeppe

This comment has been minimized.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Apr 12, 2016

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Apr 12, 2016

Commit: e5e6227

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Apr 12, 2016

comment:3

Branch is on top of #20323.


Last 10 new commits:

d93b798Change from mutating instance _test methods to class methods
94a4ac5New method _test_ncols_nonnegative
2251125GenericBackend: Remove failing _test methods from this ticket to make the patchbot and its friends happy
934456fRevert "GenericBackend: Remove failing _test methods from this ticket to make the patchbot and its friends happy"
a85cada_test_copy: New
8070754_test_copy_does_not_share_data: New
d04b270Test backend.copy() rather than copy(backend)
480a71btest_copy_some_mips: New
3b49831Add _test_solve_trac_18572 (autogenerated)
e5e6227_test_solve_trac_18572: Replace float integers by integers to make test suitable for PPL backend

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 13, 2016

Changed commit from e5e6227 to af1e885

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 13, 2016

Branch pushed to git repo; I updated commit sha1. This was a forced push. Last 10 new commits:

48b9fe5Change from mutating instance _test methods to class methods
5394729New method _test_ncols_nonnegative
7138fa0GenericBackend: Remove failing _test methods from this ticket to make the patchbot and its friends happy
9f4f25cRevert "GenericBackend: Remove failing _test methods from this ticket to make the patchbot and its friends happy"
8421950_test_copy: New
3f0578b_test_copy_does_not_share_data: New
f5b42b8Test backend.copy() rather than copy(backend)
f21ce7ctest_copy_some_mips: New
21abe28Add _test_solve_trac_18572 (autogenerated)
af1e885_test_solve_trac_18572: Replace float integers by integers to make test suitable for PPL backend

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Apr 13, 2016

comment:5

Rebased on top of #20323 branch on top of 7.2.beta4

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 21, 2016

Changed commit from af1e885 to a5a1c60

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 21, 2016

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

6d01dc8Revert "GenericBackend: Remove failing _test methods from this ticket to make the patchbot and its friends happy"
850b817Add _test_solve_trac_18572 (autogenerated)
a5a1c60_test_solve_trac_18572: Replace float integers by integers to make test suitable for PPL backend

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Apr 21, 2016

comment:7

Rebased on top of 7.2.beta5.
Dropped "Test backend.copy() rather than copy(backend)".

@dimpase
Copy link
Member

dimpase commented Apr 27, 2016

comment:8

How do you do test autogeneration (mentioned in 850b817) ?

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Apr 27, 2016

comment:9

The test method added in this commit is the raw output from #20376 (LoggingBackend) when run on the testcase of #18572.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 29, 2016

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

cc0d326Revert "GenericBackend: Remove failing _test methods from this ticket to make the patchbot and its friends happy"
6cd0b4aAdd _test_solve_trac_18572 (autogenerated)
492534d_test_solve_trac_18572: Replace float integers by integers to make test suitable for PPL backend
658972cFix doctests when Gurobi is installed
cfaf157Instead of running MixedIntegerLinearProgram doctests with the default solver, use GLPK
bb7f5deMerge branch 't/20328/tests_related_to_cplex___gurobi' into t/20424/backend_testsuite_failing_tests

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 29, 2016

Changed commit from a5a1c60 to bb7f5de

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Apr 29, 2016

comment:11

rebased on 7.2.beta6

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 26, 2016

Branch pushed to git repo; I updated commit sha1. This was a forced push. Last 10 new commits:

f0ead34CPLEXBackend: Use CPXgetstat to properly detect unboundedness
4aeaf9bGurobiBackend.add_col: Implement
a65f66bGenericBackend._test_add_col: New
51c2116GurobiBackend.add_variable: Support coefficients keyword
c2eb25dGurobiBackend: Fix GRB_INFINITY confusion
6adf3d9CVXOPTBackend: Don't test _test_solve because of #18572
def61deAdd _test_solve_trac_18572 (autogenerated)
9f287a3_test_solve_trac_18572: Replace float integers by integers to make test suitable for PPL backend
b60d790CVXOPTBackend: Disable _test_solve_trac_18572 because of #18572
07c9c5dGLPKExactBackend: Adjust output

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 26, 2016

Changed commit from bb7f5de to 07c9c5d

@mkoeppe
Copy link
Contributor Author

mkoeppe commented May 26, 2016

Changed dependencies from #20323 to #20323,#20600,#20325

@mkoeppe

This comment has been minimized.

@mkoeppe mkoeppe changed the title More tests for common MIP TestSuite More tests for common MIP TestSuite: add_col, solve; some fixes for backends May 26, 2016
@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 30, 2016

Changed commit from 07c9c5d to 1ce4eb6

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 30, 2016

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

1ce4eb6CoinBackend.add_col: Use all coefficients

@mkoeppe

This comment has been minimized.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 25, 2016

Changed commit from 1ce4eb6 to 97c4542

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 25, 2016

Branch pushed to git repo; I updated commit sha1. This was a forced push. Last 10 new commits:

e43bdc9GurobiBackend.add_col: Implement
f42bd1cGenericBackend._test_add_col: New
d4eccf6GurobiBackend.add_variable: Support coefficients keyword
e9eb094GurobiBackend: Fix GRB_INFINITY confusion
bc739f3CVXOPTBackend: Don't test _test_solve because of #18572
9639a1fAdd _test_solve_trac_18572 (autogenerated)
c2bb379_test_solve_trac_18572: Replace float integers by integers to make test suitable for PPL backend
0d75d6eCVXOPTBackend: Disable _test_solve_trac_18572 because of #18572
b2b3031GLPKExactBackend: Adjust output
97c4542CoinBackend.add_col: Use all coefficients

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jun 25, 2016

comment:18

rebased on 7.3.beta5

@dimpase
Copy link
Member

dimpase commented Jun 25, 2016

Reviewer: Dima Pasechnik

@dimpase
Copy link
Member

dimpase commented Jun 25, 2016

comment:19

looks good to me

@vbraun
Copy link
Member

vbraun commented Jun 26, 2016

Changed branch from u/mkoeppe/backend_testsuite_failing_tests to 97c4542

@jdemeyer
Copy link

jdemeyer commented Aug 9, 2017

Changed commit from 97c4542 to none

@jdemeyer
Copy link

jdemeyer commented Aug 9, 2017

comment:21

Replying to @sagetrac-git:

d4eccf6 GurobiBackend.add_variable: Support coefficients keyword

Are there any plans to support this for other backends? I noticed that it's not even documented in add_variable.__doc__ for the Gurobi backend.

The interface between backends should be consistent, so even just raise NotImplementedError("the foo backend does not support the 'coefficients' argument") in other backends would be good to have. What do you think?

@dimpase
Copy link
Member

dimpase commented Aug 9, 2017

comment:22

IMHO different backends might have different features, which might be impossible to have for each backend.

Only if there are multiple backend supporting this, we can think of adding it to the generic backend, with the default implementation as you propose.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Aug 9, 2017

comment:23

Replying to @jdemeyer:

Replying to @sagetrac-git:

d4eccf6 GurobiBackend.add_variable: Support coefficients keyword

Are there any plans to support this for other backends? I noticed that it's not even documented in add_variable.__doc__ for the Gurobi backend.

Yes, there is a plan, see #20324.

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

4 participants