-
Notifications
You must be signed in to change notification settings - Fork 5
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
Make shadow set optional #154
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #154 +/- ##
=======================================
Coverage ? 86.11%
=======================================
Files ? 17
Lines ? 1520
Branches ? 0
=======================================
Hits ? 1309
Misses ? 211
Partials ? 0 ☔ View full report in Codecov by Sentry. |
@@ -79,7 +79,7 @@ function Bonobo.get_branching_variable( | |||
Bonobo.get_branching_indices(tree.root), | |||
) | |||
status = check_feasibility(branching.bounded_lmo) | |||
if status == OPTIMALS |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this was always returning false then??
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rather, it threw an error because the compiler did not know the flag.
In the MOI case, we have a separate strong branching function, which is the same as before.
Seems one on the tests should be loosened to 1e-2
|
Yes. I think this might be due to SCIP not sticking to its own tolerances. |
The shadow set is useful if the LMO is expensive to evaluate.
But for the SimpleBoundable LMO, calling the LMO can be faster than going through the shadow set and picking a best point from there.