Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix missing starts #242

Merged
merged 1 commit into from
Apr 5, 2022
Merged

Fix missing starts #242

merged 1 commit into from
Apr 5, 2022

Conversation

aphi
Copy link
Contributor

@aphi aphi commented Jan 30, 2022

Fix for this issue: #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
Copy link

CLAassistant commented Jan 30, 2022

CLA assistant check
All committers have signed the CLA.

@aphi
Copy link
Contributor Author

aphi commented Jan 30, 2022

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

@sebheger
Copy link
Collaborator

sebheger commented Feb 9, 2022

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

  • @h-g-s @tuliotoffolo It would definitely make sense to hand over the start as Dict[Var,float] or maybe as a set of Tuples instead of a list to avoid duplicates (I don't know what will happen if you have the same variables twice in the list with different start values - maybe the last one winning or a crash?)
  • As far as I could remind gurobi could handle an incomplete start (to complete it). So the provided code should be moved to cbc interface.
  • Actually, one or more test cases should be added

@aphi
Copy link
Contributor Author

aphi commented Mar 1, 2022

@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
Copy link
Collaborator

sebheger commented Mar 23, 2022

@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)

@sebheger sebheger added this to the Release 1.14.0 milestone Mar 23, 2022
@aphi
Copy link
Contributor Author

aphi commented Mar 30, 2022

@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).

@sebheger sebheger merged commit 8729059 into coin-or:master Apr 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants