opencobra / optlang

optlang - sympy based mathematical programming language
http://optlang.readthedocs.org/
Apache License 2.0
252 stars 52 forks source link

Add MIP check in gurobi interface and update shadow_prices and reduced_cost to throw proper error if MIP #259

Closed Palaract closed 9 months ago

Palaract commented 10 months ago

Because MIPs lose convexity, shadow prices and reduced costs aren't available for them until the model was relaxed (variables transformed and general constraints removed) or fixed (fixes integer values to the best solution found). That means that it is necessary for the functions _get_reduced_costs and _get_shadow_prices to not only check if the problem is a pure integer problem but also if the model is a mixed integer problem. For that, another property is_mip was introduced.

Palaract commented 9 months ago

Nice, looks pretty good. Is there are reason for the new is_mip instead of just overwriting is_integer? They seem to be doing the same thing (check whether it is an integer problem).

I thought of that, but decided that it is a semantic problem. is_integer returns a boolean so it just decides between an integer problem and any other problem. Even if is_integer would be to able to determine if a problem is a MIP, it would be semantically incorrect to state that an integer problem is equal to a mixed integer problem if True was returned.

I can see that this may be nitpicky from my side, so I will just leave the choice up to you after reading this and implement it according to your instruction.

cdiener commented 9 months ago

Oh yeah, what I meant is that is_integer is a misnomer since it actually just checks whether there is at least one integer variable (so whether it is a MIP). I agree it's not well named and we could rename it in another PR to maybe has_integers or is_mip like you propose. Or are you saying that there is a difference between a linear problem with one or more integer variables (but not all of them) and a MIP? Either way not a big deal here. If we add your is_mip I would probably want to replace is_integer with it in all other interfaces in a later PR, so the end result would be the same. So go ahead with whatever you prefer for now.

cdiener commented 9 months ago

Merging for now. Will need a follow up to rename is_integer -> is_mip and a new major release.

Palaract commented 9 months ago

Sorry for not informing you about my absence, I am now back to business and I'm thankful to you that my PR was merged. I just wanted to inform you about my thought process, so that we can be on the same page and so that I also may be corrected if I have any mistakes there:

We actually have three kinds of problems here. The first problem is the standard linear programming problem which doesn't have any constraints regarding the integrality of its variables, this case is actually considered by cobrapy right now. The second problem is the integer problem which requires all of its variables to be integers. The last is a mixed integer problem which only requires some of its variables to be integers. The difficulty here is that the assumption of is_integer() is actually correct if one would want to see if something is a mixed integer problem, but in the specific case of Gurobi, a model can be a mixed integer problem even though it doesn't have any integer variables, how is that?

If I add a general constraint, like a piecewise linear constraint in Gurobi, the convexity of the model is endangered and for that case, binary auxiliary variables as well as so called SOS-variables are introduced. At least the auxiliary variables are integer variables in nature, but somehow they don't classify as integer variables, because they are part of a general constraint.

How do we fix that now in Optlang and CobraPy? I do think that renaming everything in other instances is not necessary, because other solvers may not even have the option to incorporate general constraints in that manner. Also is_integer() is still a suitable name if one makes clear that this means that more than zero integer variables are present, because depending on the amount of variables it may as well be a mixed integer problem as it may be a pure integer problem. I would regard Optlang as fixed now, because I actually have no idea how other solver interfaces tackle the problem and this case is rather specific for Gurobi.

The next step for me would be to look into Cobrapy and see if it actually draws the changes from this PR into optlang now. If it does I can make proper error handling there which catches the general ValueError now thrown and tries to fall back to a behaviour where it doesn't assume shadow prices or reduced costs.

Sorry for all the text, working with you is a pleasure and I hope your move went well!