From 2aca294b40cc79667b8fafe176b19a298fcad210 Mon Sep 17 00:00:00 2001 From: odow Date: Wed, 27 Sep 2023 14:25:01 +1300 Subject: [PATCH 1/2] [Bridges] change supports for AbstractConstraintAttribute to OR formulation --- src/Bridges/bridge_optimizer.jl | 57 +++++++++++---------------------- 1 file changed, 18 insertions(+), 39 deletions(-) diff --git a/src/Bridges/bridge_optimizer.jl b/src/Bridges/bridge_optimizer.jl index 8857c2c809..47223888e1 100644 --- a/src/Bridges/bridge_optimizer.jl +++ b/src/Bridges/bridge_optimizer.jl @@ -1442,49 +1442,28 @@ function MOI.supports( # This function is slightly confusing, because we need to account for # the different ways in which a constraint might be added. if F == MOI.Utilities.variable_function_type(S) - # These are VariableIndex and VectorOfVariable constraints. - if is_bridged(b, S) - # If S needs to be bridged, it usually means that either there is a - # variable bridge, or that there is a free variable followed by a - # constraint bridge (i.e., the two cases handled below). - # - # However, it might be the case, like the tests in - # Variable/flip_sign.jl, that the model supports F-in-S constraints, - # but force-bridges S sets. If so, we might be in the unusual - # situation where we support the attribute if the index was added - # via add_constraint, but not if it was added via - # add_constrained_variable. Because MOI lacks the ability to tell - # which happened just based on the type, we're going to default to - # asking the variable bridge, at the risk of a false negative. - if is_variable_bridged(b, S) - bridge = Variable.concrete_bridge_type(b, S) - return MOI.supports(recursive_model(b), attr, bridge) - else - bridge = Constraint.concrete_bridge_type(b, F, S) - return MOI.supports(recursive_model(b), attr, bridge) - end - else - # If S doesn't need to be bridged, it usually means that either the - # solver supports add_constrained_variable, or it supports free - # variables and add_constraint. - # - # In some cases, it might be that the solver supports - # add_constrained_variable, but ends up bridging add_constraint. - # Because MOI lacks the ability to tell which one was called based - # on the index type, asking the model might give a false negative - # (we support the attribute via add_constrained_variable, but the - # bridge doesn't via add_constraint because it will be bridged). - return MOI.supports(b.model, attr, MOI.ConstraintIndex{F,S}) + # supports should return true if it is possible to support the + # attribute at any point, even if it does not currently support it. + # + # So check if the model supports it if we added via `add_constraint` and + # didn't bridge: + ret = MOI.supports(b.model, attr, MOI.ConstraintIndex{F,S}) + # And if we added it via a variable bridge: + if is_variable_bridged(b, S) + bridge = Variable.concrete_bridge_type(b, S) + ret |= MOI.supports(recursive_model(b), attr, bridge) end - else - # These are normal add_constraints, so we just check if they are - # bridged. + # And if we added it via a constraint bridge: if is_bridged(b, F, S) bridge = Constraint.concrete_bridge_type(b, F, S) - return MOI.supports(recursive_model(b), attr, bridge) - else - return MOI.supports(b.model, attr, MOI.ConstraintIndex{F,S}) + ret |= MOI.supports(recursive_model(b), attr, bridge) end + return ret + elseif is_bridged(b, F, S) + bridge = Constraint.concrete_bridge_type(b, F, S) + return MOI.supports(recursive_model(b), attr, bridge) + else + return MOI.supports(b.model, attr, MOI.ConstraintIndex{F,S}) end end From 63d68fbc94e8ae66367b0eb3909dc8fca3b8b004 Mon Sep 17 00:00:00 2001 From: Oscar Dowson Date: Wed, 27 Sep 2023 15:33:21 +1300 Subject: [PATCH 2/2] Update bridge_optimizer.jl --- test/Bridges/bridge_optimizer.jl | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/test/Bridges/bridge_optimizer.jl b/test/Bridges/bridge_optimizer.jl index e8f9c0e00c..d2eba78b91 100644 --- a/test/Bridges/bridge_optimizer.jl +++ b/test/Bridges/bridge_optimizer.jl @@ -903,10 +903,7 @@ function test_Issue1992_supports_ConstraintDualStart_VariableIndex() 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)) + @test MOI.supports(model, MOI.ConstraintDualStart(), typeof(c)) return end @@ -919,11 +916,7 @@ function test_bridge_supports_issue_1992() 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.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]