Opened 4 years ago

Closed 3 years ago

Last modified 2 years ago

#20424 closed defect (fixed)

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

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: 97c4542 (Commits) Commit:
Dependencies: #20323,#20600,#20325 Stopgaps:

Description (last modified by mkoeppe)

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. ​ http://trac.sagemath.org/ticket/13148#comment:2). Some backends assign names to constraints automatically.

Change History (23)

comment:1 Changed 4 years ago by mkoeppe

  • Cc dimpase vdelecroix added
  • Description modified (diff)

comment:2 Changed 4 years ago by mkoeppe

  • Branch set to u/mkoeppe/backend_testsuite_failing_tests

comment:3 Changed 4 years ago by mkoeppe

  • Commit set to e5e6227255cc92fe61664eeabc4320be35ad6933

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

comment:4 Changed 4 years ago by git

  • Commit changed from e5e6227255cc92fe61664eeabc4320be35ad6933 to af1e88518622375bfb46d37070445d42176e5c1a

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

comment:5 Changed 4 years ago by mkoeppe

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

comment:6 Changed 4 years ago by git

  • Commit changed from af1e88518622375bfb46d37070445d42176e5c1a to a5a1c6098815a37b69bd4716d83a639ba35a7ba9

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

comment:7 Changed 4 years ago by mkoeppe

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

comment:8 Changed 3 years ago by dimpase

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

comment:9 Changed 3 years ago by mkoeppe

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

Last edited 3 years ago by mkoeppe (previous) (diff)

comment:10 Changed 3 years ago by git

  • Commit changed from a5a1c6098815a37b69bd4716d83a639ba35a7ba9 to bb7f5de0594b3381770039003ec763b901f1dc41

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

comment:11 Changed 3 years ago by mkoeppe

rebased on 7.2.beta6

comment:12 Changed 3 years ago by git

  • Commit changed from bb7f5de0594b3381770039003ec763b901f1dc41 to 07c9c5dd48f4911e1f249698c3c13892af4a06f1

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

comment:13 Changed 3 years ago by mkoeppe

  • Dependencies changed from #20323 to #20323,#20600,#20325
  • Status changed from new to needs_review

comment:14 Changed 3 years ago by mkoeppe

  • Cc jdemeyer chapoton nbruin added
  • Description modified (diff)
  • Summary changed from More tests for common MIP TestSuite to More tests for common MIP TestSuite: add_col, solve; some fixes for backends

comment:15 Changed 3 years ago by git

  • Commit changed from 07c9c5dd48f4911e1f249698c3c13892af4a06f1 to 1ce4eb632f0b86e2802e9bd5cbceab282513fe62

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

1ce4eb6CoinBackend.add_col: Use all coefficients

comment:16 Changed 3 years ago by mkoeppe

  • Description modified (diff)

comment:17 follow-up: Changed 3 years ago by git

  • Commit changed from 1ce4eb632f0b86e2802e9bd5cbceab282513fe62 to 97c4542ebaf4585cec9b2a3f56da4d32ab016def

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

comment:18 Changed 3 years ago by mkoeppe

rebased on 7.3.beta5

comment:19 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

looks good to me

comment:20 Changed 3 years ago by vbraun

  • Branch changed from u/mkoeppe/backend_testsuite_failing_tests to 97c4542ebaf4585cec9b2a3f56da4d32ab016def
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:21 in reply to: ↑ 17 ; follow-up: Changed 2 years ago by jdemeyer

  • Commit 97c4542ebaf4585cec9b2a3f56da4d32ab016def deleted

Replying to git:

d4eccf6GurobiBackend.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?

comment:22 Changed 2 years ago by dimpase

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.

comment:23 in reply to: ↑ 21 Changed 2 years ago by mkoeppe

Replying to jdemeyer:

Replying to git:

d4eccf6GurobiBackend.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.

Note: See TracTickets for help on using tickets.