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

Reconsider supports of constraint attributes in bridges #1993

Open
odow opened this issue Sep 11, 2022 · 3 comments
Open

Reconsider supports of constraint attributes in bridges #1993

odow opened this issue Sep 11, 2022 · 3 comments
Labels
Submodule: Bridges About the Bridges submodule Type: Bug
Milestone

Comments

@odow
Copy link
Member

odow commented Sep 11, 2022

See discussion in #1992, and these tests:

function test_Issue1992_supports_ConstraintDualStart_VariableIndex()
# supports should be false
model = MOI.Bridges.full_bridge_optimizer(_Issue1992(false), Float64)
x, _ = MOI.add_constrained_variables(model, MOI.Nonpositives(1))
c = MOI.add_constraint(model, MOI.VectorOfVariables(x), MOI.Nonnegatives(1))
@test !MOI.supports(model, MOI.ConstraintDualStart(), typeof(c))
# supports should be true
model = MOI.Bridges.full_bridge_optimizer(_Issue1992(true), Float64)
x, _ = MOI.add_constrained_variables(model, MOI.Nonpositives(1))
c = MOI.add_constraint(model, MOI.VectorOfVariables(x), MOI.Nonnegatives(1))
# !!! warning
# This test is broken with a false negative. See the discussion in
# PR#1992.
@test_broken MOI.supports(model, MOI.ConstraintDualStart(), typeof(c))
return
end
function test_bridge_supports_issue_1992()
inner = MOI.Utilities.UniversalFallback(MOI.Utilities.Model{Float64}())
model = MOI.Bridges.Variable.NonposToNonneg{Float64}(inner)
x = MOI.add_variable(model)
c = MOI.add_constraint(
model,
MOI.VectorOfVariables([x]),
MOI.Nonpositives(1),
)
# !!! warning
# This test is broken with a false negative. (Getting and setting the
# attribute works, even though supports is false) See the discussion in
# PR#1992.
@test_broken MOI.supports(model, MOI.ConstraintDualStart(), typeof(c))
@test MOI.get(model, MOI.ConstraintDualStart(), c) === nothing
MOI.set(model, MOI.ConstraintDualStart(), c, [1.0])
@test MOI.get(model, MOI.ConstraintDualStart(), c) == [1.0]
return
end

The issue is that the bridge/model may support an attribute for a variable-in-set constraint if it was added via add_constrained_variable(s) but not via add_constraint (or vice versa). However, we have no way of telling which way the constraint was added based on the type alone.

We can either default to &&, in which case we may return a false negative (the model supports the attribute but supports returns false) or ||, in which case we may return a false positive (the model supports the attribute for one of the ways, but we added it the other way). It's not obvious which is better, but our tests are passing with the current implementation, so we should wait for a solver to complain before reconsidering.

@odow odow added Type: Bug Submodule: Bridges About the Bridges submodule labels Sep 11, 2022
@odow odow added this to the v1.x milestone Sep 13, 2022
@odow
Copy link
Member Author

odow commented Sep 22, 2023

I think we should close this. I can't see a way to fix this in the current form. There's a note in #2180, but that requires a much larger discussion that's more than this specific issue. (And I don't really want to go there.)

It's also a very minor issue that, from what I can tell, has never impacted a user in the wild.

@blegat
Copy link
Member

blegat commented Sep 22, 2023

We should at least document what it does return then. As per the doc, supports normally only has false positive.
From reading #1992, it seems, in order to fix an issue with Ipopt, it's now a mix of false positive and false negative which is in contradiction with supports. I should take a closer look, it's not urgent but I'd rather keep that issue open.

@odow
Copy link
Member Author

odow commented Nov 13, 2023

Trying #2294 just caused a bunch of solvers to fail tests:

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Submodule: Bridges About the Bridges submodule Type: Bug
Development

No branches or pull requests

2 participants