refactor addconstraint and add some support for setting constraint na…

…mes from macros

src/JuMP.jl

@@ -279,6 +279,20 @@ end
 
 MOI.isvalid(m::Model, cr::ConstraintRef{Model}) = cr.m === m && MOI.isvalid(m.moibackend, cr.index)
 
+"""
+    addconstraint(m::Model, c::AbstractConstraint, name::String="")
+
+Add a constraint `c` to `Model m` and sets its name.
+"""
+function addconstraint(m::Model, c::AbstractConstraint, name::String="")
+    cindex = MOI.addconstraint!(m.moibackend, moi_function_and_set(c)...)
+    cref = ConstraintRef(m, cindex)
+    if !isempty(name)
+        setname(cref, name)
+    end
+    return cref
+end
+
 include("variables.jl")
 
 Base.zero(::Type{Variable}) = AffExpr(Variable[],Float64[],0.0)

src/affexpr.jl

@@ -224,33 +224,18 @@ struct AffExprConstraint{S <: MOI.AbstractScalarSet} <: AbstractConstraint
     set::S
 end
 
-"""
-    addconstraint(m::Model, c::AffExprConstraint)
+moi_function_and_set(c::AffExprConstraint) = (MOI.ScalarAffineFunction(c.func), c.set)
 
-Add an `AffExpr` constraint to `Model m`.
-"""
-function addconstraint(m::Model, c::AffExprConstraint)
-    cindex = MOI.addconstraint!(m.moibackend, MOI.ScalarAffineFunction(c.func), c.set)
-    return ConstraintRef(m, cindex)
-end
-addconstraint(m::Model, c::Array{AffExprConstraint}) =
-    error("The operators <=, >=, and == can only be used to specify scalar constraints. If you are trying to add a vectorized constraint, use the element-wise dot comparison operators (.<=, .>=, or .==) instead")
+# TODO: Find somewhere to put this error message.
+#addconstraint(m::Model, c::Array{AffExprConstraint}) =
+#    error("The operators <=, >=, and == can only be used to specify scalar constraints. If you are trying to add a vectorized constraint, use the element-wise dot comparison operators (.<=, .>=, or .==) instead")
 
 struct VectorAffExprConstraint{S <: MOI.AbstractVectorSet} <: AbstractConstraint
     func::Vector{AffExpr}
     set::S
 end
 
-"""
-    addconstraint(m::Model, c::VectorAffExprConstraint)
-
-Add the vector constraint `c` to `Model m`.
-"""
-function addconstraint(m::Model, c::VectorAffExprConstraint)
-    cindex = MOI.addconstraint!(m.moibackend, MOI.VectorAffineFunction(c.func), c.set)
-    return ConstraintRef(m, cindex)
-end
-
+moi_function_and_set(c::VectorAffExprConstraint) = (MOI.VectorAffineFunction(c.func), c.set)
 
 function constraintobject(cr::ConstraintRef{Model}, ::Type{AffExpr}, ::Type{SetType}) where {SetType <: MOI.AbstractScalarSet}
     f = MOI.get(cr.m, MOI.ConstraintFunction(), cr)::MOI.ScalarAffineFunction

src/macros.jl

@@ -360,6 +360,8 @@ macro constraint(args...)
     variable = gensym()
     quotvarname = quot(getname(c))
     escvarname  = anonvar ? variable : esc(getname(c))
+    basename = anonvar ? "" : string(getname(c))
+    # TODO: support the basename keyword argument
 
     if isa(x, Symbol)
         constraint_error(args, "Incomplete constraint specification $x. Are you missing a comparison (<=, >=, or ==)?")
@@ -382,7 +384,7 @@ macro constraint(args...)
         if x.args[1] == :in
             @assert length(x.args) == 3
             newaff, parsecode = parseExprToplevel(x.args[2], :q)
