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
530 stars 92 forks source link

Fix missing starts #242

Closed aphi closed 2 years ago

aphi commented 2 years ago

Fix for this issue: https://github.com/coin-or/python-mip/issues/229

Context:

CBC officially expects MIP starts to be provided with all non-continuous variables specified. Although this may change in CBC v3 (see first item in changelog: https://github.com/coin-or/Cbc)

However python-mip documentation states "Only the main binary/integer decision variables which appear with non-zero values in the initial feasible solution need to be informed". This implies that zero-valued integer decision variables are the default and need not be supplied. The issue demonstrates that's not the case, as CBC complains.

I've tested this change and it resolves the issue. Odd behaviour can be replicated with partial starts in CBC with other MIPs. e.g. adding to example/knapsack.py this line m.start = [(x[0], 0)], the MIPStart will fail to find a starting solution (despite all zeros as a valid feasible solution) while under this PR it works as intended

Before Screenshot from 2022-01-31 02-50-32

After Screenshot from 2022-01-31 02-49-59 .

CLAassistant commented 2 years ago

CLA assistant check
All committers have signed the CLA.

aphi commented 2 years ago

Happy to modify the code to match a preferred codestyle - just let me know! 🙏

sebheger commented 2 years ago

@aphi Thanks for your contribution. I will raise three questions/remarks:

aphi commented 2 years ago

@sebheger Thanks for the feedback 🙏.

hand over the start as Dict[Var,float] or maybe as a set of Tuples instead of a list to avoid duplicates

  1. I agree with this. I think it would need to be a Dict[Var,float], since the set of tuples could still include the same Var twice. Will leave it for the package authors to decide on, but I don't think needs to be part of this PR.

provided code should be moved to cbc interface

  1. Good point - I moved it there in the latest commit.

test cases

  1. I'm not sure what test we can add specifically. To expose this issue, we'd want a test that fails if CBC rejects a valid mipstart, but the solver doesn't readily feed that situation back and it will continue without a mipstart.

At a minimum, we already have test/rcpsp_test.py and examples/tsp-mipstart which add a mip start and at least confirm no runtime errors in doing so.

sebheger commented 2 years ago

@aphi Looks fine so far, please check style with black.

As a test case, I would expect to add a really minimalistic example. Could image something like:

minimize x s.t. x + y = 1 x is binary y is continuous

Then set start only x=1. After optimization check that you receive two results (the start with x=1 and y=0 and the optimal x=0 and y=1). So this unit test should fail before your bug fix and success afterward.

Please squash commits and give the final commit a nice name (ideally linked to issue which is solved)

aphi commented 2 years ago

@sebheger Style now fixed with black, and I've squashed the commits.

After optimization check that you receive two results (the start with x=1 and y=0 and the optimal x=0 and y=1).

I haven't added that test case as this issue wouldn't be exposed by it. The same optimal solution result is still ultimately found regardless of what values are provided as a mipstart. This issue (#229) fixes mipstarts not being used by CBC to improve the search (unless all zero-valued variables are specified, which shouldn't be a condition).