-
Notifications
You must be signed in to change notification settings - Fork 25
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
Adding choco #413
Merged
Merged
Adding choco #413
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
All tests are succeeding now, also added pychoco to the github tests. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Adding support to Choco solver.
Also added some tests.
Compared to ortools, choco does not need transformation only_bv_implies, because it supports complete reification. However, I had to split the part where the bool var is sent to the left part and the boolexpr to the right part from only_bv_implies, creating a new transformation, i.e., only_bv_reifies, keeping in only_bv_implies only the part that the reification is split into 2 implications. We can also put the only_bv_reifies part inside the reify_rewrite function, which would make sense to me.
Choco also it supports reification in all constraints as after creating a constraint, this can be either posted or be reified. Thus all bool globals are included in reify_rewrite.
That is also why I renamed the function _post_constraint() to _get_constraint(), as I use it also to get constraints and reify them. The actual post function is called in add.
Also, note that Choco has many restrictions on the parameters each constraint accepts. Some constraints accept only variables in some arguments, so if we have an integer we must create a variable that will be equal to that integet in order to use this constraint.
These checks and creation of variables is currently in _get_constraint(). Should it be in a different transformation? Not sure, it is not generic enough for this.
In addition to these restrictions, choco needs all num expressions to be in canonical form (variables left side, constant right side), thus a new transformation is introduced to deal with it, using a part from linearize. Also, changed linearize to called
this transformation inside instead of reusing the same code.
Also, currently the "implies" constraints and the reification are not fully supported in the python API, so, until they are added in the python API which will happen soon, they are added with different ways to the solver.
Finally, the python API for Choco currently has a bug on the time limit (which will be soon resolved) and thus it is not supported currently.