cvxpy/tests: Add test for infeasible boolean problem #1705
Conversation
with one method for every solver that can handle integer variables? |
240d4dd to
6728766
Compare
|
Does this problem have a clear qualitative property that existing test MILPs do not? An example of such a property is "the root LP relaxation is infeasible". If this problem does introduce a clear qualitative property compared to existing test cases, then yes, please add the test for every solver that supports MILP. |
|
The new tests are failing in several environments, e.g., https://github.com/cvxpy/cvxpy/runs/5549355220?check_suite_focus=true#step:6:1367 https://github.com/cvxpy/cvxpy/runs/5549355386?check_suite_focus=true#step:6:1375 |
Yes, this is working as intended. The CBC solver is broken until a working version of CyLP with coin-or/CyLP#150 is released and deployed. |
I'd say clear enough, it looks like it is the first tested example of an infeasible ILP for which ILP preprocessing does not already lead to an infeasible relaxation, at least for CBC's preprocessor, with default settings. So I'm going to add it to the other solvers. |
…also with other MILP solvers
|
@mkoeppe we don't want to merge changes that result in failing tests. I suggest the new CBC test be modified so it expects an assertion failure when CyLP is at or below a certain version and raises a warning to let the user know that situation was encountered. The test should expect no assertion failure when CyLP is a sufficiently high version. Would the tests pass if this change was also made: https://github.com/cvxpy/cvxpy/pull/1703/files? If so, please make those changes in this PR. (We could infer from continuous integration results before that change to infer that the intended code-path was hit.) |
|
@mkoeppe this looks good to me. You might want to change how CBC status codes are handled per the discussion in d390389#commitcomment-68775673, which came up after PR #1703 was merged. Barring any unforeseen developments, I'll approve this PR once you mark it as "ready for review." |
I think this needs more discussion, so it's best to leave it for a separate PR. |
|
In the CI configurations, it now skips the new test as intended. I've tested this version locally with https://trac.sagemath.org/ticket/31962 (https://git.sagemath.org/sage.git/commit/?id=6daae939c9ee40f8fa5643a85dc655c067a21525), and it runs and passes the test. |
Agreed. |
There was a problem hiding this comment.
LGTM. I'll wait to merge until the end of the day in case @SteveDiamond or @akshayka want to review it.
|
LGTM |
…github.com:cvxpy/cvxpy * 'master' of github.com:QiuWJX/cvxpy: changed the copyright and docstring create resources folder in codes undid changes standardized the code style created a folder called ./cvxpy/tests/resources/ Add unit test for gurobi `model.write()` Add 'save_file' kwargs for GUROBI. - use Gurobi.write(save_file) to save the model file. - cplex and mosek have the similar functionality. cvxpy/tests: Add test for infeasible boolean problem (#1705) Handle new CyLP statuses from coin-or/CyLP#150 (#1707) * 'master' of github.com:cvxpy/cvxpy: SCIP: add version requirement (< 4.0.0) and fix solve_via_data to eliminate certain error messages update error message for mixed-integer problems (#1738) remove out of date news fix TypeError in GLOP and PDLP interfaces (#1736) Propagate today's documentation improvements from release/1.2.x (#1733) Cylp update (#1727) Add 'save_file' kwargs for GUROBI. (#1720) pip install cvxpy[GLOP,XPRESS] (#1719) Fix #1714 cvxpy/tests: Add test for infeasible boolean problem (#1705) Handle new CyLP statuses from coin-or/CyLP#150 (#1707)
Description
Please include a short summary of the change.
We add a new test
StandardTestLPs.test_mi_lp_5. This test fails with CBC because of coin-or/CyLP#81#1703 attempts to fix it.
Issue link (if applicable):
https://trac.sagemath.org/ticket/31962#comment:48
Type of change
Contribution checklist