Skip to content

Commit b94c326

Browse files
committed
[Bridges] change graph to favor Constraint bridges over Variable bridges
1 parent b5778d0 commit b94c326

File tree

3 files changed

+28
-4
lines changed

3 files changed

+28
-4
lines changed

src/Bridges/debug.jl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -664,7 +664,7 @@ function print_active_bridges(
664664
# The constraint can be both variable bridged and constraint
665665
# bridged. Which is cheaper?
666666
v_node = b.variable_node[(S,)]
667-
if bridging_cost(b.graph, v_node) <= bridging_cost(b.graph, c_node)
667+
if MOI.Bridges.is_variable_edge_best(b.graph, v_node)
668668
return print_active_bridges(io, b, S, offset)
669669
end
670670
else

src/Bridges/graph.jl

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -330,7 +330,22 @@ constrained on creation, or as a free variable followed by a constraint.
330330
"""
331331
function is_variable_edge_best(graph::Graph, node::VariableNode)
332332
_compute_bellman_ford(graph)
333-
return graph.variable_dist[node.index] == _dist(graph, node)
333+
# This is the cost of adding a constrained variable
334+
dist_as_variable = graph.variable_dist[node.index]
335+
# This is the cost of adding the constraint, if we were to add it.
336+
dist_as_constraint = INFINITY
337+
# If free variables are bridged but the functionize bridge was not
338+
# added, constraint_node is `ConstraintNode(INVALID_NODE_INDEX)`.
339+
constraint_node = graph.variable_constraint_node[node.index]
340+
if constraint_node.index != INVALID_NODE_INDEX
341+
dist_as_constraint = _dist(graph, constraint_node)
342+
if dist_as_constraint != INFINITY
343+
dist_as_constraint += graph.variable_constraint_cost[node.index]
344+
end
345+
end
346+
# We should prefer the variable bridge ONLY if the cost is strictly less
347+
# than bridging via constraints.
348+
return dist_as_variable < dist_as_constraint
334349
end
335350

336351
function _updated_dist(

src/Bridges/lazy_bridge_optimizer.jl

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -293,8 +293,17 @@ function node(
293293
)
294294
end
295295
else
296-
# We add `+1` as we treat constrained variables as constraints.
297-
set_variable_constraint_node(b.graph, variable_node, node(b, F, S), 1)
296+
# Add a node to the graph so we can convert a constrained variable into
297+
# a free variable plus a constraint. In MOI v1.50 and earlier, the cost
298+
# associated with this node was +1. However, practice has shown that we
299+
# want to encourage constraint bridges over variable bridges, and
300+
# penalizing the switch from constrained variable to constraint with an
301+
# additional +1 meant we often used a bridge like Variable.Vectorize.
302+
# With +0 here, it's now better to use Constraint.Vectorize.
303+
#
304+
# For more details, see:
305+
# https://github.com/jump-dev/ParametricOptInterface.jl/issues/201
306+
set_variable_constraint_node(b.graph, variable_node, node(b, F, S), +0)
298307
end
299308
for (i, BT) in enumerate(b.variable_bridge_types)
300309
if Variable.supports_constrained_variable(BT, S)

0 commit comments

Comments
 (0)