-            constraintcall = :(addconstraint($m, constructconstraint!($newaff,$(esc(x.args[3])))))
+            constraintcall = :(addconstraint($m, constructconstraint!($newaff,$(esc(x.args[3]))), $(namecall(basename, idxvars))))
         else
             # Simple comparison - move everything to the LHS
             @assert length(x.args) == 3
@@ -393,9 +395,10 @@ macro constraint(args...)
             # `set` is an MOI.AbstractScalarSet, if `newaff` is not scalar, vectorized should be true.
             # Otherwise, `constructconstraint!(::AbstractArray, ::MOI.AbstractScalarSet)` throws an helpful error
             if vectorized
+                # TODO: Pass through names here.
                 constraintcall = :(addconstraint.($m, constructconstraint!.($newaff,$set)))
             else
-                constraintcall = :(addconstraint($m, constructconstraint!($newaff,$set)))
+                constraintcall = :(addconstraint($m, constructconstraint!($newaff,$set), $(namecall(basename, idxvars))))
             end
         end
         addkwargs!(constraintcall, kwargs)
@@ -421,7 +424,7 @@ macro constraint(args...)
         newlb, parselb = parseExprToplevel(x.args[1],:lb)
         newub, parseub = parseExprToplevel(x.args[5],:ub)
 
-        constraintcall = :($addconstr($m, constructconstraint!($newaff,$newlb,$newub)))
+        constraintcall = :($addconstr($m, constructconstraint!($newaff,$newlb,$newub), $(namecall(basename, idxvars))))
         addkwargs!(constraintcall, kwargs)
         code = quote
             aff = zero(AffExpr)
@@ -870,7 +873,7 @@ variabletype(m::Model) = Variable
 # Returns a new variable belonging to the model `m`. Additional positional arguments can be used to dispatch the call to a different method.
 # The return type should only depends on the positional arguments for `variabletype` to make sense.
 function constructvariable!(m::Model, _error::Function, haslb::Bool, lowerbound::Number, hasub::Bool, upperbound::Number,
-                            hasfix::Bool, fixedvalue::Number, binary::Bool, integer::Bool, name::AbstractString,
+                            hasfix::Bool, fixedvalue::Number, binary::Bool, integer::Bool, name::String,
                             hasstart::Bool, start::Number; extra_kwargs...)
     for (kwarg, _) in extra_kwargs
         _error("Unrecognized keyword argument $kwarg")
@@ -905,6 +908,19 @@ const EMPTYSTRING = ""
 
 variable_error(args, str) = error("In @variable($(join(args,","))): ", str)
 
+# Given a basename and idxvars, returns an expression that constructs the name
+# of the object. For use within macros only.
+function namecall(basename, idxvars)
+    length(idxvars) == 0 && return basename
+    ex = Expr(:call,:string,basename,"[")
+    for i in 1:length(idxvars)
+        push!(ex.args, esc(idxvars[i]))
+        i < length(idxvars) && push!(ex.args,",")
+    end
+    push!(ex.args,"]")
+    return ex
+end
+
 # @variable(m, expr, extra...; kwargs...)
 # where `extra` is a list of extra positional arguments and `kwargs` is a list of keyword arguments.
 #
@@ -1095,13 +1111,7 @@ macro variable(args...)
     clear_dependencies(i) = (isdependent(idxvars,idxsets[i],i) ? () : idxsets[i])
 
     # Code to be used to create each variable of the container.
-    namecall = Expr(:call,:string,basename,"[")
-    for i in 1:length(idxvars)
-        push!(namecall.args, esc(idxvars[i]))
-        i < length(idxvars) && push!(namecall.args,",")
-    end
-    push!(namecall.args,"]")
-    variablecall = :( constructvariable!($m, $(extra...), $_error, $haslb, $lb, $hasub, $ub, $hasfix, $fixedvalue, $binary, $integer, $namecall, $hasstart, $value) )
+    variablecall = :( constructvariable!($m, $(extra...), $_error, $haslb, $lb, $hasub, $ub, $hasfix, $fixedvalue, $binary, $integer, $(namecall(basename, idxvars)), $hasstart, $value) )
     addkwargs!(variablecall, extra_kwargs)
     code = :( $(refcall) = $variablecall )
     # Determine the return type of constructvariable!. This is needed to create the container holding them.

