_set_support refactor + warning on shared indicators

Author: dnguyen227

src/basicsteps.jl

@@ -21,8 +21,8 @@ function _check_global_constraint(model::JuMP.AbstractModel, cref)
               "cannot be intersected into a disjunction.")
     end
     supported = con isa JuMP.ScalarConstraint ||
-        (con isa JuMP.VectorConstraint && JuMP.moi_set(con) isa
-            Union{_MOI.Nonnegatives, _MOI.Nonpositives, _MOI.Zeros})
+        (con isa JuMP.VectorConstraint &&
+            _set_support(JuMP.moi_set(con)))
     supported || error("Global constraint `$cref` with set " *
         "`$(JuMP.moi_set(con))` cannot be intersected into a disjunction.")
     _check_expression(JuMP.jump_function(con))
@@ -67,8 +67,11 @@ function _check_basic_step_input(
     end
     all_indicators = [lv for d in disjunctions
                       for lv in JuMP.constraint_object(d).indicators]
-    allunique(all_indicators) || error(
-        "The `disjunctions` for a basic step share indicator variables.")
+    allunique(all_indicators) || @warn "The `disjunctions` for a basic " *
+        "step share indicator variables, so some product disjuncts are " *
+        "logically infeasible. Their indicators are forced off by the " *
+        "linking constraints, so results remain correct, but the " *
+        "product disjunction is larger than its pruned equivalent."
     indicator_set = Set(all_indicators)
     input_set = Set(disjunctions)
     for (idx, disj) in _disjunctions(model)
@@ -150,8 +153,10 @@ function _add_product_disjuncts(
         products[idx] = w
         _product_to_parents(model)[w] = parents
         # constraint objects are shared across product disjuncts; safe
-        # since reformulations never mutate stored functions
-        for parent in parents, (con, cname) in disjunct_snapshot[parent]
+        # since reformulations never mutate stored functions. unique:
+        # a shared parent indicator appears on multiple axes
+        for parent in unique(parents),
+            (con, cname) in disjunct_snapshot[parent]
             JuMP.add_constraint(model, _DisjunctConstraint(con, w), cname)
         end
         _intersect_globals(model, w, global_snapshot)
@@ -240,7 +245,11 @@ parents of each product indicator can be queried with
 
 Each input disjunction must use `exactly1 = true` (or a
 logical-complement pair) and must not be nested or contain nested
-disjunctions. The number of product disjuncts is the product of the
+disjunctions. Input disjunctions may share indicator variables (a
+warning is emitted): the product then contains logically infeasible
+disjuncts whose indicators the linking constraints force off, so
+results are correct but the model is larger than its pruned
+equivalent. The number of product disjuncts is the product of the
 input disjunction sizes, so repeated basic steps grow the model
 multiplicatively (a warning is emitted above 100).
 

src/constraints.jl

@@ -221,6 +221,11 @@ function JuMP.build_constraint(
     return _DisjunctConstraint(constr, tag.indicator)
 end
 
+_set_support(::_MOI.AbstractSet) = false
+_set_support(
+    ::Union{_MOI.Nonnegatives, _MOI.Nonpositives, _MOI.Zeros}
+) = true
+
 # Allows for building DisjunctConstraints for VectorConstraints since these get parsed differently by JuMP (JuMP changes the set to a MOI.AbstractScalarSet)
 for SetType in (
     JuMP.Nonnegatives, JuMP.Nonpositives, JuMP.Zeros,

test/constraints/basicsteps.jl

@@ -286,14 +286,13 @@ function test_basic_step_errors()
     other = disjunction(m4, C)
     @test_throws ErrorException apply_basic_step(m4, [inner, other])
     @test_throws ErrorException apply_basic_step(m4, [outer, other])
-    # indicators shared between inputs or with an outside disjunction
+    # input indicators used by an outside disjunction
     m5 = GDPModel()
     @variable(m5, P[1:2], Logical)
     @variable(m5, Q[1:2], Logical)
     d5 = disjunction(m5, P)
     d6 = disjunction(m5, Q)
     d7 = disjunction(m5, [P[1], Q[2]])
-    @test_throws ErrorException apply_basic_step(m5, [d5, d7])
     @test_throws ErrorException apply_basic_step(m5, [d5, d6])
     # variable bound refs and reformulation constraints as globals
     m6, x6, _, _, d8, d9, _ = _basic_step_gdp(optimizer = nothing)
@@ -304,6 +303,59 @@ function test_basic_step_errors()
     rc = first(DP._reformulation_constraints(m6))
     @test_throws ErrorException apply_basic_step(
         m6, [d8, d9], constraints = [rc])
+    # vector global with an unsupported set
+    m7 = GDPModel()
+    @variable(m7, z[1:2])
+    @variable(m7, R[1:2], Logical)
+    @constraint(m7, z[1] <= 1, Disjunct(R[1]))
+    @constraint(m7, z[1] >= 2, Disjunct(R[2]))
+    d10 = disjunction(m7, R)
+    g_soc = @constraint(m7, [1.0 + z[1], z[2]] in SecondOrderCone())
+    @test_throws ErrorException apply_basic_step(
+        m7, [d10], constraints = [g_soc])
+end
+
+function test_basic_step_shared_indicators()
+    # d1 = [Y1, Y2] and d2 = [Y1, Y3] couple through Y1: the logic
+    # admits {Y1} (obj 11) or {Y2, Y3} (obj 12)
+    function shared_gdp()
+        model = GDPModel(HiGHS.Optimizer)
+        set_attribute(model, MOI.Silent(), true)
+        @variable(model, 0 <= x[1:2] <= 10)
+        @variable(model, Y[1:3], Logical)
+        @objective(model, Max, x[1] + x[2])
+        @constraint(model, x[1] <= 1, Disjunct(Y[1]))
+        @constraint(model, x[1] >= 3, Disjunct(Y[2]))
+        @constraint(model, x[2] <= 2, Disjunct(Y[3]))
+        d1 = disjunction(model, [Y[1], Y[2]])
+        d2 = disjunction(model, [Y[1], Y[3]])
+        return model, d1, d2
+    end
+    # the unpruned product is built with a warning
+    model, d1, d2 = shared_gdp()
+    new_dref = @test_logs (:warn, r"share indicator") apply_basic_step(
+        model, [d1, d2], name = "bs")
+    @test is_valid(model, new_dref)
+    W = constraint_object(new_dref).indicators
+    @test length(W) == 4
+    # the diagonal cell gets the shared parent's constraints only once
+    @test length(DP._indicator_to_constraints(model)[W[1]]) == 1
+    @test length(DP._indicator_to_constraints(model)[W[2]]) == 2
+    # product exactly1 + both marginal links for Y1 + one each for
+    # Y2, Y3 (the double marginal is what forces dead cells off)
+    @test length(DP._logical_constraints(model)) == 5
+    # solve equivalence before and after the basic step
+    for method in (BigM(), Hull())
+        m, e1, e2 = shared_gdp()
+        optimize!(m, gdp_method = method)
+        @test objective_value(m) ≈ 12
+        m, e1, e2 = shared_gdp()
+        @test_logs (:warn, r"share indicator") apply_basic_step(
+            m, [e1, e2])
+        optimize!(m, gdp_method = method)
+        @test termination_status(m) == MOI.OPTIMAL
+        @test objective_value(m) ≈ 12
+    end
 end
 
 function test_basic_step_solve_equivalence()
@@ -407,6 +459,7 @@ end
     test_basic_step_relax_products()
     test_basic_step_complement()
     test_basic_step_errors()
+    test_basic_step_shared_indicators()
     test_basic_step_solve_equivalence()
     test_basic_step_hull_tightness()
     test_basic_step_repeated()