deprecate + on constraints (#659)

* deprecate `+` on constraints

* add test, fix matrix case

* fix example

* Update test/test_utilities.jl

Co-authored-by: Oscar Dowson <[email protected]>

* add test

* Apply suggestions from code review

Co-authored-by: Oscar Dowson <[email protected]>

* Update docs/src/examples/optimization_with_complex_variables/povm_simulation.jl

---------

Co-authored-by: Oscar Dowson <[email protected]>

Author: ericphanson

docs/src/examples/general_examples/basic_usage.jl

@@ -22,9 +22,8 @@ x = Variable(4)
 c = [1; 2; 3; 4]
 A = I(4)
 b = [10; 10; 10; 10]
-p = minimize(dot(c, x)) # or c' * x
-p.constraints += A * x <= b
-p.constraints += [x >= 1; x <= 10; x[2] <= 5; x[1] + x[4] - x[2] <= 10]
+constraints = [A * x <= b, x >= 1, x <= 10, x[2] <= 5, x[1] + x[4] - x[2] <= 10]
+p = minimize(dot(c, x), constraints) # or c' * x
 solve!(p, SCS.Optimizer; silent_solver = true)
 
 println(round(p.optval, digits = 2))

docs/src/examples/general_examples/chebyshev_center.jl

@@ -22,11 +22,13 @@ b = ones(4, 1);
 # Create and solve the model
 r = Variable(1)
 x_c = Variable(2)
-p = maximize(r)
-p.constraints += a1' * x_c + r * norm(a1, 2) <= b[1];
-p.constraints += a2' * x_c + r * norm(a2, 2) <= b[2];
-p.constraints += a3' * x_c + r * norm(a3, 2) <= b[3];
-p.constraints += a4' * x_c + r * norm(a4, 2) <= b[4];
+constraints = [
+    a1' * x_c + r * norm(a1, 2) <= b[1],
+    a2' * x_c + r * norm(a2, 2) <= b[2],
+    a3' * x_c + r * norm(a3, 2) <= b[3],
+    a4' * x_c + r * norm(a4, 2) <= b[4],
+]
+p = maximize(r, constraints)
 solve!(p, SCS.Optimizer; silent_solver = true)
 p.optval
 

docs/src/examples/mixed_integer/n_queens.jl

@@ -10,12 +10,12 @@ x = Variable((n, n), BinVar)
 
 # Now we impose the constraints: at most one queen on any anti-diagonal, at most one queen on any diagonal, and we must have exactly one queen per row and per column.
 ## At most one queen on any anti-diagonal
-constr = Constraint[sum(antidiag(x, k)) <= 1 for k in -n+2:n-2]
+constraints = Constraint[sum(antidiag(x, k)) <= 1 for k in -n+2:n-2]
 ## At most one queen on any diagonal
-constr += Constraint[sum(diag(x, k)) <= 1 for k in -n+2:n-2]
+append!(constraints, [sum(diag(x, k)) <= 1 for k in -n+2:n-2])
 ## Exactly one queen per row and one queen per column
-constr += Constraint[sum(x, dims = 1)==1, sum(x, dims = 2)==1]
-p = satisfy(constr)
+append!(constraints, [sum(x, dims = 1) == 1, sum(x, dims = 2) == 1])
+p = satisfy(constraints)
 solve!(p, GLPK.Optimizer)
 
 # Let us test the results:

docs/src/examples/optimization_with_complex_variables/povm_simulation.jl

@@ -42,14 +42,26 @@ function get_visibility(K)
     P = [[ComplexVariable(2, 2) for i in 1:2] for j in 1:6]
     q = Variable(6, Positive())
     t = Variable(1, Positive())
-    constraints = [isposdef(P[i][j]) for i in 1:6 for j in 1:2]
-    constraints += sum(q) == 1
-    constraints += t <= 1
-    constraints += [P[i][1] + P[i][2] == q[i] * I(2) for i in 1:6]
-    constraints += t * K[1] + (1 - t) * noise[1] == P[1][1] + P[2][1] + P[3][1]
-    constraints += t * K[2] + (1 - t) * noise[2] == P[1][2] + P[4][1] + P[5][1]
-    constraints += t * K[3] + (1 - t) * noise[3] == P[2][2] + P[4][2] + P[6][1]
-    constraints += t * K[4] + (1 - t) * noise[4] == P[3][2] + P[5][2] + P[6][2]
+    constraints = Constraint[isposdef(P[i][j]) for i in 1:6 for j in 1:2]
+    push!(constraints, sum(q) == 1)
+    push!(constraints, t <= 1)
+    append!(constraints, [P[i][1] + P[i][2] == q[i] * I(2) for i in 1:6])
+    push!(
+        constraints,
+        t * K[1] + (1 - t) * noise[1] == P[1][1] + P[2][1] + P[3][1],
+    )
+    push!(
+        constraints,
+        t * K[2] + (1 - t) * noise[2] == P[1][2] + P[4][1] + P[5][1],
+    )
+    push!(
+        constraints,
+        t * K[3] + (1 - t) * noise[3] == P[2][2] + P[4][2] + P[6][1],
+    )
+    push!(
+        constraints,
+        t * K[4] + (1 - t) * noise[4] == P[3][2] + P[5][2] + P[6][2],
+    )
     p = maximize(t, constraints)
     solve!(p, SCS.Optimizer; silent_solver = true)
     return p.optval

src/deprecations.jl

@@ -123,3 +123,35 @@ end
 function ComplexVariable(set::Symbol, sets::Symbol...)
     return ComplexVariable((1, 1), set, sets...)
 end
+
+# `+` on constraints
+function warn_deprecated_constraint_concatenation()
+    @warn(
+        "Concatenating collections of constraints together with `+` or `+=` to produce a new list of constraints is deprecated. Instead, use `vcat` to concatenate collections of constraints.",
+        maxlog = 1
+    )
+    return
+end
+
+function Base.:+(x::Array{<:Constraint}, y::Array{<:Constraint})
+    warn_deprecated_constraint_concatenation()
+    return vcat(x, y)
+end
+
+function Base.:+(x::Constraint, y::Constraint)
+    @warn(
+        "Adding constraints together (with `+` or `+=`) to produce a list of constraints is deprecated. Instead, construct a list of constraints via `[constraint1, constraint2]`",
+        maxlog = 1
+    )
+    return [x, y]
+end
+
+function Base.:+(x::Constraint, y::Array{<:Constraint})
+    warn_deprecated_constraint_concatenation()
+    return vcat(x, y)
+end
+
+function Base.:+(x::Array{<:Constraint}, y::Constraint)
+    warn_deprecated_constraint_concatenation()
+    return vcat(x, y)
+end

src/problem_depot/problems/affine.jl

@@ -754,11 +754,7 @@ end
         @test p.status == MOI.OPTIMAL
     end
 
-    constr = x >= 0
-    constr += x >= 1
-    constr += x <= 10
-    constr2 = x >= 0
-    constr2 += [x >= 2, x <= 3] + constr
+    constr = [x >= 0, x >= 1, x <= 10]
     p = satisfy(constr; numeric_type = T)
 
     handle_problem!(p)

src/problem_depot/problems/socp.jl

@@ -476,9 +476,8 @@ end
     x = Variable(5)
     q = 1.379  # q norm constraint that generates many inequalities
     qs = q / (q - 1)  # Conjugate to q
-    p = minimize(x' * v; numeric_type = T)
+    p = minimize(x' * v, norm(x, q) <= 1; numeric_type = T)
 
-    p.constraints += (norm(x, q) <= 1)
     if test
         @test problem_vexity(p) == ConvexVexity()
     end

src/problems.jl

@@ -6,21 +6,21 @@
 mutable struct Problem{T<:Real} <: AbstractExpr
     head::Symbol
     objective::Union{AbstractExpr,Nothing}
-    constraints::Array{Constraint}
+    constraints::Vector{Constraint}
     status::MOI.TerminationStatusCode
     model::Union{MOI.ModelLike,Nothing}
     function Problem{T}(
         head::Symbol,
         objective::Union{AbstractExpr,Nothing},
-        constraints::Array = Constraint[],
+        constraints = Constraint[],
     ) where {T<:Real}
         if objective !== nothing && sign(objective) == Convex.ComplexSign()
             error("Objective cannot be a complex expression")
         end
         return new(
             head,
             objective,
-            constraints,
+            vec(constraints),
             MOI.OPTIMIZE_NOT_CALLED,
             nothing,
         )
@@ -197,7 +197,7 @@ function Problem{T}(
     constraint::Constraint,
     constraints::Constraint...,
 ) where {T<:Real}
-    return Problem{T}(head, objective, [constraint, constraints...])
+    return Problem{T}(head, objective, Constraint[constraint, constraints...])
 end
 
 # Allow users to simply type minimize
@@ -206,12 +206,16 @@ function minimize(
     constraints::Constraint...;
     numeric_type = Float64,
 )
-    return Problem{numeric_type}(:minimize, objective, collect(constraints))
+    return Problem{numeric_type}(
+        :minimize,
+        objective,
+        collect(Constraint, constraints),
+    )
 end
 
 function minimize(
     objective::AbstractExpr,
-    constraints::Array{<:Constraint} = Constraint[];
+    constraints = Constraint[];
     numeric_type = Float64,
 )
     return Problem{numeric_type}(:minimize, objective, constraints)
@@ -227,7 +231,7 @@ end
 
 function minimize(
     objective::Value,
-    constraints::Array{<:Constraint} = Constraint[];
+    constraints = Constraint[];
     numeric_type = Float64,
 )
     return minimize(constant(objective), constraints; numeric_type)
@@ -244,7 +248,7 @@ end
 
 function maximize(
     objective::AbstractExpr,
-    constraints::Array{<:Constraint} = Constraint[];
+    constraints = Constraint[];
     numeric_type = Float64,
 )
     return Problem{numeric_type}(:maximize, objective, constraints)
@@ -260,52 +264,41 @@ end
 
 function maximize(
     objective::Value,
-    constraints::Array{<:Constraint} = Constraint[];
+    constraints = Constraint[];
     numeric_type = Float64,
 )
     return maximize(constant(objective), constraints; numeric_type)
 end
 
 # Allow users to simply type satisfy (if there is no objective)
 function satisfy(constraints::Constraint...; numeric_type = Float64)
-    return Problem{numeric_type}(:satisfy, nothing, [constraints...])
+    return Problem{numeric_type}(:satisfy, nothing, Constraint[constraints...])
 end
 
-function satisfy(
-    constraints::Array{<:Constraint} = Constraint[];
-    numeric_type = Float64,
-)
+function satisfy(constraints = Constraint[]; numeric_type = Float64)
     return Problem{numeric_type}(:satisfy, nothing, constraints)
 end
 
 function satisfy(constraint::Constraint; numeric_type = Float64)
     return satisfy([constraint]; numeric_type = numeric_type)
 end
 
-function add_constraints!(p::Problem, constraints::Array{<:Constraint})
+function add_constraints!(p::Problem, constraints)
     return append!(p.constraints, constraints)
 end
 
 function add_constraints!(p::Problem, constraint::Constraint)
     return add_constraints!(p, [constraint])
 end
 
-function add_constraint!(p::Problem, constraints::Array{<:Constraint})
+function add_constraint!(p::Problem, constraints)
     return add_constraints!(p, constraints)
 end
 
 function add_constraint!(p::Problem, constraint::Constraint)
     return add_constraints!(p, constraint)
 end
 
-Base.:+(x::Array{<:Constraint}, y::Array{<:Constraint}) = vcat(x, y)
-
-Base.:+(x::Constraint, y::Constraint) = [x, y]
-
-Base.:+(x::Constraint, y::Array{<:Constraint}) = vcat(x, y)
-
-Base.:+(x::Array{<:Constraint}, y::Constraint) = vcat(x, y)
-
 iscomplex(c::Constraint) = iscomplex(c.lhs) || iscomplex(c.rhs)
 
 function add_constraint!(context::Context, c::Constraint)

test/test_utilities.jl

@@ -1008,6 +1008,16 @@ function test_deprecation_in_symbol()
     return
 end
 
+function test_deprecation_adding_constraints()
+    x = Variable()
+    c = x == x
+    @test_logs (:warn, r"^Adding") c + c
+    @test_logs (:warn, r"^Concatenating") [c] + c
+    @test_logs (:warn, r"^Concatenating") c + [c]
+    @test_logs (:warn, r"^Concatenating") [c] + [c]
+    return
+end
+
 function test_dcp_rules()
     vexities = (
         Convex.ConcaveVexity(),
@@ -1118,6 +1128,20 @@ function test_dcp_rules()
     return
 end
 
+function test_problem_untyped_constraints()
+    x = Variable(1, Positive())
+    # Test untyped constraints
+    p = maximize(x, Any[x<=1])
+    @test p isa Problem
+    p = maximize(x, Any[x <= 1;; x <= 1])
+    @test p isa Problem
+    p = minimize(x, Any[x<=1])
+    @test p isa Problem
+    p = minimize(x, Any[x <= 1;; x <= 1])
+    @test p isa Problem
+    return
+end
+
 function test_problem_maximize()
     x = Variable(1, Positive())
     p = maximize(exp(x), x <= 1)