coin-or / python-mip

Python-MIP: collection of Python tools for the modeling and solution of Mixed-Integer Linear programs
Eclipse Public License 2.0
516 stars 90 forks source link

Consider constraints with empty linexpr correctly #237

Closed sebheger closed 5 months ago

sebheger commented 2 years ago

Fixes #213

sebheger commented 2 years ago

@h-g-s @tuliotoffolo

CLAassistant commented 2 years ago

CLA assistant check
All committers have signed the CLA.

tuliotoffolo commented 2 years ago

Hi @sebheger, thank you for your contribution. We're now only waiting for you to agree with the Contributor License Agreement (CLA) to merge your pull request.

sebheger commented 2 years ago

@tuliotoffolo Resigned CLA.

sebheger commented 2 years ago

I recognized that the CI is not including gurobi - and my test cases fail on gurobi. I hadn't installed gurobi on the system where i did the fix.

It says that my infeasible case is unbounded after being solved with gurobi.

sebheger commented 2 years ago

Ok, after debugged the gurobi part:


 if status == 4:  # INF_OR_UNBD
     return OptimizationStatus.UNBOUNDED

This is an additional thing to fix. See

In order to determine if the model is infeasible or unbounded, you should set the DualReductions parameter to 0, call reset on the model, and optimize once again. The optimization status that is returned should be either INFEASIBLE or UNBOUNDED.

https://support.gurobi.com/hc/en-us/articles/4402704428177-How-do-I-resolve-the-error-Model-is-infeasible-or-unbounded-

tuliotoffolo commented 2 years ago

Indeed... I'll talk to @h-g-s to discuss how we will approach this issue. We may need a "INF_OR_UNBD" status in Python-MIP.

sebheger commented 2 years ago

Indeed... I'll talk to @h-g-s to discuss how we will approach this issue. We may need a "INF_OR_UNBD" status in Python-MIP.

I would suggest following gurobis suggested approach. To reset the model, set presolves dual_reductions to 0 and "optimize" again. Checking infeasibility for MIP is in most cases done comparably quickly (as long as you don't to know hints about WHY it is infeasible). I guess this gurobi INF_OR_UNBD thing is originated by weak duality. So just knowing that the dual is infeasible, could be both possible status for the primal.

sebheger commented 2 years ago

@tuliotoffolo Take a look. Did some beautifications is cbc & tests and introduced a fix for the gurobi status.

tuliotoffolo commented 2 years ago

Thanks, @sebheger. Looks very nice! :) There's only one thing I would change: I don't think that disabling dual reductions and re-optimizing 'automatically' is the way to go, since depending on the model this may be undesirable. In fact, that's probably the reason why Gurobi does not do so automatically. I will further analyze it, but so far I personally prefer to return "INF_OR_UNBD" and create a function containing your code so that the user can convert "INF_OR_UNBD" into "INFEASIBLE" or "UNBOUNDED".

sebheger commented 2 years ago

@tuliotoffolo I introduced new OptimizationStatus as suggested. I put the code for automatic re-optimization into a comment for the moment. There would be some clarfications needed where to place a method to reoptimize, as the status of an optimization run is only persisted in the Model, but not in GurobiSolver.

sebheger commented 2 years ago

After I had a look at #184 I am not sure if my fixes solve the overall problem. This should be reported to CBC. It should be ideally supported on CBC side to correctly add empty rows (in the same way as supporting empty colums), which could be later modified by the columns parameter of the variable.

BramVanroy commented 10 months ago

@sebheger Thank you for your work on this. As far as I understand, you indicate that this is not a full solution to the problem and something would need to change on the side of cbc. Has this be reported to them somewhere?

That being said, I assume that this PR already deals with some issues - hopefully also mine :-) So is there any chance of getting this PR merged and pushed to PyPi? cc @tuliotoffolo

tuliotoffolo commented 5 months ago

Revisiting this pull request: @sebheger, is there any reason for not merging it?

sebheger commented 5 months ago

@tuliotoffolo Feel free to merge it, it solves #237 by a hack. I was hoping it also solves #184, but the segfault still exstists, because both issues arise from an problematic behaviour of the C-api to cbc.

tuliotoffolo commented 5 months ago

Thanks, @sebheger. Indeed, it solves #237 by a hack. I'll take a look at CBC's C api and see if I can help there.