-
Notifications
You must be signed in to change notification settings - Fork 94
[Bridges] fix deleting variable with constraint bridges #2818
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
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
The underlying cause is the same as #2696 We don't really have a chain-of-custody to discriminate constraints that were added by the user at the top level from constraints added by bridges, unless we walk all the bridges to check. julia> MOI.Utilities.@model(
Model2817b,
(),
(MOI.LessThan,),
(),
(),
(),
(MOI.ScalarAffineFunction,),
(),
()
);
julia> function MOI.supports_constraint(
::Model2817b,
::Type{MOI.VariableIndex},
::Type{S},
) where {S<:MOI.AbstractScalarSet}
return false
end
julia> model = MOI.instantiate(Model2817b{Float64}; with_bridge_type = Float64);
julia> y, _ = MOI.add_constrained_variables(model, MOI.Nonpositives(1));
julia> ci = MOI.ConstraintIndex{MOI.VariableIndex,MOI.LessThan{Float64}}(1)
MathOptInterface.ConstraintIndex{MathOptInterface.VariableIndex, MathOptInterface.LessThan{Float64}}(1)
julia> MOI.is_valid(model, ci)
true |
Okay. I think that's everything passing tests at least. Now to document what exactly I did... |
Because of the false positive potential in | ||
`is_valid(::AbstractBridgeOptimizer, MOI.ConstraintIndex)`, we need to ensure | ||
that we delete constraints only if they were not added by a different constraint |
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.
What do you mean by different bridge ? It seems to me that if a bridge tries to delete a variable, now it won't delete any constraint created on that variable. So we are kind of assuming that the bridges can't rely on the fact that when they delete variables, the constraint on this variable won't be delete automatically, while the user can expect this behavior
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.
Hmm. Maybe I need to check this. I thought if a bridge deletes a variable it will, by the logic in this function, delete the related constraints.
I think we should also clarify in the docstring of |
Co-authored-by: Benoît Legat <[email protected]>
I don't think this is necessary. Do you have an example that fails? If a bridge has, for example, added a binary variable, the ZeroOne constraint can't exist if the variable has been deleted. |
Confirmed that it's not necessary, and I've added a test for it. |
Closes #2817
I don't think this is the proper fix. But passes the test. I want to run
solver-tests
with it if CI is greenhttps://github.com/jump-dev/MathOptInterface.jl/actions/runs/17026926756