PyPSA / linopy

Linear optimization with N-D labeled arrays in Python
https://linopy.readthedocs.io
MIT License
165 stars 48 forks source link

Constraint with binary variable not being respected? #53

Closed prakaa closed 1 year ago

prakaa commented 2 years ago

Hi there,

I'm comparing a few packages for solving a MILP with binary variables: https://github.com/prakaa/Battery-Optimisation-Benchmarking

I have formulated the problem to respect linopy's requirements (e.g. all variables on LHS, minimisation only) and have been able to find an appropriate solution in JuMP, pyomo and python-mip. However the solution does not appear to be right for linopy: https://github.com/prakaa/Battery-Optimisation-Benchmarking/blob/master/battery_optimisation_benchmarking/python/linopy.ipynb

Specifically, there is a pair of constraints that contain a binary variable ("charge state") that should ensure that the modelled battery cannot discharge and charge at the same time. This appears to be respected in other packages (making me think the model formulation is OK), but does not appear to be in linopy.

Have I made a mistake in formulating the model using linopy, or is this a bug?

FabianHofmann commented 2 years ago

Hey @prakaa it is hard to tell just from code. One thing I spotted:

The line

        discharge_mw - power_cap * (1 - charge_state) <= 0,

does not work (and acutally linopy should raise an error here). Constants should always be on the right hand side, e.g.

        discharge_mw + power_cap * charge_state <= power_cap,

And a tip: when initializing the model could you can set the argument force_dim_names to true. This makes sure that no extra dimensions are created when aligning variables. But from what I see it looks good.

prakaa commented 2 years ago

Thanks @FabianHofmann, that fixed the binary variable issue. Unfortunately I'm still not getting an objective comparable to those I get from pyomo/mip/JuMP. Not sure why, the LP file looks OK...

That error is not raised unless a non-constant is on the RHS - if the RHS is 0, then the error is not raised. So in this case the error was not raised.

Happy to suggest a change via PR, but I think that requirement re constraints could be clearer in the docs on this page https://linopy.readthedocs.io/en/latest/creating-constraints.html. In my mind that change to the docs would include:

FabianHofmann commented 2 years ago

A PR with your points would be fantastic. The only thing I can offer to solve that is to that you send me you nb with the underlying data. Otherwise its hard to debug...

coroa commented 1 year ago

Oh, i didn't know there was an issue about this already. Well, the problem is #57 .

prakaa commented 1 year ago

Thanks @coroa. @FabianHofmann sorry I am a bit snowed under, will hopefully get to that PR in the next couple of weeks.

FabianHofmann commented 1 year ago

This seems to be solved in the current master. I you still encounter problems please reopen :)