src/quadexpr.jl

@@ -131,15 +131,7 @@ struct QuadExprConstraint{S <: MOI.AbstractScalarSet} <: AbstractConstraint
     set::S
 end
 
-"""
-    addconstraint(m::Model, c::QuadExprConstraint)
-
-Add a `QuadExpr` constraint to `Model m`.
-"""
-function addconstraint(m::Model, c::QuadExprConstraint)
-    cindex = MOI.addconstraint!(m.moibackend, MOI.ScalarQuadraticFunction(c.func), c.set)
-    return ConstraintRef(m, cindex)
-end
+moi_function_and_set(c::QuadExprConstraint) = (MOI.ScalarQuadraticFunction(c.func), c.set)
 
 function constraintobject(cr::ConstraintRef{Model}, ::Type{QuadExpr}, ::Type{SetType}) where {SetType <: MOI.AbstractScalarSet}
     m = cr.m

src/sd.jl

@@ -13,16 +13,10 @@ function _constructconstraint!(Q::Matrix{JuMP.Variable}, ::PSDCone)
     SDVariableConstraint(Q)
 end
 
-"""
-    addconstraint(m::Model, c::SDVariableConstraint)
-
-Add a SD variable constraint to `Model m`.
-"""
-function addconstraint(m::Model, c::SDVariableConstraint)
+function moi_function_and_set(c::SDVariableConstraint)
     @assert issymmetric(c.Q)
     n = Base.LinAlg.checksquare(c.Q)
-    cindex = MOI.addconstraint!(m.moibackend, MOI.VectorOfVariables([index(c.Q[i, j]) for j in 1:n for i in 1:j]), MOI.PositiveSemidefiniteConeTriangle(n))
-    return ConstraintRef(m, cindex)
+    return (MOI.VectorOfVariables([index(c.Q[i, j]) for j in 1:n for i in 1:j]), MOI.PositiveSemidefiniteConeTriangle(n))
 end
 
 function constructconstraint!(x::AbstractMatrix, ::PSDCone)

src/variables.jl

@@ -116,33 +116,14 @@ struct SingleVariableConstraint{S <: MOI.AbstractScalarSet} <: AbstractConstrain
     set::S
 end
 
-"""
-    addconstraint(m::Model, c::SingleVariableConstraint)
-
-Add a `SingleVariable` constraint to `Model m`.
-"""
-function addconstraint(m::Model, c::SingleVariableConstraint)
-    cindex = MOI.addconstraint!(m.moibackend, MOI.SingleVariable(c.func), c.set)
-    return ConstraintRef(m, cindex)
-end
-addconstraint(m::Model, c::Array{SingleVariableConstraint}) =
-    error("The operators <=, >=, and == can only be used to specify scalar constraints. If you are trying to add a vectorized constraint, use the element-wise dot comparison operators (.<=, .>=, or .==) instead")
+moi_function_and_set(c::SingleVariableConstraint) = (MOI.SingleVariable(c.func), c.set)
 
 struct VectorOfVariablesConstraint{S <: MOI.AbstractVectorSet} <: AbstractConstraint
     func::Vector{Variable}
     set::S
 end
 
-"""
-    addconstraint(m::Model, c::VectorOfVariablesConstraint)
-
-Add the vector constraint `c` to `Model m`.
-"""
-function addconstraint(m::Model, c::VectorOfVariablesConstraint)
-    cindex = MOI.addconstraint!(m.moibackend, MOI.VectorOfVariables(c.func), c.set)
-    return ConstraintRef(m, cindex)
-end
-
+moi_function_and_set(c::VectorOfVariablesConstraint) = (MOI.VectorOfVariables(c.func), c.set)
 
 function constraintobject(cr::ConstraintRef{Model}, ::Type{Variable}, ::Type{SetType}) where {SetType <: MOI.AbstractScalarSet}
     f = MOI.get(cr.m, MOI.ConstraintFunction(), cr)::MOI.SingleVariable

