[Bridges] change graph to favor Constraint bridges over Variable bridges

Author: odow

src/Bridges/debug.jl

@@ -664,7 +664,7 @@ function print_active_bridges(
             # The constraint can be both variable bridged and constraint
             # bridged. Which is cheaper?
             v_node = b.variable_node[(S,)]
-            if bridging_cost(b.graph, v_node) <= bridging_cost(b.graph, c_node)
+            if MOI.Bridges.is_variable_edge_best(b.graph, v_node)
                 return print_active_bridges(io, b, S, offset)
             end
         else

src/Bridges/graph.jl

@@ -330,7 +330,22 @@ constrained on creation, or as a free variable followed by a constraint.
 """
 function is_variable_edge_best(graph::Graph, node::VariableNode)
     _compute_bellman_ford(graph)
-    return graph.variable_dist[node.index] == _dist(graph, node)
+    # This is the cost of adding a constrained variable
+    dist_as_variable = graph.variable_dist[node.index]
+    # This is the cost of adding the constraint, if we were to add it.
+    dist_as_constraint = INFINITY
+    # If free variables are bridged but the functionize bridge was not
+    # added, constraint_node is `ConstraintNode(INVALID_NODE_INDEX)`.
+    constraint_node = graph.variable_constraint_node[node.index]
+    if constraint_node.index != INVALID_NODE_INDEX
+        dist_as_constraint = _dist(graph, constraint_node)
+        if dist_as_constraint != INFINITY
+            dist_as_constraint += graph.variable_constraint_cost[node.index]
+        end
+    end
+    # We should prefer the variable bridge ONLY if the cost is strictly less
+    # than bridging via constraints.
+    return dist_as_variable < dist_as_constraint
 end
 
 function _updated_dist(

src/Bridges/lazy_bridge_optimizer.jl

@@ -293,8 +293,17 @@ function node(
             )
         end
     else
-        # We add `+1` as we treat constrained variables as constraints.
-        set_variable_constraint_node(b.graph, variable_node, node(b, F, S), 1)
+        # Add a node to the graph so we can convert a constrained variable into
+        # a free variable plus a constraint. In MOI v1.50 and earlier, the cost
+        # associated with this node was +1. However, practice has shown that we
+        # want to encourage constraint bridges over variable bridges, and
+        # penalizing the switch from constrained variable to constraint with an
+        # additional +1 meant we often used a bridge like Variable.Vectorize.
+        # With +0 here, it's now better to use Constraint.Vectorize.
+        #
+        # For more details, see:
+        # https://github.com/jump-dev/ParametricOptInterface.jl/issues/201
+        set_variable_constraint_node(b.graph, variable_node, node(b, F, S), +0)
     end
     for (i, BT) in enumerate(b.variable_bridge_types)
         if Variable.supports_constrained_variable(BT, S)

test/Bridges/General/test_bridge_optimizer.jl

@@ -1240,6 +1240,16 @@ function test_issue_2452_multiple_variable_bridges()
     MOI.add_constraint(src, x, MOI.LessThan(1.0))
     c = MOI.add_constraint(src, 2.0 * x, MOI.EqualTo(3.0))
     dest = MOI.instantiate(Model2452{Float64}; with_bridge_type = Float64)
+    # Remove constraint bridges so that we do
+    # x in LessThan -> [y] in Nonpositives, y:=x-1, [z] in Nonnegatives, z:=-y
+    MOI.Bridges.remove_bridge(
+        dest,
+        MOI.Bridges.Constraint.LessToGreaterBridge{Float64},
+    )
+    MOI.Bridges.remove_bridge(
+        dest,
+        MOI.Bridges.Constraint.NonposToNonnegBridge{Float64},
+    )
     index_map = MOI.copy_to(dest, src)
     set = MOI.get(dest, MOI.ConstraintSet(), index_map[c])
     @test set == MOI.EqualTo(3.0)
@@ -1264,13 +1274,11 @@ function test_2452()
     @test MOI.get(dest, MOI.ConstraintSet(), index_map[c]) == set
     @test ≈(
         MOI.get(dest.model, MOI.ConstraintFunction(), ci),
-        MOI.Utilities.operate(vcat, Float64, -1.0 + 2.0 * y),
+        MOI.Utilities.operate(vcat, Float64, -3.0 + 2.0 * y),
     )
     @test MOI.get(dest.model, MOI.ConstraintSet(), ci) == MOI.Zeros(1)
-    @test_throws(
-        MOI.SetAttributeNotAllowed,
-        MOI.set(dest, MOI.ConstraintSet(), index_map[c], set),
-    )
+    MOI.set(dest, MOI.ConstraintSet(), index_map[c], set)
+    @test MOI.get(dest.model, MOI.ConstraintSet(), ci) == MOI.Zeros(1)
     return
 end
 

test/Bridges/General/test_lazy_bridge_optimizer.jl

@@ -1306,6 +1306,11 @@ MOI.Utilities.@model(
 
 function _test_constrained_variables_in_RSOC(T)
     bridged = MOI.Bridges.full_bridge_optimizer(NoRSOCModel{T}(), T)
+    # Remove the constraint bridge to force via Variable bridges.
+    MOI.Bridges.remove_bridge(
+        bridged,
+        MOI.Bridges.Constraint.RSOCtoSOCBridge{T},
+    )
     @test MOI.supports_constraint(
         bridged,
         MOI.VectorOfVariables,

test/Utilities/test_copy.jl

@@ -482,7 +482,7 @@ function test_create_variables_using_supports_add_constrained_variable()
           Type[MOI.Nonpositives, MOI.Zeros, MOI.Nonnegatives]
     @test MOI.supports_add_constrained_variables(bridged_dest, MOI.Nonnegatives)
     @test MOI.get(bridged_dest, MOI.VariableBridgingCost{MOI.Nonnegatives}()) ==
-          2.0
+          1.0
     @test MOI.supports_constraint(
         bridged_dest,
         MOI.VectorOfVariables,
@@ -546,7 +546,7 @@ function test_create_variables_using_supports_add_constrained_variable()
     @test MOI.get(
         bridged_dest,
         MOI.VariableBridgingCost{MOI.GreaterThan{Float64}}(),
-    ) == 1.0
+    ) == 0.0
     @test MOI.get(
         bridged_dest,
         MOI.ConstraintBridgingCost{MOI.VariableIndex,MOI.GreaterThan{Float64}}(),
@@ -560,7 +560,7 @@ function test_create_variables_using_supports_add_constrained_variable()
     @test MOI.get(
         bridged_dest,
         MOI.VariableBridgingCost{MOI.LessThan{Float64}}(),
-    ) == 1.0
+    ) == 0.0
     @test MOI.get(
         bridged_dest,
         MOI.ConstraintBridgingCost{MOI.VariableIndex,MOI.LessThan{Float64}}(),