-
Notifications
You must be signed in to change notification settings - Fork 81
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
Run the lazy constraints callback at MIPNODE #427
Conversation
Executes the lazy constraints callback (if defined) when where=MIPNODE so that the callback is executed at fractional values. Added a test using callback_knapsack_model to ensure fractional solutions reach the callback.
585e84c
to
f9c620c
Compare
Hi @simonbowly, Yes, I think I originally only called it at MIPSOL because we didn't have a The JuMP documentation does suggest that it may be called at fractional nodes: Re the 0.10.0 stuff, yes, we're very close (https://github.com/jump-dev/JuMP.jl/milestone/12) to releasing a new version of JuMP. Gurobi 0.10 works with MathOptInterface 0.10, but JuMP 0.21 relies on MathOptInterface 0.9. Can you post a screenshot of the tests passing? We can't run Gurobi on CI due to licensing issues. |
p.s. if you want to chat about JuMP + Gurobi.jl, we're very keen to have some more official support from Gurobi on this. The licensing issue is a major pain point. |
This came up in the context of my academic work by the way. But I hadn't realised licensing was blocking CI for you - I'll email you directly about this as I'm sure we can sort something out! |
@blegat do you regard this as breaking? I guess if we push this out as 0.10.1, must users will be updating from JuMP 0.21 and Gurobi 0.9 to JuMP 0.22 and Gurobi 0.10.1 so it's a good time to do this. |
The other option, and one that we should encourage, is for users to write solver-specific callbacks whenever the MOI defaults aren't suitable: |
So I knew I'd gone over this at some point. My original implementation had it, but I removed it here: I don't remember the reasoning, but we do the same in CPLEX: What's the motivation for adding lazy constraints to fractional solutions? |
I see - this is for cases where knowledge of fractional solutions might allow the user to identify a necessary lazy constraint earlier in the solve, before an integer solution which violates it arises. In some sense it's equivalent to a user cut, but the behaviour can be slightly different. If only calling the lazy constraint callback at integer solutions is the MOI default, this is also fine, but the JuMP docs for solver-independent lazy constraint callbacks (with callback_node_status) seem to imply that fractional node relaxations should reach this callback. This does occur when using GLPK. I expected this to be consistent across solvers and assumed Gurobi needed to be brought into line with the others, sorry! |
Now I see that GLPK doesn't distinguish these cases in its own callback reason codes, and GLPK.jl has to check integrality to create the callback status, which would explain the difference. |
The operative word in the JuMP docs is "may". It's painfully difficult to ensure consistency across the solvers that implement callbacks. There's essentially no guarantee that JuMP's solver-independent callbacks behave the same across solvers. All it is is a nice syntax for some common callbacks; the actual behavior is solver-dependent. That said, I don't see a good reason not to call the lazy constraint at fractional nodes, especially since it's documented and we now have the status mechanism. I'll wait for @blegat to chime in, but this looks good to me. |
Indeed, we could also simply release v0.11.
I agree, it's also good to remove this difference with GLPK. |
Thanks @simonbowly. My email is [email protected] if you want to discuss the licensing issue. |
Executes the lazy constraints callback (if defined) when where=MIPNODE so that the callback is executed at fractional values. Added a test using callback_knapsack_model to ensure some fractional solutions reach the callback in this example.
Note: branched this from v0.9.14 as v0.10.0 forces a downgrade to JuMP 0.18