test/constraint.jl

@@ -7,12 +7,20 @@
 
         # x <= 10.0 doesn't translate to a SingleVariable constraint because
         # the LHS is first subtracted to form x - 10.0 <= 0.
-        cref = @constraint(m, x in MOI.LessThan(10.0))
+        @constraint(m, cref, x in MOI.LessThan(10.0))
+        @test JuMP.name(cref) == "cref"
         c = JuMP.constraintobject(cref, Variable, MOI.LessThan)
         @test c.func == x
         @test c.set == MOI.LessThan(10.0)
         @test_throws TypeError JuMP.constraintobject(cref, QuadExpr, MOI.LessThan)
         @test_throws TypeError JuMP.constraintobject(cref, AffExpr, MOI.EqualTo)
+
+        @variable(m, y[1:2])
+        @constraint(m, cref2[i=1:2], y[i] in MOI.LessThan(float(i)))
+        @test JuMP.name(cref2[1]) == "cref2[1]"
+        c = JuMP.constraintobject(cref2[1], Variable, MOI.LessThan)
+        @test c.func == y[1]
+        @test c.set == MOI.LessThan(1.0)
     end
 
     @testset "VectorOfVariables constraints" begin

test/generate_and_solve.jl

@@ -170,13 +170,9 @@
         @variable(m, y)
         @objective(m, Min, x^2)
 
-        c1 = @constraint(m, 2x*y <= 1)
-        c2 = @constraint(m, y^2 == x^2)
-        c3 = @constraint(m, 2x + 3y*x >= 2)
-
-        JuMP.setname(c1, "c1")
-        JuMP.setname(c2, "c2")
-        JuMP.setname(c3, "c3")
+        @constraint(m, c1, 2x*y <= 1)
+        @constraint(m, c2, y^2 == x^2)
+        @constraint(m, c3, 2x + 3y*x >= 2)
 
         modelstring = """
         variables: x, y
@@ -231,12 +227,9 @@
             z
         end
         @objective(m, Max, 1.0*x)
-        varsoc = @constraint(m, [x,y,z] in MOI.SecondOrderCone(3))
-        JuMP.setname(varsoc, "varsoc")
-        affsoc = @constraint(m, [x+y,z,1.0] in MOI.SecondOrderCone(3))
-        JuMP.setname(affsoc, "affsoc")
-        rotsoc = @constraint(m, [x+1,y,z] in MOI.RotatedSecondOrderCone(3))
-        JuMP.setname(rotsoc, "rotsoc")
+        @constraint(m, varsoc, [x,y,z] in MOI.SecondOrderCone(3))
+        @constraint(m, affsoc, [x+y,z,1.0] in MOI.SecondOrderCone(3))
+        @constraint(m, rotsoc, [x+1,y,z] in MOI.RotatedSecondOrderCone(3))
 
         modelstring = """
         variables: x, y, z

test/variable.jl

@@ -167,6 +167,19 @@ using Base.Test
         @test length.(indices(x)) == (3,2)
     end
 
+    @testset "basename= in @variable" begin
+        m = Model()
+        @variable(m, x)
+        @test JuMP.name(x) == "x"
+
+        y = @variable(m, basename="foo")
+        @test JuMP.name(y) == "foo"
+
+        z = @variable(m, z[i=2:3], basename="t")
+        @test JuMP.name(z[2]) == "t[2]"
+        @test JuMP.name(z[3]) == "t[3]"
+    end
+
     # TODO reenable when printing comes back
     # @testset "condition in indexing" begin
     #    fa = repl[:for_all]