From 8ce75359e20dc3b5583a4c13761820491b0307f3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Legat?= Date: Sun, 22 Sep 2019 18:59:21 +0200 Subject: [PATCH 1/7] Move Containers tests to Containers folder --- test/{ => Containers}/Containers.jl | 0 test/{ => Containers}/DenseAxisArray.jl | 0 test/{ => Containers}/SparseAxisArray.jl | 0 test/{ => Containers}/generate_container.jl | 0 4 files changed, 0 insertions(+), 0 deletions(-) rename test/{ => Containers}/Containers.jl (100%) rename test/{ => Containers}/DenseAxisArray.jl (100%) rename test/{ => Containers}/SparseAxisArray.jl (100%) rename test/{ => Containers}/generate_container.jl (100%) diff --git a/test/Containers.jl b/test/Containers/Containers.jl similarity index 100% rename from test/Containers.jl rename to test/Containers/Containers.jl diff --git a/test/DenseAxisArray.jl b/test/Containers/DenseAxisArray.jl similarity index 100% rename from test/DenseAxisArray.jl rename to test/Containers/DenseAxisArray.jl diff --git a/test/SparseAxisArray.jl b/test/Containers/SparseAxisArray.jl similarity index 100% rename from test/SparseAxisArray.jl rename to test/Containers/SparseAxisArray.jl diff --git a/test/generate_container.jl b/test/Containers/generate_container.jl similarity index 100% rename from test/generate_container.jl rename to test/Containers/generate_container.jl From 05f42426f66895f65e212c9bccd97aa751aeffee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Legat?= Date: Tue, 24 Sep 2019 10:26:44 +0200 Subject: [PATCH 2/7] Create containers with map instead of for loops --- src/Containers/Containers.jl | 2 + src/Containers/SparseAxisArray.jl | 2 + src/Containers/container.jl | 31 ++ src/Containers/macro.jl | 187 ++++++++++ src/Containers/nested_iterator.jl | 44 +++ src/JuMP.jl | 5 +- src/macros.jl | 488 ++++++-------------------- src/parse_expr.jl | 15 +- src/sd.jl | 59 +++- src/shapes.jl | 1 + src/variables.jl | 50 ++- test/Containers/Containers.jl | 1 + test/Containers/generate_container.jl | 2 +- test/Containers/macro.jl | 24 ++ test/JuMPExtension.jl | 6 + test/constraint.jl | 10 +- test/hygiene.jl | 4 +- test/macros.jl | 21 +- test/runtests.jl | 2 +- 19 files changed, 536 insertions(+), 418 deletions(-) create mode 100644 src/Containers/container.jl create mode 100644 src/Containers/macro.jl create mode 100644 src/Containers/nested_iterator.jl create mode 100644 test/Containers/macro.jl diff --git a/src/Containers/Containers.jl b/src/Containers/Containers.jl index 85b3197f040..32d11c4f7fb 100644 --- a/src/Containers/Containers.jl +++ b/src/Containers/Containers.jl @@ -26,5 +26,7 @@ export DenseAxisArray, SparseAxisArray include("DenseAxisArray.jl") include("SparseAxisArray.jl") include("generate_container.jl") +include("container.jl") +include("macro.jl") end diff --git a/src/Containers/SparseAxisArray.jl b/src/Containers/SparseAxisArray.jl index bb746b230ad..001bdcaa522 100644 --- a/src/Containers/SparseAxisArray.jl +++ b/src/Containers/SparseAxisArray.jl @@ -3,6 +3,8 @@ # License, v. 2.0. If a copy of the MPL was not distributed with this # file, You can obtain one at http://mozilla.org/MPL/2.0/. +include("nested_iterator.jl") + """ struct SparseAxisArray{T,N,K<:NTuple{N, Any}} <: AbstractArray{T,N} data::Dict{K,T} diff --git a/src/Containers/container.jl b/src/Containers/container.jl new file mode 100644 index 00000000000..29de1a0facb --- /dev/null +++ b/src/Containers/container.jl @@ -0,0 +1,31 @@ +const ArrayIndices{N} = Base.Iterators.ProductIterator{NTuple{N, Base.OneTo{Int}}} +container(f::Function, indices) = container(f, indices, default_container(indices)) +default_container(::ArrayIndices) = Array +function container(f::Function, indices::ArrayIndices, ::Type{Array}) + return map(I -> f(I...), indices) +end +default_container(::Base.Iterators.ProductIterator) = DenseAxisArray +function container(f::Function, indices::Base.Iterators.ProductIterator, + ::Type{DenseAxisArray}) + return DenseAxisArray(map(I -> f(I...), indices), indices.iterators...) +end +default_container(::NestedIterator) = SparseAxisArray +function container(f::Function, indices, + ::Type{SparseAxisArray}) + mappings = map(I -> I => f(I...), indices) + data = Dict(mappings) + if length(mappings) != length(data) + unique_indices = Set() + duplicate = nothing + for index in indices + if index in unique_indices + duplicate = index + break + end + push!(unique_indices, index) + end + # TODO compute idx + error("Repeated index ", duplicate, ". Index sets must have unique elements.") + end + return SparseAxisArray(Dict(data)) +end diff --git a/src/Containers/macro.jl b/src/Containers/macro.jl new file mode 100644 index 00000000000..47a49d4a3b7 --- /dev/null +++ b/src/Containers/macro.jl @@ -0,0 +1,187 @@ +using Base.Meta + +""" + _extract_kw_args(args) + +Process the arguments to a macro, separating out the keyword arguments. +Return a tuple of (flat_arguments, keyword arguments, and requested_container), +where `requested_container` is a symbol to be passed to `parse_container`. +""" +function _extract_kw_args(args) + kw_args = filter(x -> isexpr(x, :(=)) && x.args[1] != :container , collect(args)) + flat_args = filter(x->!isexpr(x, :(=)), collect(args)) + requested_container = :Auto + for kw in args + if isexpr(kw, :(=)) && kw.args[1] == :container + requested_container = kw.args[2] + end + end + return flat_args, kw_args, requested_container +end + +function _try_parse_idx_set(arg::Expr) + # [i=1] and x[i=1] parse as Expr(:vect, Expr(:(=), :i, 1)) and + # Expr(:ref, :x, Expr(:kw, :i, 1)) respectively. + if arg.head === :kw || arg.head === :(=) + @assert length(arg.args) == 2 + return true, arg.args[1], arg.args[2] + elseif isexpr(arg, :call) && arg.args[1] === :in + return true, arg.args[2], arg.args[3] + else + return false, nothing, nothing + end +end +function _explicit_oneto(index_set) + s = Meta.isexpr(index_set,:escape) ? index_set.args[1] : index_set + if Meta.isexpr(s,:call) && length(s.args) == 3 && s.args[1] == :(:) && s.args[2] == 1 + return :(Base.OneTo($index_set)) + else + return index_set + end +end + +function _expr_is_splat(ex::Expr) + if ex.head == :(...) + return true + elseif ex.head == :escape + return _expr_is_splat(ex.args[1]) + end + return false +end +_expr_is_splat(::Any) = false + +""" + _parse_ref_sets(expr::Expr) + +Helper function for macros to construct container objects. Takes an `Expr` that +specifies the container, e.g. `:(x[i=1:3,[:red,:blue]],k=S; i+k <= 6)`, and +returns: + + 1. `idxvars`: Names for the index variables, e.g. `[:i, gensym(), :k]` + 2. `idxsets`: Sets used for indexing, e.g. `[1:3, [:red,:blue], S]` + 3. `condition`: Expr containing any conditional imposed on indexing, or `:()` if none is present +""" +function _parse_ref_sets(_error::Function, expr::Expr) + c = copy(expr) + idxvars = Any[] + idxsets = Any[] + # On 0.7, :(t[i;j]) is a :ref, while t[i,j;j] is a :typed_vcat. + # In both cases :t is the first arg. + if isexpr(c, :typed_vcat) || isexpr(c, :ref) + popfirst!(c.args) + end + condition = :() + if isexpr(c, :vcat) || isexpr(c, :typed_vcat) + # Parameters appear as plain args at the end. + if length(c.args) > 2 + _error("Unsupported syntax $c.") + elseif length(c.args) == 2 + condition = pop!(c.args) + end # else no condition. + elseif isexpr(c, :ref) || isexpr(c, :vect) + # Parameters appear at the front. + if isexpr(c.args[1], :parameters) + if length(c.args[1].args) != 1 + _error("Invalid syntax: $c. Multiple semicolons are not " * + "supported.") + end + condition = popfirst!(c.args).args[1] + end + end + if isexpr(c, :vcat) || isexpr(c, :typed_vcat) || isexpr(c, :ref) + if isexpr(c.args[1], :parameters) + @assert length(c.args[1].args) == 1 + condition = popfirst!(c.args).args[1] + end # else no condition. + end + + for s in c.args + parse_done = false + if isa(s, Expr) + parse_done, idxvar, _idxset = _try_parse_idx_set(s::Expr) + if parse_done + idxset = esc(_idxset) + end + end + if !parse_done # No index variable specified + idxvar = gensym() + idxset = esc(s) + end + push!(idxvars, idxvar) + push!(idxsets, idxset) + end + return idxvars, idxsets, condition +end +_parse_ref_sets(_error::Function, expr) = (Any[], Any[], :()) + +""" + _build_ref_sets(_error::Function, expr) + +Helper function for macros to construct container objects. Takes an `Expr` that +specifies the container, e.g. `:(x[i=1:3,[:red,:blue]],k=S; i+k <= 6)`, and +returns: + + 1. `idxvars`: Names for the index variables, e.g. `[:i, gensym(), :k]` + 2. `idxsets`: Sets used for indexing, e.g. `[1:3, [:red,:blue], S]` + 3. `condition`: Expr containing any conditional imposed on indexing, or `:()` if none is present +""" +function _build_ref_sets(_error::Function, expr) + idxvars, idxsets, condition = _parse_ref_sets(_error, expr) + if any(_expr_is_splat.(idxsets)) + _error("cannot use splatting operator `...` in the definition of an index set.") + end + has_dependent = has_dependent_sets(idxvars, idxsets) + if has_dependent || condition != :() + esc_idxvars = esc.(idxvars) + idxfuns = [:(($(esc_idxvars[1:(i - 1)]...),) -> $(idxsets[i])) for i in 1:length(idxvars)] + if condition == :() + indices = :(Containers.NestedIterator(($(idxfuns...),))) + else + condition_fun = :(($(esc_idxvars...),) -> $(esc(condition))) + indices = :(Containers.NestedIterator(($(idxfuns...),), $condition_fun)) + end + else + indices = :(Base.Iterators.product(($(_explicit_oneto.(idxsets)...)))) + end + return idxvars, indices +end + +function container_code(idxvars, indices, code, requested_container) + if isempty(idxvars) + return code + end + if !(requested_container in [:Auto, :Array, :DenseAxisArray, :SparseAxisArray]) + # We do this two-step interpolation, first into the string, and then + # into the expression because interpolating into a string inside an + # expression has scoping issues. + error_message = "Invalid container type $requested_container. Must be " * + "Auto, Array, DenseAxisArray, or SparseAxisArray." + return :(error($error_message)) + end + if requested_container == :DenseAxisArray + requested_container = :(JuMP.Containers.DenseAxisArray) + elseif requested_container == :SparseAxisArray + requested_container = :(JuMP.Containers.SparseAxisArray) + end + esc_idxvars = esc.(idxvars) + func = :(($(esc_idxvars...),) -> $code) + if requested_container == :Auto + return :(Containers.container($func, $indices)) + else + return :(Containers.container($func, $indices, $requested_container)) + end +end +function parse_container(_error, var, value, requested_container) + idxvars, indices = _build_ref_sets(_error, var) + return container_code(idxvars, indices, value, requested_container) +end + +macro container(args...) + args, kw_args, requested_container = _extract_kw_args(args) + @assert length(args) == 2 + @assert isempty(kw_args) + var, value = args + name = var.args[1] + code = parse_container(error, var, esc(value), requested_container) + return :($(esc(name)) = $code) +end diff --git a/src/Containers/nested_iterator.jl b/src/Containers/nested_iterator.jl new file mode 100644 index 00000000000..d5097f9afe4 --- /dev/null +++ b/src/Containers/nested_iterator.jl @@ -0,0 +1,44 @@ +struct NestedIterator{T} + iterators::T # Tuple of functions + condition::Function +end +NestedIterator(iterator) = NestedIterator(iterator, (args...) -> true) +Base.IteratorSize(::Type{<:NestedIterator}) = Base.SizeUnknown() +Base.IteratorEltype(::Type{<:NestedIterator}) = Base.EltypeUnknown() +function next_iterate(it::NestedIterator, i, elems, states, iterator, elem_state) + if elem_state === nothing + return nothing + end + elem, state = elem_state + elems_states = first_iterate( + it, i + 1, (elems..., elem), + (states..., (iterator, state, elem))) + if elems_states !== nothing + return elems_states + end + return next_iterate(it, i, elems, states, iterator, iterate(iterator, state)) +end +function first_iterate(it::NestedIterator, i, elems, states) + if i > length(it.iterators) + if it.condition(elems...) + return elems, states + else + return nothing + end + end + iterator = it.iterators[i](elems...) + return next_iterate(it, i, elems, states, iterator, iterate(iterator)) +end +function tail_iterate(it::NestedIterator, i, elems, states) + if i > length(it.iterators) + return nothing + end + next = tail_iterate(it, i + 1, (elems..., states[i][3]), states) + if next !== nothing + return next + end + iterator = states[i][1] + next_iterate(it, i, elems, states[1:(i - 1)], iterator, iterate(iterator, states[i][2])) +end +Base.iterate(it::NestedIterator) = first_iterate(it, 1, tuple(), tuple()) +Base.iterate(it::NestedIterator, states) = tail_iterate(it, 1, tuple(), states) diff --git a/src/JuMP.jl b/src/JuMP.jl index 8a976ea1c9c..487561bd21e 100644 --- a/src/JuMP.jl +++ b/src/JuMP.jl @@ -23,6 +23,8 @@ import ForwardDiff include("_Derivatives/_Derivatives.jl") using ._Derivatives +include("Containers/Containers.jl") + # Exports are at the end of the file. # Deprecations for JuMP v0.18 -> JuMP v0.19 transition @@ -460,7 +462,7 @@ end """ set_time_limit_sec(model::Model, limit) -Sets the time limit (in seconds) of the solver. +Sets the time limit (in seconds) of the solver. Can be unset using `unset_time_limit_sec` or with `limit` set to `nothing`. """ function set_time_limit_sec(model::Model, limit) @@ -768,7 +770,6 @@ struct NonlinearParameter <: AbstractJuMPScalar end include("copy.jl") -include("Containers/Containers.jl") include("operators.jl") include("macros.jl") include("optimizer_interface.jl") diff --git a/src/macros.jl b/src/macros.jl index ece4b866ff5..2caf4305025 100644 --- a/src/macros.jl +++ b/src/macros.jl @@ -14,210 +14,13 @@ end include("parse_expr.jl") -function _build_ref_sets(expr::Expr, cname) - c = copy(expr) - idxvars = Any[] - idxsets = Any[] - # Creating an indexed set of refs - refcall = Expr(:ref, cname) - # On 0.7, :(t[i;j]) is a :ref, while t[i,j;j] is a :typed_vcat. - # In both cases :t is the first arg. - if isexpr(c, :typed_vcat) || isexpr(c, :ref) - popfirst!(c.args) - end - condition = :() - if isexpr(c, :vcat) || isexpr(c, :typed_vcat) - # Parameters appear as plain args at the end. - if length(c.args) > 2 - error("Unsupported syntax $c.") - elseif length(c.args) == 2 - condition = pop!(c.args) - end # else no condition. - elseif isexpr(c, :ref) || isexpr(c, :vect) - # Parameters appear at the front. - if isexpr(c.args[1], :parameters) - if length(c.args[1].args) != 1 - error("Invalid syntax: $c. Multiple semicolons are not " * - "supported.") - end - condition = popfirst!(c.args).args[1] - end - end - if isexpr(c, :vcat) || isexpr(c, :typed_vcat) || isexpr(c, :ref) - if isexpr(c.args[1], :parameters) - @assert length(c.args[1].args) == 1 - condition = popfirst!(c.args).args[1] - end # else no condition. - end - - for s in c.args - parse_done = false - if isa(s, Expr) - parse_done, idxvar, _idxset = _try_parse_idx_set(s::Expr) - if parse_done - idxset = esc(_idxset) - end - end - if !parse_done # No index variable specified - idxvar = gensym() - idxset = esc(s) - end - push!(idxvars, idxvar) - push!(idxsets, idxset) - push!(refcall.args, esc(idxvar)) - end - return refcall, idxvars, idxsets, condition -end - -_build_ref_sets(c, cname) = (cname, Any[], Any[], :()) - -""" - JuMP._build_ref_sets(expr::Expr) - -Helper function for macros to construct container objects. Takes an `Expr` that specifies the container, e.g. `:(x[i=1:3,[:red,:blue]],k=S; i+k <= 6)`, and returns: - - 1. `refcall`: Expr to reference a particular element in the container, e.g. `:(x[i,red,s])` - 2. `idxvars`: Names for the index variables, e.g. `[:i, gensym(), :k]` - 3. `idxsets`: Sets used for indexing, e.g. `[1:3, [:red,:blue], S]` - 4. `condition`: Expr containing any conditional imposed on indexing, or `:()` if none is present -""" -_build_ref_sets(c) = _build_ref_sets(c, _get_name(c)) - -function _expr_is_splat(ex::Expr) - if ex.head == :(...) - return true - elseif ex.head == :escape - return _expr_is_splat(ex.args[1]) - end - return false -end -_expr_is_splat(::Any) = false - -""" - JuMP._get_looped_code(varname, code, condition, idxvars, idxsets, sym, requestedcontainer::Symbol; lowertri=false) - -Helper function for macros to transform expression objects containing kernel code, index sets, conditionals, etc. to an expression that performs the desired loops that iterate over the kernel code. Arguments to the function are: - - 1. `varname`: name and appropriate indexing sets (if any) for container that is assigned to in the kernel code, e.g. `:myvar` or `:(x[i=1:3,[:red,:blue]])` - 2. `code`: `Expr` containing kernel code - 3. `condition`: `Expr` that is evaluated immediately before kernel code in each iteration. If none, pass `:()`. - 4. `idxvars`: Names for the index variables for each loop, e.g. `[:i, gensym(), :k]` - 5. `idxsets`: Sets used to define iteration for each loop, e.g. `[1:3, [:red,:blue], S]` - 6. `sym`: A `Symbol`/`Expr` containing the element type of the container that is being iterated over, e.g. `:AffExpr` or `:VariableRef` - 7. `requestedcontainer`: Argument that is passed through to `generate_container`. Either `:Auto`, `:Array`, `:DenseAxisArray`, or `:SparseAxisArray`. - 8. `lowertri`: `Bool` keyword argument that is `true` if the iteration is over a cartesian array and should only iterate over the lower triangular entries, filling upper triangular entries with copies, e.g. `x[1,3] === x[3,1]`, and `false` otherwise. -""" -function _get_looped_code(varname, code, condition, idxvars, idxsets, sym, requestedcontainer::Symbol; lowertri=false) - - # if we don't have indexing, just return to avoid allocating stuff - if isempty(idxsets) - return code - end - - hascond = (condition != :()) - - if !(requestedcontainer in [:Auto, :Array, :DenseAxisArray, :SparseAxisArray]) - # We do this two-step interpolation, first into the string, and then - # into the expression because interpolating into a string inside an - # expression has scoping issues. - error_message = "Invalid container type $requestedcontainer. Must be " * - "Auto, Array, DenseAxisArray, or SparseAxisArray." - return :(error($error_message)) - end - - if hascond - if requestedcontainer == :Auto - requestedcontainer = :SparseAxisArray - elseif requestedcontainer == :Array || requestedcontainer == :DenseAxisArray - return :(error("Requested container type is incompatible with ", - "conditional indexing. Use :SparseAxisArray or ", - ":Auto instead.")) - end - end - containercode, autoduplicatecheck = Containers.generate_container(sym, - idxvars, idxsets, requestedcontainer) - - if lowertri - @assert !hascond - @assert length(idxvars) == 2 - @assert !Containers.has_dependent_sets(idxvars, idxsets) - - i, j = esc(idxvars[1]), esc(idxvars[2]) - expr = copy(code) - vname = expr.args[1].args[1] - tmp = gensym() - expr.args[1] = tmp - code = quote - for $i in $(idxsets[1]), $j in $(idxsets[2]) - $i <= $j || continue - $expr - $vname[$i,$j] = $tmp - $vname[$j,$i] = $tmp - end - end - else - if !autoduplicatecheck # we need to check for duplicate keys in the index set - if length(idxvars) > 1 - keytuple = Expr(:tuple, esc.(idxvars)...) - else - keytuple = esc(idxvars[1]) - end - code = quote - if haskey($varname, $keytuple) - error(string("Repeated index ", $keytuple,". Index sets must have unique elements.")) - end - $code - end - end - if hascond - code = quote - $(esc(condition)) || continue - $code - end - end - for (idxvar, idxset) in zip(reverse(idxvars),reverse(idxsets)) - code = quote - for $(esc(idxvar)) in $idxset - $code - end - end - end - end - - - return quote - $varname = $containercode - $code - nothing - end -end - -""" - _extract_kw_args(args) - -Process the arguments to a macro, separating out the keyword arguments. -Return a tuple of (flat_arguments, keyword arguments, and requestedcontainer), -where `requestedcontainer` is a symbol to be passed to `_get_looped_code`. -""" -function _extract_kw_args(args) - kw_args = filter(x -> isexpr(x, :(=)) && x.args[1] != :container , collect(args)) - flat_args = filter(x->!isexpr(x, :(=)), collect(args)) - requestedcontainer = :Auto - for kw in args - if isexpr(kw, :(=)) && kw.args[1] == :container - requestedcontainer = kw.args[2] - end - end - return flat_args, kw_args, requestedcontainer -end - """ _add_kw_args(call, kw_args) Add the keyword arguments `kw_args` to the function call expression `call`, escaping the expressions. The elements of `kw_args` should be expressions of the form `:(key = value)`. The `kw_args` vector can be extracted from the arguments -of a macro with [`_extract_kw_args`](@ref). +of a macro with [`Containers._extract_kw_args`](@ref). ## Examples @@ -261,26 +64,20 @@ function _assert_valid_model(m, macrocode) end """ - _macro_return(model, code, variable) + _macro_return(code) Return a block of code that 1. runs the code block `code` in a local scope and -2. returns the value of a local variable named `variable`. `variable` must - reference a variable defined by `code`. +2. returns the value of the `code`. """ -function _macro_return(code, variable) - return quote - let - # The let block ensures that all variables created behave like - # local variables, see - # https://github.com/JuliaOpt/JuMP.jl/issues/1496. - # To make $variable accessible from outside we need to return it at - # the end of the block. - $code - $variable - end - end +function _macro_return(code) + # The let block ensures that all variables created behave like + # local variables, see + # https://github.com/JuliaOpt/JuMP.jl/issues/1496. + # This is needed as `sum` are transformed into `for` loops in + # `parse_expr`. + return :( let; $code; end ) end function _error_if_cannot_register(model::AbstractModel, name::Symbol) @@ -300,18 +97,16 @@ end """ _macro_assign_and_return(code, variable, name; - final_variable=variable, model_for_registering=nothing) Return runs `code` in a local scope which returns the value of `variable` -and then assign `final_variable` to `name`. +and then assign `variable` to `name`. If `model_for_registering` is given, the generated code assigns the resulting object to the model dictionary. """ function _macro_assign_and_return(code, variable, name; - final_variable=variable, model_for_registering=nothing) - macro_code = _macro_return(code, variable) + macro_code = _macro_return(code) return quote $(if model_for_registering !== nothing :(_error_if_cannot_register($model_for_registering, @@ -320,10 +115,10 @@ function _macro_assign_and_return(code, variable, name; $variable = $macro_code $(if model_for_registering !== nothing :(object_dictionary($model_for_registering)[$(quot(name))] = - $final_variable) + $variable) end) # This assignment should be in the scope calling the macro - $(esc(name)) = $final_variable + $(esc(name)) = $variable end end @@ -564,7 +359,7 @@ constraint reference. function _constraint_macro(args, macro_name::Symbol, parsefun::Function) _error(str...) = _macro_error(macro_name, args, str...) - args, kw_args, requestedcontainer = _extract_kw_args(args) + args, kw_args, requestedcontainer = Containers._extract_kw_args(args) if length(args) < 2 if length(kw_args) > 0 @@ -599,12 +394,9 @@ function _constraint_macro(args, macro_name::Symbol, parsefun::Function) (x.head == :block) && _error("Code block passed as constraint. Perhaps you meant to use @constraints instead?") - # Strategy: build up the code for add_constraint, and if needed - # we will wrap in loops to assign to the ConstraintRefs - refcall, idxvars, idxsets, condition = _build_ref_sets(c, variable) - if any(_expr_is_splat.(idxsets)) - _error("cannot use splatting operator `...` in the definition of an index set.") - end + # Strategy: build up the code for add_constraint, and if needed we will wrap + # in a function returning `ConstraintRef`s and give it to `Containers.container`. + idxvars, indices = Containers._build_ref_sets(_error, c) vectorized, parsecode, buildcall = parsefun(_error, x.args...) _add_kw_args(buildcall, kw_args) @@ -616,22 +408,16 @@ function _constraint_macro(args, macro_name::Symbol, parsefun::Function) end code = quote $parsecode - $(refcall) = $constraintcall + $constraintcall end - # Determine the return type of add_constraint. This is needed for JuMP extensions for which this is different than ConstraintRef - if vectorized - contype = :( AbstractArray{constraint_type($m)} ) # TODO use a concrete type instead of AbstractArray, see #525, #1310 - else - contype = :( constraint_type($m) ) - end - creationcode = _get_looped_code(variable, code, condition, idxvars, idxsets, contype, requestedcontainer) + creationcode = Containers.container_code(idxvars, indices, code, requestedcontainer) if anonvar # Anonymous constraint, no need to register it in the model-level # dictionary nor to assign it to a variable in the user scope. # We simply return the constraint reference - macro_code = _macro_return(creationcode, variable) + macro_code = _macro_return(creationcode) else # We register the constraint reference to its name and # we assign it to a variable in the local scope of this name @@ -818,7 +604,7 @@ macro build_constraint(constraint_expr) $result_variable = $build_call end - return _macro_return(code, result_variable) + return _macro_return(code) end _add_JuMP_prefix(s::Symbol) = Expr(:., JuMP, :($(QuoteNode(s)))) @@ -977,8 +763,9 @@ macro objective(model, args...) q = Val{false}() $parsecode set_objective($esc_model, $sense_expr, $newaff) + $newaff end - return _assert_valid_model(esc_model, _macro_return(code, newaff)) + return _assert_valid_model(esc_model, _macro_return(code)) end # Return a standalone, unnamed expression @@ -990,7 +777,7 @@ macro _build_expression(x) q = Val{false}() $parsecode end - return _macro_return(code, newaff) + return _macro_return(code) end @@ -1021,8 +808,8 @@ expr = @expression(m, [i=1:3], i*sum(x[j] for j=1:3)) ``` """ macro expression(args...) - macro_error(str...) = _macro_error(:expression, args, str...) - args, kw_args, requestedcontainer = _extract_kw_args(args) + _error(str...) = _macro_error(:expression, args, str...) + args, kw_args, requestedcontainer = Containers._extract_kw_args(args) if length(args) == 3 m = esc(args[1]) c = args[2] @@ -1032,36 +819,27 @@ macro expression(args...) c = gensym() x = args[2] else - macro_error("needs at least two arguments.") + _error("needs at least two arguments.") end - length(kw_args) == 0 || macro_error("unrecognized keyword argument") + length(kw_args) == 0 || _error("unrecognized keyword argument") anonvar = isexpr(c, :vect) || isexpr(c, :vcat) || length(args) == 2 variable = gensym() - refcall, idxvars, idxsets, condition = _build_ref_sets(c, variable) - if any(_expr_is_splat.(idxsets)) - macro_error("cannot use splatting operator `...` in the definition of an index set.") - end + idxvars, indices = Containers._build_ref_sets(_error, c) newaff, parsecode = _parse_expr_toplevel(x, :q) code = quote q = Val{false}() $parsecode end - if isa(c,Expr) - code = quote - $code - (isa($newaff,AffExpr) || isa($newaff,Number) || isa($newaff,VariableRef)) || error("Collection of expressions with @expression must be linear. For quadratic expressions, use your own array.") - end - end code = quote $code - $(refcall) = $newaff + $newaff end - code = _get_looped_code(variable, code, condition, idxvars, idxsets, :AffExpr, requestedcontainer) + code = Containers.container_code(idxvars, indices, code, requestedcontainer) # don't do anything with the model, but check that it's valid anyway if anonvar - macro_code = _macro_return(code, variable) + macro_code = _macro_return(code) else macro_code = _macro_assign_and_return(code, variable, _get_name(c), model_for_registering = m) @@ -1086,6 +864,13 @@ function build_variable(_error::Function, info::VariableInfo; extra_kw_args...) end return ScalarVariable(info) end +function build_variable(_error::Function, infos::Vector{<:VariableInfo}, + set::AbstractVectorSet; extra_kw_args...) + for (kwarg, _) in extra_kw_args + _error("Unrecognized keyword argument $kwarg") + end + return ConstrainedVariables(infos, moi_set(set, length(infos))) +end function _macro_error(macroname, args, str...) error("In `@$macroname($(join(args, ", ")))`: ", str...) @@ -1299,14 +1084,12 @@ JuMP.add_variable(model, JuMP.build_variable(error, info), "x") The following creates a `DenseAxisArray` of index set `[:a, :b]` and with respective upper bounds 2 and 3 and names `x[a]` and `x[b]` as with the second example above but does it without using the `@variable` macro -```julia +```jldoctest; setup = :(using JuMP; model = Model()) # Without the `@variable` macro -data = Vector{JuMP.variable_type(model)}(undef, length(keys(ub))) -x = JuMP.Containers.DenseAxisArray(data, keys(ub)) -for i in keys(ub) - info = VariableInfo(false, NaN, true, ub[i], false, NaN, false, NaN, false, false) - x[i] = JuMP.add_variable(model, JuMP.build_variable(error, info), "x[\$i]") -end +x = JuMP.Containers.container(i -> begin + info = VariableInfo(false, NaN, true, ub[i], false, NaN, false, NaN, false, false) + x[i] = JuMP.add_variable(model, JuMP.build_variable(error, info), "x[\$i]") + end, Base.Iterators.product(keys(ub)) ``` The following are equivalent ways of creating a `Matrix` of size @@ -1328,7 +1111,7 @@ macro variable(args...) model = esc(args[1]) - extra, kw_args, requestedcontainer = _extract_kw_args(args[2:end]) + extra, kw_args, requestedcontainer = Containers._extract_kw_args(args[2:end]) # if there is only a single non-keyword argument, this is an anonymous # variable spec and the one non-kwarg is the model @@ -1365,7 +1148,7 @@ macro variable(args...) end anonvar = isexpr(var, :vect) || isexpr(var, :vcat) || anon_singleton - anonvar && explicit_comparison && error("Cannot use explicit bounds via >=, <= with an anonymous variable") + anonvar && explicit_comparison && _error("Cannot use explicit bounds via >=, <= with an anonymous variable") variable = gensym() # TODO: Should we generate non-empty default names for variables? name = _get_name(var) @@ -1382,8 +1165,13 @@ macro variable(args...) # process keyword arguments obj = nothing - sdp = any(t -> (t == :PSD), extra) - symmetric = (sdp || any(t -> (t == :Symmetric), extra)) + set = nothing + if any(t -> (t == :PSD), extra) + set = :(JuMP.PSDCone()) + end + if any(t -> (t == :Symmetric), extra) + set = :(JuMP.SymMatrixSpace()) + end extra = filter(x -> (x != :PSD && x != :Symmetric), extra) # filter out PSD and sym tag for ex in extra if ex == :Int @@ -1392,88 +1180,49 @@ macro variable(args...) _set_binary_or_error(_error, infoexpr) end end - extra = esc.(filter(ex -> !(ex in [:Int,:Bin]), extra)) + extra = esc.(filter(ex -> !(ex in [:Int, :Bin]), extra)) if !isempty(variable_type_kw_args) push!(extra, esc(variable_type_kw_args[1].args[2])) end info = _constructor_expr(infoexpr) - if isa(var,Symbol) + if isa(var, Symbol) # Easy case - a single variable - sdp && _error("Cannot add a semidefinite scalar variable") + set === nothing || _error("Cannot add a scalar constrained variable in $set.") buildcall = :( build_variable($_error, $info, $(extra...)) ) _add_kw_args(buildcall, extra_kw_args) variablecall = :( add_variable($model, $buildcall, $base_name) ) # The looped code is trivial here since there is a single variable creationcode = :($variable = $variablecall) - final_variable = variable else - isa(var,Expr) || _error("Expected $var to be a variable name") + isa(var, Expr) || _error("Expected $var to be a variable name") # We now build the code to generate the variables (and possibly the # SparseAxisArray to contain them) - refcall, idxvars, idxsets, condition = _build_ref_sets(var, variable) - if any(_expr_is_splat.(idxsets)) - _error("cannot use splatting operator `...` in the definition of an index set.") - end - clear_dependencies(i) = (Containers.is_dependent(idxvars,idxsets[i],i) ? () : idxsets[i]) + idxvars, indices = Containers._build_ref_sets(_error, var) # Code to be used to create each variable of the container. buildcall = :( build_variable($_error, $info, $(extra...)) ) _add_kw_args(buildcall, extra_kw_args) - variablecall = :( add_variable($model, $buildcall, $(_name_call(base_name, idxvars))) ) - code = :( $(refcall) = $variablecall ) - # Determine the return type of add_variable. This is needed to create the container holding them. - vartype = :( variable_type($model, $(extra...)) ) - - if symmetric - # Sanity checks on PSD input stuff - condition == :() || - _error("Cannot have conditional indexing for PSD variables") - length(idxvars) == length(idxsets) == 2 || - _error("PSD variables must be 2-dimensional") - !symmetric || (length(idxvars) == length(idxsets) == 2) || - _error("Symmetric variables must be 2-dimensional") - Containers.has_dependent_sets(idxvars, idxsets) && - _error("Cannot have index dependencies in symmetric variables") - for _rng in idxsets - isexpr(_rng, :escape) || - _error("Internal error 1") - rng = _rng.args[1] # undo escaping - (isexpr(rng,:call) && length(rng.args) == 3 && rng.args[1] == :(:) && rng.args[2] == 1) || - _error("Index sets for SDP variables must be ranges of the form 1:N") - end - - if infoexpr.has_lb || infoexpr.has_ub - _error("Semidefinite or symmetric variables cannot be provided bounds") - end - # 1:3 is parsed as (:call, :, 1, 3) - dimension_check = :($(esc(idxsets[1].args[1].args[3])) == - $(esc(idxsets[2].args[1].args[3]))) - creationcode = quote - $dimension_check || error("Cannot construct symmetric variables with nonsquare dimensions.") - $(_get_looped_code(variable, code, condition, idxvars, idxsets, vartype, requestedcontainer; lowertri=symmetric)) - $(if sdp - quote - JuMP.add_constraint($model, JuMP.build_constraint($_error, Symmetric($variable), JuMP.PSDCone())) - end - end) - end - final_variable = :(Symmetric($variable)) + name_code = _name_call(base_name, idxvars) + if set === nothing + variablecall = :( add_variable($model, $buildcall, $name_code) ) + creationcode = Containers.container_code(idxvars, indices, variablecall, requestedcontainer) else - creationcode = _get_looped_code(variable, code, condition, idxvars, idxsets, vartype, requestedcontainer) - final_variable = variable + scalar_variables = Containers.container_code(idxvars, indices, buildcall, requestedcontainer) + names = Containers.container_code(idxvars, indices, name_code, requestedcontainer) + buildcall = :( build_variable($_error, $scalar_variables, $set, $(extra...)) ) + creationcode = :( add_variable($model, $buildcall, $names) ) end end if anonvar # Anonymous variable, no need to register it in the model-level # dictionary nor to assign it to a variable in the user scope. # We simply return the variable - macro_code = _macro_return(creationcode, final_variable) + macro_code = _macro_return(creationcode) else # We register the variable reference to its name and # we assign it to a variable in the local scope of this name macro_code = _macro_assign_and_return(creationcode, variable, name, - final_variable=final_variable, model_for_registering = model) end return _assert_valid_model(model, macro_code) @@ -1497,7 +1246,7 @@ macro NLobjective(model, sense, x) $ex = $(_process_NL_expr(model, x)) set_objective($(esc(model)), $sense_expr, $ex) end - return _assert_valid_model(esc(model), _macro_return(code, nothing)) + return _assert_valid_model(esc(model), _macro_return(code)) end """ @@ -1511,30 +1260,28 @@ Add a constraint described by the nonlinear expression `expr`. See also @NLconstraint(model, [i = 1:3], sin(i * x) <= 1 / i) ``` """ -macro NLconstraint(m, x, extra...) +macro NLconstraint(m, x, args...) + _error(str...) = _macro_error(:NLconstraint, (m, x, args...), str...) esc_m = esc(m) # Two formats: # - @NLconstraint(m, a*x <= 5) # - @NLconstraint(m, myref[a=1:5], sin(x^a) <= 5) - extra, kw_args, requestedcontainer = _extract_kw_args(extra) - (length(extra) > 1 || length(kw_args) > 0) && error("in @NLconstraint: too many arguments.") + extra, kw_args, requestedcontainer = Containers._extract_kw_args(args) + (length(extra) > 1 || length(kw_args) > 0) && _error("too many arguments.") # Canonicalize the arguments - c = length(extra) == 1 ? x : gensym() - x = length(extra) == 1 ? extra[1] : x + c = length(extra) == 1 ? x : gensym() + con = length(extra) == 1 ? extra[1] : x anonvar = isexpr(c, :vect) || isexpr(c, :vcat) || length(extra) != 1 variable = gensym() # Strategy: build up the code for non-macro add_constraint, and if needed # we will wrap in loops to assign to the ConstraintRefs - refcall, idxvars, idxsets, condition = _build_ref_sets(c, variable) - if any(_expr_is_splat.(idxsets)) - error("@NLconstraint: cannot use splatting operator `...` in the definition of an index set.") - end + idxvars, indices = Containers._build_ref_sets(_error, c) # Build the constraint - if isexpr(x, :call) # one-sided constraint + if isexpr(con, :call) # one-sided constraint # Simple comparison - move everything to the LHS - op = x.args[1] + op = con.args[1] if op == :(==) lb = 0.0 ub = 0.0 @@ -1545,44 +1292,44 @@ macro NLconstraint(m, x, extra...) lb = 0.0 ub = Inf else - error("in @NLconstraint ($(string(x))): expected comparison operator (<=, >=, or ==).") + _error("expected comparison operator (<=, >=, or ==).") end - lhs = :($(x.args[2]) - $(x.args[3])) + lhs = :($(con.args[2]) - $(con.args[3])) code = quote c = _NonlinearConstraint($(_process_NL_expr(m, lhs)), $lb, $ub) push!($esc_m.nlp_data.nlconstr, c) - $(refcall) = ConstraintRef($esc_m, NonlinearConstraintIndex(length($esc_m.nlp_data.nlconstr)), ScalarShape()) + ConstraintRef($esc_m, NonlinearConstraintIndex(length($esc_m.nlp_data.nlconstr)), ScalarShape()) end - elseif isexpr(x, :comparison) + elseif isexpr(con, :comparison) # ranged row - if (x.args[2] != :<= && x.args[2] != :≤) || (x.args[4] != :<= && x.args[4] != :≤) - error("in @NLconstraint ($(string(x))): only ranged rows of the form lb <= expr <= ub are supported.") + if (con.args[2] != :<= && con.args[2] != :≤) || (con.args[4] != :<= && con.args[4] != :≤) + _error("only ranged rows of the form lb <= expr <= ub are supported.") end - lb = x.args[1] - ub = x.args[5] + lb = con.args[1] + ub = con.args[5] code = quote if !isa($(esc(lb)),Number) - error(string("in @NLconstraint (",$(string(x)),"): expected ",$(string(lb))," to be a number.")) + _error("expected ", $(string(lb)), " to be a number.") elseif !isa($(esc(ub)),Number) - error(string("in @NLconstraint (",$(string(x)),"): expected ",$(string(ub))," to be a number.")) + _error("expected ", $(string(ub)), " to be a number.") end - c = _NonlinearConstraint($(_process_NL_expr(m, x.args[3])), $(esc(lb)), $(esc(ub))) + c = _NonlinearConstraint($(_process_NL_expr(m, con.args[3])), $(esc(lb)), $(esc(ub))) push!($esc_m.nlp_data.nlconstr, c) - $(refcall) = ConstraintRef($esc_m, NonlinearConstraintIndex(length($esc_m.nlp_data.nlconstr)), ScalarShape()) + ConstraintRef($esc_m, NonlinearConstraintIndex(length($esc_m.nlp_data.nlconstr)), ScalarShape()) end else # Unknown - error("in @NLconstraint ($(string(x))): constraints must be in one of the following forms:\n" * - " expr1 <= expr2\n" * " expr1 >= expr2\n" * - " expr1 == expr2") + _error("constraints must be in one of the following forms:\n" * + " expr1 <= expr2\n" * " expr1 >= expr2\n" * + " expr1 == expr2") end - looped = _get_looped_code(variable, code, condition, idxvars, idxsets, :(ConstraintRef{Model,NonlinearConstraintIndex}), requestedcontainer) + looped = Containers.container_code(idxvars, indices, code, requestedcontainer) creation_code = quote _init_NLP($esc_m) $looped end if anonvar - macro_code = _macro_return(creation_code, variable) + macro_code = _macro_return(creation_code) else macro_code = _macro_assign_and_return(creation_code, variable, _get_name(c), @@ -1610,9 +1357,10 @@ my_expr_2 = @NLexpression(m, log(1 + sum(exp(x[i])) for i in 1:2)) ``` """ macro NLexpression(args...) - args, kw_args, requestedcontainer = _extract_kw_args(args) + _error(str...) = _macro_error(:NLexpression, args, str...) + args, kw_args, requestedcontainer = Containers._extract_kw_args(args) if length(args) <= 1 - error("in @NLexpression: To few arguments ($(length(args))); must pass the model and nonlinear expression as arguments.") + _error("To few arguments ($(length(args))); must pass the model and nonlinear expression as arguments.") elseif length(args) == 2 m, x = args c = gensym() @@ -1620,22 +1368,17 @@ macro NLexpression(args...) m, c, x = args end if length(args) > 3 || length(kw_args) > 0 - error("in @NLexpression: To many arguments ($(length(args))).") + _error("To many arguments ($(length(args))).") end anonvar = isexpr(c, :vect) || isexpr(c, :vcat) || length(args) == 2 variable = gensym() - refcall, idxvars, idxsets, condition = _build_ref_sets(c, variable) - if any(_expr_is_splat.(idxsets)) - error("@NLexpression: cannot use splatting operator `...` in the definition of an index set.") - end - code = quote - $(refcall) = NonlinearExpression($(esc(m)), $(_process_NL_expr(m, x))) - end - creation_code = _get_looped_code(variable, code, condition, idxvars, idxsets, :NonlinearExpression, requestedcontainer) + idxvars, indices = Containers._build_ref_sets(_error, c) + code = :( NonlinearExpression($(esc(m)), $(_process_NL_expr(m, x))) ) + creation_code = Containers.container_code(idxvars, indices, code, requestedcontainer) if anonvar - macro_code = _macro_return(creation_code, variable) + macro_code = _macro_return(creation_code) else macro_code = _macro_assign_and_return(creation_code, variable, _get_name(c), @@ -1679,37 +1422,34 @@ value(y[9]) ``` """ macro NLparameter(m, ex, extra...) + _error(str...) = _macro_error(:NLparameter, (m, ex, extra...), str...) - extra, kw_args, requestedcontainer = _extract_kw_args(extra) - (length(extra) == 0 && length(kw_args) == 0) || error("in @NLperameter: too many arguments.") + extra, kw_args, requestedcontainer = Containers._extract_kw_args(extra) + (length(extra) == 0 && length(kw_args) == 0) || _error("Too many arguments.") if !isexpr(ex, :call) || length(ex.args) != 3 || ex.args[1] != :(==) - error("In @NLparameter($m, $ex): syntax error.") + _error("Syntax error.") end c = ex.args[2] x = ex.args[3] anonvar = isexpr(c, :vect) || isexpr(c, :vcat) if anonvar - error("In @NLparameter($m, $ex): Anonymous nonlinear parameter syntax is not currently supported") + _error("Anonymous nonlinear parameter syntax is not currently supported.") end - m = esc(m) + esc_m = esc(m) variable = gensym() - refcall, idxvars, idxsets, condition = _build_ref_sets(c, variable) - if any(_expr_is_splat.(idxsets)) - error("@NLparameter: cannot use splatting operator `...` in the definition of an index set.") - end + idxvars, indices = Containers._build_ref_sets(_error, c) code = quote if !isa($(esc(x)), Number) - error(string("in @NLparameter (", $(string(ex)), "): expected ", - $(string(x))," to be a number.")) + _error("Expected ", $(string(x)), " to be a number.") end - $(refcall) = _new_parameter($m, $(esc(x))) + _new_parameter($esc_m, $(esc(x))) end - creation_code = _get_looped_code(variable, code, condition, idxvars, idxsets, :NonlinearParameter, :Auto) + creation_code = Containers.container_code(idxvars, indices, code, :Auto) # TODO: NLparameters are not registered in the model because we don't yet # have an anonymous version. macro_code = _macro_assign_and_return(creation_code, variable, _get_name(c)) - return _assert_valid_model(m, macro_code) + return _assert_valid_model(esc_m, macro_code) end diff --git a/src/parse_expr.jl b/src/parse_expr.jl index 6da1707dd87..874d03beb55 100644 --- a/src/parse_expr.jl +++ b/src/parse_expr.jl @@ -3,21 +3,8 @@ # License, v. 2.0. If a copy of the MPL was not distributed with this # file, You can obtain one at http://mozilla.org/MPL/2.0/. -function _try_parse_idx_set(arg::Expr) - # [i=1] and x[i=1] parse as Expr(:vect, Expr(:(=), :i, 1)) and - # Expr(:ref, :x, Expr(:kw, :i, 1)) respectively. - if arg.head === :kw || arg.head === :(=) - @assert length(arg.args) == 2 - return true, arg.args[1], arg.args[2] - elseif isexpr(arg, :call) && arg.args[1] === :in - return true, arg.args[2], arg.args[3] - else - return false, nothing, nothing - end -end - function _parse_idx_set(arg::Expr) - parse_done, idxvar, idxset = _try_parse_idx_set(arg) + parse_done, idxvar, idxset = Containers._try_parse_idx_set(arg) if parse_done return idxvar, idxset end diff --git a/src/sd.jl b/src/sd.jl index b5854471d64..82910fea936 100644 --- a/src/sd.jl +++ b/src/sd.jl @@ -1,3 +1,5 @@ +struct SymMatrixSpace end + """ PSDCone @@ -89,6 +91,10 @@ function reshape_set(::MOI.PositiveSemidefiniteConeTriangle, ::SymmetricMatrixShape) return PSDCone() end +function vectorize(matrix, ::SymmetricMatrixShape) + n = LinearAlgebra.checksquare(matrix) + return [matrix[i, j] for j in 1:n for i in 1:j] +end """ SquareMatrixShape @@ -107,6 +113,49 @@ end function reshape_set(::MOI.PositiveSemidefiniteConeSquare, ::SquareMatrixShape) return PSDCone() end +vectorize(matrix, ::SquareMatrixShape) = vec(matrix) + +function _square_side(_error::Function, ::Containers.SparseAxisArray) + _error("Cannot have index dependencies in symmetric variables.") +end +function _square_side(_error::Function, ::Containers.DenseAxisArray) + _error("Index sets for symmetric variables must be ranges of the form 1:N.") +end +function _square_side(_error::Function, ::Array) + _error("Symmetric variables must be 2-dimensional.") +end +function _square_side(_error::Function, variables::Matrix) + n, m = size(variables) + if n != m + _error("Symmetric variables must be 2-dimensional.") + end + return n +end + +function _vectorize_variables(_error::Function, matrix::Matrix) + n = LinearAlgebra.checksquare(matrix) + for j in 1:n + for i in 1:j + if matrix[i, j] != matrix[j, i] + _error("Non-symmetric bounds, integrality or starting values for symmetric variable.") + end + end + end + return vectorize(matrix, SymmetricMatrixShape(n)) +end + +function build_variable(_error::Function, variables, ::SymMatrixSpace) + n = _square_side(_error, variables) + set = MOI.Reals(MOI.dimension(MOI.PositiveSemidefiniteConeTriangle(n))) + shape = SymmetricMatrixShape(n) + return ConstrainedVariables(_vectorize_variables(_error, variables), set, shape) +end +function build_variable(_error::Function, variables, ::PSDCone) + n = _square_side(_error, variables) + set = MOI.PositiveSemidefiniteConeTriangle(n) + shape = SymmetricMatrixShape(n) + return ConstrainedVariables(_vectorize_variables(_error, variables), set, shape) +end """ function build_constraint(_error::Function, Q::Symmetric{V, M}, @@ -136,9 +185,10 @@ function build_constraint(_error::Function, Q::Symmetric{V, M}, ::PSDCone) where {V <: AbstractJuMPScalar, M <: AbstractMatrix{V}} n = LinearAlgebra.checksquare(Q) - VectorConstraint([Q[i, j] for j in 1:n for i in 1:j], + shape = SymmetricMatrixShape(n) + VectorConstraint(vectorize(Q, shape), MOI.PositiveSemidefiniteConeTriangle(n), - SymmetricMatrixShape(n)) + shape) end """ @@ -169,7 +219,8 @@ function build_constraint(_error::Function, Q::AbstractMatrix{<:AbstractJuMPScalar}, ::PSDCone) n = LinearAlgebra.checksquare(Q) - VectorConstraint(vec(Q), + shape = SquareMatrixShape(n) + VectorConstraint(vectorize(Q, shape), MOI.PositiveSemidefiniteConeSquare(n), - SquareMatrixShape(n)) + shape) end diff --git a/src/shapes.jl b/src/shapes.jl index a5a756d26ea..57d05302f7d 100644 --- a/src/shapes.jl +++ b/src/shapes.jl @@ -118,3 +118,4 @@ Vector for which the vectorized form corresponds exactly to the vector given. """ struct VectorShape <: AbstractShape end reshape_vector(vectorized_form, ::VectorShape) = vectorized_form +vectorize(x, ::VectorShape) = x diff --git a/src/variables.jl b/src/variables.jl index 4cc7f90039d..549724048ac 100644 --- a/src/variables.jl +++ b/src/variables.jl @@ -764,7 +764,14 @@ end function _moi_add_variable(backend, model, v::ScalarVariable, name::String) index = MOI.add_variable(backend) var_ref = VariableRef(model, index) - info = v.info + _moi_constrain_variable(backend, index, v.info) + if !isempty(name) + set_name(var_ref, name) + end + return var_ref +end + +function _moi_constrain_variable(backend::MOI.ModelLike, index, info) # We don't call the _moi* versions (e.g., _moi_set_lower_bound) because they # have extra checks that are not necessary for newly created variables. if info.has_lb @@ -787,12 +794,45 @@ function _moi_add_variable(backend, model, v::ScalarVariable, name::String) MOI.add_constraint(backend, MOI.SingleVariable(index), MOI.Integer()) end if info.has_start - set_start_value(var_ref, info.start) + MOI.set(backend, MOI.VariablePrimalStart(), index, + Float64(info.start)) end - if !isempty(name) - set_name(var_ref, name) +end + +struct ConstrainedVariables{S <: MOI.AbstractVectorSet, Shape <: AbstractShape, + ScalarVarType <: AbstractVariable} <: AbstractVariable + scalar_variables::Vector{ScalarVarType} + set::S + shape::Shape +end +function ConstrainedVariables(variables::Vector{<:AbstractVariable}, set::MOI.AbstractVectorSet) + return ConstrainedVariables(variables, set, VectorShape()) +end + +function add_variable(model::Model, variable::ConstrainedVariables, names) + var_indices = _moi_add_constrained_variables( + backend(model), variable.scalar_variables, variable.set, vectorize(names, variable.shape)) + var_refs = [VariableRef(model, var_index) for var_index in var_indices] + return reshape_vector(var_refs, variable.shape) +end + +function _moi_add_constrained_variables( + backend::MOI.ModelLike, scalar_variables::Vector{<:ScalarVariable}, + set::MOI.AbstractVectorSet, names::Vector{String}) + if set isa MOI.Reals + var_indices = MOI.add_variables(backend, MOI.dimension(set)) + else + var_indices, con_index = MOI.add_constrained_variables(backend, set) end - return var_ref + for (index, variable) in zip(var_indices, scalar_variables) + _moi_constrain_variable(backend, index, variable.info) + end + if names !== nothing + for (var_index, name) in zip(var_indices, names) + MOI.set(backend, MOI.VariableName(), var_index, name) + end + end + return var_indices end """ diff --git a/test/Containers/Containers.jl b/test/Containers/Containers.jl index fee9e6429b8..2c5e48ce094 100644 --- a/test/Containers/Containers.jl +++ b/test/Containers/Containers.jl @@ -6,4 +6,5 @@ using JuMP.Containers include("DenseAxisArray.jl") include("SparseAxisArray.jl") include("generate_container.jl") + include("macro.jl") end diff --git a/test/Containers/generate_container.jl b/test/Containers/generate_container.jl index 0a675403daf..4951ba26e61 100644 --- a/test/Containers/generate_container.jl +++ b/test/Containers/generate_container.jl @@ -9,7 +9,7 @@ using JuMP.Containers macro dummycontainer(expr, requestedtype) name = gensym() - refcall, indexvars, indexsets, condition = JuMP._build_ref_sets(expr, name) + indexvars, indexsets, condition = JuMP.Containers._parse_ref_sets(error, expr) if condition == :() return Containers.generate_container(Bool, indexvars, indexsets, requestedtype)[1] diff --git a/test/Containers/macro.jl b/test/Containers/macro.jl new file mode 100644 index 00000000000..8e953ae94ed --- /dev/null +++ b/test/Containers/macro.jl @@ -0,0 +1,24 @@ +using Test +using JuMP +using JuMP.Containers + +@testset "Macro" begin + @testset "Array" begin + Containers.@container(x[i = 1:3], i^2) + @test x isa Vector{Int} + Containers.@container(x[i = 1:3, j = 1:3], i^2) + @test x isa Matrix{Int} + end + @testset "DenseAxisArray" begin + Containers.@container(x[i = 2:3], i^2) + @test x isa Containers.DenseAxisArray{Int, 1} + Containers.@container(x[i = 2:3, j = 1:2], i + j) + @test x isa Containers.DenseAxisArray{Int, 2} + end + @testset "SparseAxisArray" begin + Containers.@container(x[i = 1:3, j = 1:i], i + j) + @test x isa Containers.SparseAxisArray{Int, 2} + Containers.@container(x[i=1:10; iseven(i)], i) + @test x isa Containers.SparseAxisArray{Int, 1} + end +end diff --git a/test/JuMPExtension.jl b/test/JuMPExtension.jl index 9ab3b6edf54..115705d61d2 100644 --- a/test/JuMPExtension.jl +++ b/test/JuMPExtension.jl @@ -59,6 +59,12 @@ function JuMP.add_variable(m::MyModel, v::JuMP.AbstractVariable, name::String="" JuMP.set_name(vref, name) vref end +function JuMP.add_variable(model::MyModel, variable::JuMP.ConstrainedVariables, names) + var_refs = JuMP.add_variable.(model, variable.scalar_variables, + JuMP.vectorize(names, variable.shape)) + JuMP.add_constraint(model, JuMP.VectorConstraint(var_refs, variable.set)) + return JuMP.reshape_vector(var_refs, variable.shape) +end function JuMP.delete(model::MyModel, vref::MyVariableRef) @assert JuMP.is_valid(model, vref) delete!(model.variables, vref.idx) diff --git a/test/constraint.jl b/test/constraint.jl index 03e53d5d482..9dd6a331b99 100644 --- a/test/constraint.jl +++ b/test/constraint.jl @@ -195,12 +195,14 @@ function constraints_test(ModelType::Type{<:JuMP.AbstractModel}, @variable m x[1:2] @constraint m cref1[i=2:4] x .== [i, i+1] ConstraintRefType = eltype(cref1[2]) - @test cref1 isa JuMP.Containers.DenseAxisArray{AbstractArray{ConstraintRefType}} + @test cref1 isa JuMP.Containers.DenseAxisArray{Vector{ConstraintRefType}} @constraint m cref2[i=1:3, j=1:4] x .≤ [i+j, i-j] - @test cref2 isa Matrix{AbstractArray{ConstraintRefType}} + ConstraintRefType = eltype(cref2[1]) + @test cref2 isa Matrix{Vector{ConstraintRefType}} @variable m y[1:2, 1:2] - @constraint m cref3[i=1:2] x[i,:] .== 1 - @test cref3 isa Vector{AbstractArray{ConstraintRefType}} + @constraint m cref3[i=1:2] y[i,:] .== 1 + ConstraintRefType = eltype(cref3[1]) + @test cref3 isa Vector{Vector{ConstraintRefType}} end @testset "QuadExpr constraints" begin diff --git a/test/hygiene.jl b/test/hygiene.jl index f5214d9888b..d33eb6ff798 100644 --- a/test/hygiene.jl +++ b/test/hygiene.jl @@ -51,8 +51,8 @@ model = JuMP.Model() i = 10 j = 10 JuMP.@expression(model, ex[j = 2:3], sum(i for i in 1:j)) -@test ex[2] == JuMP.AffExpr(3) -@test ex[3] == JuMP.AffExpr(6) +@test ex[2] == 3 +@test ex[3] == 6 @test i == 10 @test j == 10 diff --git a/test/macros.jl b/test/macros.jl index 5b2a9361564..bab1ac56c64 100644 --- a/test/macros.jl +++ b/test/macros.jl @@ -50,19 +50,18 @@ mutable struct MyVariable end @testset "Extension of @variable with build_variable #1029" begin - local MyVariable = Tuple{JuMP.VariableInfo, Int, Int} - JuMP.variable_type(m::Model, ::Type{MyVariable}) = MyVariable + local MyVariable{S, T, U, V} = Tuple{JuMP.VariableInfo{S, T, U, V}, Int, Int} names = Dict{MyVariable, String}() - function JuMP.add_variable(m::Model, v::MyVariable, name::String="") + function JuMP.add_variable(::Model, v::MyVariable, name::String="") names[v] = name - v + return v end # Since `VariableInfo` is an immutable struct, two objects with the same # fields have the same hash hence we need to add an id to distinguish # variables in the `names` dictionary. id = 0 function JuMP.build_variable(_error::Function, info::JuMP.VariableInfo, ::Type{MyVariable}; test_kw::Int = 0) - (info, test_kw, id += 1) + return (info, test_kw, id += 1) end m = Model() @variable(m, 1 <= x <= 2, MyVariable, binary = true, test_kw = 1, start = 3) @@ -83,7 +82,7 @@ end @test test_kw == 1 @variable(m, y[1:3] >= 0, MyVariable, test_kw = 2) - @test isa(y, Vector{MyVariable}) + @test isa(y, Vector{<:MyVariable}) for i in 1:3 info = y[i][1] test_kw = y[i][2] @@ -454,8 +453,8 @@ end i = 10 j = 10 @expression(model, ex[j = 2:3], sum(i for i in 1:j)) - @test ex[2] == AffExpr(3) - @test ex[3] == AffExpr(6) + @test ex[2] == 3 + @test ex[3] == 6 @test i == 10 @test j == 10 end @@ -500,7 +499,7 @@ end ) @constraint(model, [axes(A)...], x >= 1) @test_macro_throws ErrorException( - "@NLconstraint: cannot use splatting operator `...` in the definition of an index set." + "In `@NLconstraint(model, [axes(A)...], x >= 1)`: cannot use splatting operator `...` in the definition of an index set." ) @NLconstraint(model, [axes(A)...], x >= 1) @test_macro_throws ErrorException( @@ -508,11 +507,11 @@ end ) @expression(model, [axes(A)...], x) @test_macro_throws ErrorException( - "@NLexpression: cannot use splatting operator `...` in the definition of an index set." + "In `@NLexpression(model, [axes(A)...], x)`: cannot use splatting operator `...` in the definition of an index set." ) @NLexpression(model, [axes(A)...], x) @test_macro_throws ErrorException( - "@NLparameter: cannot use splatting operator `...` in the definition of an index set." + "In `@NLparameter(model, p[axes(A)...] == x)`: cannot use splatting operator `...` in the definition of an index set." ) @NLparameter(model, p[axes(A)...] == x) end diff --git a/test/runtests.jl b/test/runtests.jl index e8c56a3ed46..bde270f53f7 100755 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -21,7 +21,7 @@ const MOI = MathOptInterface const MOIT = MOI.Test const MOIU = MOI.Utilities -include("Containers.jl") +include("Containers/Containers.jl") include("utilities.jl") include("JuMPExtension.jl") From c771e5d58ef1ce60dca32bd1073a45f0f18363eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Legat?= Date: Tue, 24 Sep 2019 16:37:20 +0200 Subject: [PATCH 3/7] Update docstrings --- docs/src/constraints.md | 14 +++++------ docs/src/variables.md | 6 ++--- src/Containers/container.jl | 16 +++++++++--- src/Containers/macro.jl | 8 +++--- src/Containers/nested_iterator.jl | 21 ++++++++++++++++ src/macros.jl | 42 +++++++++++++++++++++---------- src/sd.jl | 40 ++++++++++++++++++++++------- src/variables.jl | 19 ++++++++++++++ 8 files changed, 127 insertions(+), 39 deletions(-) diff --git a/docs/src/constraints.md b/docs/src/constraints.md index e07e84d114d..2dae857bc01 100644 --- a/docs/src/constraints.md +++ b/docs/src/constraints.md @@ -251,7 +251,7 @@ following. One way of adding a group of constraints compactly is the following: ```jldoctest constraint_arrays; setup=:(model=Model(); @variable(model, x)) julia> @constraint(model, con[i = 1:3], i * x <= i + 1) -3-element Array{ConstraintRef{Model,C,Shape} where Shape<:AbstractShape where C,1}: +3-element Array{ConstraintRef{Model,MathOptInterface.ConstraintIndex{MathOptInterface.ScalarAffineFunction{Float64},MathOptInterface.LessThan{Float64}},ScalarShape},1}: con[1] : x <= 2.0 con[2] : 2 x <= 3.0 con[3] : 3 x <= 4.0 @@ -264,7 +264,7 @@ julia> con[1] con[1] : x <= 2.0 julia> con[2:3] -2-element Array{ConstraintRef{Model,C,Shape} where Shape<:AbstractShape where C,1}: +2-element Array{ConstraintRef{Model,MathOptInterface.ConstraintIndex{MathOptInterface.ScalarAffineFunction{Float64},MathOptInterface.LessThan{Float64}},ScalarShape},1}: con[2] : 2 x <= 3.0 con[3] : 3 x <= 4.0 ``` @@ -273,7 +273,7 @@ Anonymous containers can also be constructed by dropping the name (e.g. `con`) before the square brackets: ```jldoctest constraint_arrays julia> @constraint(model, [i = 1:2], i * x <= i + 1) -2-element Array{ConstraintRef{Model,C,Shape} where Shape<:AbstractShape where C,1}: +2-element Array{ConstraintRef{Model,MathOptInterface.ConstraintIndex{MathOptInterface.ScalarAffineFunction{Float64},MathOptInterface.LessThan{Float64}},ScalarShape},1}: x <= 2.0 2 x <= 3.0 ``` @@ -294,10 +294,10 @@ variables. ```jldoctest constraint_jumparrays; setup=:(model=Model(); @variable(model, x)) julia> @constraint(model, con[i = 1:2, j = 2:3], i * x <= j + 1) -2-dimensional DenseAxisArray{ConstraintRef{Model,C,Shape} where Shape<:AbstractShape where C,2,...} with index sets: - Dimension 1, 1:2 +2-dimensional DenseAxisArray{ConstraintRef{Model,MathOptInterface.ConstraintIndex{MathOptInterface.ScalarAffineFunction{Float64},MathOptInterface.LessThan{Float64}},ScalarShape},2,...} with index sets: + Dimension 1, Base.OneTo(2) Dimension 2, 2:3 -And data, a 2×2 Array{ConstraintRef{Model,C,Shape} where Shape<:AbstractShape where C,2}: +And data, a 2×2 Array{ConstraintRef{Model,MathOptInterface.ConstraintIndex{MathOptInterface.ScalarAffineFunction{Float64},MathOptInterface.LessThan{Float64}},ScalarShape},2}: con[1,2] : x <= 3.0 con[1,3] : x <= 4.0 con[2,2] : 2 x <= 3.0 con[2,3] : 2 x <= 4.0 ``` @@ -311,7 +311,7 @@ similar to the [syntax for constructing](@ref variable_sparseaxisarrays) a ```jldoctest constraint_jumparrays; setup=:(model=Model(); @variable(model, x)) julia> @constraint(model, con[i = 1:2, j = 1:2; i != j], i * x <= j + 1) -JuMP.Containers.SparseAxisArray{ConstraintRef{Model,C,Shape} where Shape<:AbstractShape where C,2,Tuple{Any,Any}} with 2 entries: +JuMP.Containers.SparseAxisArray{ConstraintRef{Model,MathOptInterface.ConstraintIndex{MathOptInterface.ScalarAffineFunction{Float64},MathOptInterface.LessThan{Float64}},ScalarShape},2,Tuple{Int64,Int64}} with 2 entries: [1, 2] = con[1,2] : x <= 3.0 [2, 1] = con[2,1] : 2 x <= 2.0 ``` diff --git a/docs/src/variables.md b/docs/src/variables.md index 60a95373c27..3746fe2d226 100644 --- a/docs/src/variables.md +++ b/docs/src/variables.md @@ -324,7 +324,7 @@ return a `DenseAxisArray`. For example: ```jldoctest variables_jump_arrays; setup=:(model=Model()) julia> @variable(model, x[1:2, [:A,:B]]) 2-dimensional DenseAxisArray{VariableRef,2,...} with index sets: - Dimension 1, 1:2 + Dimension 1, Base.OneTo(2) Dimension 2, Symbol[:A, :B] And data, a 2×2 Array{VariableRef,2}: x[1,A] x[1,B] @@ -371,7 +371,7 @@ For example, this applies when indices have a dependence upon previous indices (called *triangular indexing*). JuMP supports this as follows: ```jldoctest; setup=:(model=Model()) julia> @variable(model, x[i=1:2, j=i:2]) -JuMP.Containers.SparseAxisArray{VariableRef,2,Tuple{Any,Any}} with 3 entries: +JuMP.Containers.SparseAxisArray{VariableRef,2,Tuple{Int64,Int64}} with 3 entries: [1, 2] = x[1,2] [2, 2] = x[2,2] [1, 1] = x[1,1] @@ -382,7 +382,7 @@ syntax appends a comparison check that depends upon the named indices and is separated from the indices by a semi-colon (`;`). For example: ```jldoctest; setup=:(model=Model()) julia> @variable(model, x[i=1:4; mod(i, 2)==0]) -JuMP.Containers.SparseAxisArray{VariableRef,1,Tuple{Any}} with 2 entries: +JuMP.Containers.SparseAxisArray{VariableRef,1,Tuple{Int64}} with 2 entries: [4] = x[4] [2] = x[2] ``` diff --git a/src/Containers/container.jl b/src/Containers/container.jl index 29de1a0facb..bef17066c5e 100644 --- a/src/Containers/container.jl +++ b/src/Containers/container.jl @@ -1,11 +1,21 @@ -const ArrayIndices{N} = Base.Iterators.ProductIterator{NTuple{N, Base.OneTo{Int}}} +const ArrayIndices{N} = Iterators.ProductIterator{NTuple{N, Base.OneTo{Int}}} container(f::Function, indices) = container(f, indices, default_container(indices)) default_container(::ArrayIndices) = Array function container(f::Function, indices::ArrayIndices, ::Type{Array}) return map(I -> f(I...), indices) end -default_container(::Base.Iterators.ProductIterator) = DenseAxisArray -function container(f::Function, indices::Base.Iterators.ProductIterator, +function _oneto(indices) + if indices isa UnitRange{Int} && indices == 1:length(indices) + return Base.OneTo(length(indices)) + end + error("Index set for array is not one-based interval.") +end +function container(f::Function, indices::Iterators.ProductIterator, + ::Type{Array}) + container(f, Iterators.ProductIterator(_oneto.(indices.iterators)), Array) +end +default_container(::Iterators.ProductIterator) = DenseAxisArray +function container(f::Function, indices::Iterators.ProductIterator, ::Type{DenseAxisArray}) return DenseAxisArray(map(I -> f(I...), indices), indices.iterators...) end diff --git a/src/Containers/macro.jl b/src/Containers/macro.jl index 47a49d4a3b7..14380b36f30 100644 --- a/src/Containers/macro.jl +++ b/src/Containers/macro.jl @@ -54,7 +54,7 @@ _expr_is_splat(::Any) = false _parse_ref_sets(expr::Expr) Helper function for macros to construct container objects. Takes an `Expr` that -specifies the container, e.g. `:(x[i=1:3,[:red,:blue]],k=S; i+k <= 6)`, and +specifies the container, e.g. `:(x[i=1:3,[:red,:blue],k=S; i+k <= 6])`, and returns: 1. `idxvars`: Names for the index variables, e.g. `[:i, gensym(), :k]` @@ -118,12 +118,12 @@ _parse_ref_sets(_error::Function, expr) = (Any[], Any[], :()) _build_ref_sets(_error::Function, expr) Helper function for macros to construct container objects. Takes an `Expr` that -specifies the container, e.g. `:(x[i=1:3,[:red,:blue]],k=S; i+k <= 6)`, and +specifies the container, e.g. `:(x[i=1:3,[:red,:blue],k=S; i+k <= 6])`, and returns: 1. `idxvars`: Names for the index variables, e.g. `[:i, gensym(), :k]` - 2. `idxsets`: Sets used for indexing, e.g. `[1:3, [:red,:blue], S]` - 3. `condition`: Expr containing any conditional imposed on indexing, or `:()` if none is present + 2. `indices`: Iterators over the indices indexing, e.g. + `Constainers.NestedIterators((1:3, [:red,:blue], S), (i, ##..., k) -> i + k <= 6)`. """ function _build_ref_sets(_error::Function, expr) idxvars, idxsets, condition = _parse_ref_sets(_error, expr) diff --git a/src/Containers/nested_iterator.jl b/src/Containers/nested_iterator.jl index d5097f9afe4..706a130e459 100644 --- a/src/Containers/nested_iterator.jl +++ b/src/Containers/nested_iterator.jl @@ -1,3 +1,24 @@ +""" + struct NestedIterator{T} + iterators::T # Tuple of functions + condition::Function + end + +Iterators over the tuples that are produced by a nested for loop. +For instance, if `length(iterators) == 3` , this corresponds to the tuples +`(i1, i2, i3)` produced by: +``` +for i1 in iterators[1]() + for i2 in iterator[2](i1) + for i3 in iterator[3](i1, i2) + if condition(i1, i2, i3) + # produces (i1, i2, i3) + end + end + end +end +``` +""" struct NestedIterator{T} iterators::T # Tuple of functions condition::Function diff --git a/src/macros.jl b/src/macros.jl index 2caf4305025..15661ba9485 100644 --- a/src/macros.jl +++ b/src/macros.jl @@ -864,13 +864,6 @@ function build_variable(_error::Function, info::VariableInfo; extra_kw_args...) end return ScalarVariable(info) end -function build_variable(_error::Function, infos::Vector{<:VariableInfo}, - set::AbstractVectorSet; extra_kw_args...) - for (kwarg, _) in extra_kw_args - _error("Unrecognized keyword argument $kwarg") - end - return ConstrainedVariables(infos, moi_set(set, length(infos))) -end function _macro_error(macroname, args, str...) error("In `@$macroname($(join(args, ", ")))`: ", str...) @@ -1042,12 +1035,28 @@ x = @variable(model, base_name="x", lower_bound=0) The following are equivalent ways of creating a `DenseAxisArray` of index set `[:a, :b]` and with respective upper bounds 2 and 3 and names `x[a]` and `x[b]`. -```julia +The upper bound can either be specified in `expr`: +```jldoctest variable_macro; setup = :(using JuMP; model = Model()) ub = Dict(:a => 2, :b => 3) -# Specify everything in `expr` @variable(model, x[i=keys(ub)] <= ub[i]) -# Specify the upper bound using a keyword argument -@variable(model, x[i=keys(ub)], upper_bound=ub[i]) + +# output +1-dimensional DenseAxisArray{VariableRef,1,...} with index sets: + Dimension 1, Symbol[:a, :b] +And data, a 2-element Array{VariableRef,1}: + x[a] + x[b] +``` +or it can be specified with the `upper_bound` keyword argument: +```jldoctest variable_macro +@variable(model, y[i=keys(ub)], upper_bound=ub[i]) + +# output +1-dimensional DenseAxisArray{VariableRef,1,...} with index sets: + Dimension 1, Symbol[:a, :b] +And data, a 2-element Array{VariableRef,1}: + y[a] + y[b] ``` ## Note for extending the variable macro @@ -1084,12 +1093,19 @@ JuMP.add_variable(model, JuMP.build_variable(error, info), "x") The following creates a `DenseAxisArray` of index set `[:a, :b]` and with respective upper bounds 2 and 3 and names `x[a]` and `x[b]` as with the second example above but does it without using the `@variable` macro -```jldoctest; setup = :(using JuMP; model = Model()) +```jldoctest variable_macro # Without the `@variable` macro x = JuMP.Containers.container(i -> begin info = VariableInfo(false, NaN, true, ub[i], false, NaN, false, NaN, false, false) x[i] = JuMP.add_variable(model, JuMP.build_variable(error, info), "x[\$i]") - end, Base.Iterators.product(keys(ub)) + end, Base.Iterators.product(keys(ub))) + +# output +1-dimensional DenseAxisArray{VariableRef,1,...} with index sets: + Dimension 1, Symbol[:a, :b] +And data, a 2-element Array{VariableRef,1}: + x[a] + x[b] ``` The following are equivalent ways of creating a `Matrix` of size diff --git a/src/sd.jl b/src/sd.jl index 82910fea936..663bf466898 100644 --- a/src/sd.jl +++ b/src/sd.jl @@ -144,12 +144,36 @@ function _vectorize_variables(_error::Function, matrix::Matrix) return vectorize(matrix, SymmetricMatrixShape(n)) end +""" + build_constraint(_error::Function, variables, ::SymMatrixSpace) + +Return a `ConstrainedVariables` of shape [`SymmetricMatrixShape`](@ref) +creating variables in `MOI.Reals`, i.e. "free" variables unless they are +constrained after their creation. + +This function is used by the [`@variable`](@ref) macro as follows: +```julia +@variable(model, Q[1:2, 1:2], Symmetric) +``` +""" function build_variable(_error::Function, variables, ::SymMatrixSpace) n = _square_side(_error, variables) set = MOI.Reals(MOI.dimension(MOI.PositiveSemidefiniteConeTriangle(n))) shape = SymmetricMatrixShape(n) return ConstrainedVariables(_vectorize_variables(_error, variables), set, shape) end + +""" + build_constraint(_error::Function, variables, ::PSDCone) + +Return a `ConstrainedVariables` of shape [`SymmetricMatrixShape`](@ref) +constraining the variables to be positive semidefinite. + +This function is used by the [`@variable`](@ref) macro as follows: +```julia +@variable(model, Q[1:2, 1:2], PSD) +``` +""" function build_variable(_error::Function, variables, ::PSDCone) n = _square_side(_error, variables) set = MOI.PositiveSemidefiniteConeTriangle(n) @@ -158,16 +182,14 @@ function build_variable(_error::Function, variables, ::PSDCone) end """ - function build_constraint(_error::Function, Q::Symmetric{V, M}, - ::PSDCone) where {V <: AbstractJuMPScalar, - M <: AbstractMatrix{V}} + build_constraint(_error::Function, Q::Symmetric{V, M}, + ::PSDCone) where {V <: AbstractJuMPScalar, + M <: AbstractMatrix{V}} Return a `VectorConstraint` of shape [`SymmetricMatrixShape`](@ref) constraining the matrix `Q` to be positive semidefinite. -This function is used by the [`@variable`](@ref) macro to create a symmetric -semidefinite matrix of variables and by the [`@constraint`](@ref) macros as -follows: +This function is used by the [`@constraint`](@ref) macros as follows: ```julia @constraint(model, Symmetric(Q) in PSDCone()) ``` @@ -192,9 +214,9 @@ function build_constraint(_error::Function, Q::Symmetric{V, M}, end """ - function build_constraint(_error::Function, - Q::AbstractMatrix{<:AbstractJuMPScalar}, - ::PSDCone) + build_constraint(_error::Function, + Q::AbstractMatrix{<:AbstractJuMPScalar}, + ::PSDCone) Return a `VectorConstraint` of shape [`SquareMatrixShape`](@ref) constraining the matrix `Q` to be symmetric and positive semidefinite. diff --git a/src/variables.jl b/src/variables.jl index 549724048ac..65693890de7 100644 --- a/src/variables.jl +++ b/src/variables.jl @@ -799,12 +799,31 @@ function _moi_constrain_variable(backend::MOI.ModelLike, index, info) end end +""" + ConstrainedVariables <: AbstractVariable + +Vector of variables `scalar_variables` constrained to belong to `set`. +Adding this variable can be thought as doing: +```julia +function JuMP.add_variable(model::Model, variable::JuMP.ConstrainedVariables, names) + var_refs = JuMP.add_variable.(model, variable.scalar_variables, + JuMP.vectorize(names, variable.shape)) + JuMP.add_constraint(model, JuMP.VectorConstraint(var_refs, variable.set)) + return JuMP.reshape_vector(var_refs, variable.shape) +end +``` +but adds the variables with `MOI.add_constrained_variables(model, variable.set)` +instead. See [the MOI documentation](http://www.juliaopt.org/MathOptInterface.jl/v0.9.3/apireference/#Variables-1) +for the difference between adding the variables with `MOI.add_constrained_variables` +and adding them with `MOI.add_variables` and adding the constraint separately. +""" struct ConstrainedVariables{S <: MOI.AbstractVectorSet, Shape <: AbstractShape, ScalarVarType <: AbstractVariable} <: AbstractVariable scalar_variables::Vector{ScalarVarType} set::S shape::Shape end + function ConstrainedVariables(variables::Vector{<:AbstractVariable}, set::MOI.AbstractVectorSet) return ConstrainedVariables(variables, set, VectorShape()) end From 63ff9cc60e63f1ff6baad4e5a46d487642ce2836 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Legat?= Date: Tue, 24 Sep 2019 19:33:25 +0200 Subject: [PATCH 4/7] Fix for iterators with shape --- src/Containers/Containers.jl | 2 ++ src/Containers/SparseAxisArray.jl | 2 -- src/Containers/container.jl | 12 +++---- src/Containers/macro.jl | 6 ++-- src/Containers/nested_iterator.jl | 4 ++- src/Containers/vectorized_product_iterator.jl | 32 +++++++++++++++++++ src/macros.jl | 2 +- test/Containers/macro.jl | 6 ++++ 8 files changed, 53 insertions(+), 13 deletions(-) create mode 100644 src/Containers/vectorized_product_iterator.jl diff --git a/src/Containers/Containers.jl b/src/Containers/Containers.jl index 32d11c4f7fb..7f91aca1362 100644 --- a/src/Containers/Containers.jl +++ b/src/Containers/Containers.jl @@ -26,6 +26,8 @@ export DenseAxisArray, SparseAxisArray include("DenseAxisArray.jl") include("SparseAxisArray.jl") include("generate_container.jl") +include("vectorized_product_iterator.jl") +include("nested_iterator.jl") include("container.jl") include("macro.jl") diff --git a/src/Containers/SparseAxisArray.jl b/src/Containers/SparseAxisArray.jl index 001bdcaa522..bb746b230ad 100644 --- a/src/Containers/SparseAxisArray.jl +++ b/src/Containers/SparseAxisArray.jl @@ -3,8 +3,6 @@ # License, v. 2.0. If a copy of the MPL was not distributed with this # file, You can obtain one at http://mozilla.org/MPL/2.0/. -include("nested_iterator.jl") - """ struct SparseAxisArray{T,N,K<:NTuple{N, Any}} <: AbstractArray{T,N} data::Dict{K,T} diff --git a/src/Containers/container.jl b/src/Containers/container.jl index bef17066c5e..5a9ac5ddb9e 100644 --- a/src/Containers/container.jl +++ b/src/Containers/container.jl @@ -1,4 +1,4 @@ -const ArrayIndices{N} = Iterators.ProductIterator{NTuple{N, Base.OneTo{Int}}} +const ArrayIndices{N} = VectorizedProductIterator{NTuple{N, Base.OneTo{Int}}} container(f::Function, indices) = container(f, indices, default_container(indices)) default_container(::ArrayIndices) = Array function container(f::Function, indices::ArrayIndices, ::Type{Array}) @@ -10,14 +10,14 @@ function _oneto(indices) end error("Index set for array is not one-based interval.") end -function container(f::Function, indices::Iterators.ProductIterator, +function container(f::Function, indices::VectorizedProductIterator, ::Type{Array}) - container(f, Iterators.ProductIterator(_oneto.(indices.iterators)), Array) + container(f, vectorized_product(_oneto.(indices.prod.iterators)...), Array) end -default_container(::Iterators.ProductIterator) = DenseAxisArray -function container(f::Function, indices::Iterators.ProductIterator, +default_container(::VectorizedProductIterator) = DenseAxisArray +function container(f::Function, indices::VectorizedProductIterator, ::Type{DenseAxisArray}) - return DenseAxisArray(map(I -> f(I...), indices), indices.iterators...) + return DenseAxisArray(map(I -> f(I...), indices), indices.prod.iterators...) end default_container(::NestedIterator) = SparseAxisArray function container(f::Function, indices, diff --git a/src/Containers/macro.jl b/src/Containers/macro.jl index 14380b36f30..68df9f63d66 100644 --- a/src/Containers/macro.jl +++ b/src/Containers/macro.jl @@ -135,13 +135,13 @@ function _build_ref_sets(_error::Function, expr) esc_idxvars = esc.(idxvars) idxfuns = [:(($(esc_idxvars[1:(i - 1)]...),) -> $(idxsets[i])) for i in 1:length(idxvars)] if condition == :() - indices = :(Containers.NestedIterator(($(idxfuns...),))) + indices = :(Containers.nested($(idxfuns...))) else condition_fun = :(($(esc_idxvars...),) -> $(esc(condition))) - indices = :(Containers.NestedIterator(($(idxfuns...),), $condition_fun)) + indices = :(Containers.nested($(idxfuns...); condition = $condition_fun)) end else - indices = :(Base.Iterators.product(($(_explicit_oneto.(idxsets)...)))) + indices = :(Containers.vectorized_product($(_explicit_oneto.(idxsets)...))) end return idxvars, indices end diff --git a/src/Containers/nested_iterator.jl b/src/Containers/nested_iterator.jl index 706a130e459..165f0b5bb65 100644 --- a/src/Containers/nested_iterator.jl +++ b/src/Containers/nested_iterator.jl @@ -23,7 +23,9 @@ struct NestedIterator{T} iterators::T # Tuple of functions condition::Function end -NestedIterator(iterator) = NestedIterator(iterator, (args...) -> true) +function nested(iterators...; condition = (args...) -> true) + return NestedIterator(iterators, condition) +end Base.IteratorSize(::Type{<:NestedIterator}) = Base.SizeUnknown() Base.IteratorEltype(::Type{<:NestedIterator}) = Base.EltypeUnknown() function next_iterate(it::NestedIterator, i, elems, states, iterator, elem_state) diff --git a/src/Containers/vectorized_product_iterator.jl b/src/Containers/vectorized_product_iterator.jl new file mode 100644 index 00000000000..2e23960fd00 --- /dev/null +++ b/src/Containers/vectorized_product_iterator.jl @@ -0,0 +1,32 @@ +""" + struct VectorizedProductIterator{T} + prod::Iterators.ProductIterator{T} + end + +Same as `Base.Iterators.ProductIterator` except that it is independent +on the `IteratorSize` of the elements of `prod.iterators`. +For instance: +* `size(Iterators.product(1, 2))` is `tuple()` while + `size(VectorizedProductIterator(1, 2))` is `(1, 1)`. +* `size(Iterators.product(ones(2, 3)))` is `(2, 3)` while + `size(VectorizedProductIterator(ones(2, 3)))` is `(1, 1)`. +""" +struct VectorizedProductIterator{T} + prod::Iterators.ProductIterator{T} +end +function vectorized_product(iterators...) + return VectorizedProductIterator(Iterators.product(iterators...)) +end +function Base.IteratorSize(::Type{<:VectorizedProductIterator{<:Tuple{Vararg{Any, N}}}}) where N + return Base.HasShape{N}() +end +Base.IteratorEltype(::Type{<:VectorizedProductIterator}) = Base.EltypeUnknown() +Base.size(it::VectorizedProductIterator) = _prod_size(it.prod.iterators) +_prod_size(::Tuple{}) = () +_prod_size(t::Tuple) = (length(t[1]), _prod_size(Base.tail(t))...) +Base.axes(it::VectorizedProductIterator) = _prod_indices(it.prod.iterators) +_prod_indices(::Tuple{}) = () +_prod_indices(t::Tuple) = (Base.OneTo(length(t[1])), _prod_indices(Base.tail(t))...) +Base.ndims(it::VectorizedProductIterator) = length(axes(it)) +Base.length(it::VectorizedProductIterator) = prod(size(it)) +Base.iterate(it::VectorizedProductIterator, args...) = iterate(it.prod, args...) diff --git a/src/macros.jl b/src/macros.jl index 15661ba9485..0dc89e969e0 100644 --- a/src/macros.jl +++ b/src/macros.jl @@ -1098,7 +1098,7 @@ above but does it without using the `@variable` macro x = JuMP.Containers.container(i -> begin info = VariableInfo(false, NaN, true, ub[i], false, NaN, false, NaN, false, false) x[i] = JuMP.add_variable(model, JuMP.build_variable(error, info), "x[\$i]") - end, Base.Iterators.product(keys(ub))) + end, JuMP.Containers.vectorized_product(keys(ub))) # output 1-dimensional DenseAxisArray{VariableRef,1,...} with index sets: diff --git a/test/Containers/macro.jl b/test/Containers/macro.jl index 8e953ae94ed..7fe774a8170 100644 --- a/test/Containers/macro.jl +++ b/test/Containers/macro.jl @@ -14,6 +14,12 @@ using JuMP.Containers @test x isa Containers.DenseAxisArray{Int, 1} Containers.@container(x[i = 2:3, j = 1:2], i + j) @test x isa Containers.DenseAxisArray{Int, 2} + Containers.@container(x[4], 0.0) + @test x isa Containers.DenseAxisArray{Float64, 1} + Containers.@container(x[4, 5], 0) + @test x isa Containers.DenseAxisArray{Int, 2} + Containers.@container(x[4, 1:3, 5], 0) + @test x isa Containers.DenseAxisArray{Int, 3} end @testset "SparseAxisArray" begin Containers.@container(x[i = 1:3, j = 1:i], i + j) From 974dfeeabafaec1102c6da03a218a941d2b797d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Legat?= Date: Tue, 1 Oct 2019 10:42:49 +0200 Subject: [PATCH 5/7] Address comments --- docs/src/containers.md | 25 +++--- src/Containers/container.jl | 67 ++++++++++++++- src/Containers/macro.jl | 83 ++++++++++++++++++- src/Containers/nested_iterator.jl | 5 ++ src/Containers/vectorized_product_iterator.jl | 10 ++- src/macros.jl | 23 ++--- test/Containers/Containers.jl | 2 + test/Containers/macro.jl | 2 +- test/Containers/nested_iterator.jl | 14 ++++ .../Containers/vectorized_product_iterator.jl | 9 ++ 10 files changed, 205 insertions(+), 35 deletions(-) create mode 100644 test/Containers/nested_iterator.jl create mode 100644 test/Containers/vectorized_product_iterator.jl diff --git a/docs/src/containers.md b/docs/src/containers.md index 105bc6b635c..06cbc886b85 100644 --- a/docs/src/containers.md +++ b/docs/src/containers.md @@ -21,10 +21,15 @@ JuMP.Containers.SparseAxisArray Containers in macros -------------------- -The `generate_container` function encodes the logic for how containers are -constructed in JuMP's macros. +The `container` function encodes the logic for how containers are +constructed in JuMP's macros. The `@container` macro is available to create +containers independently of any JuMP model. ```@docs -JuMP.Containers.generate_container +JuMP.Containers.container +JuMP.Containers.default_container +JuMP.Containers.VectorizedProductIterator +JuMP.Containers.NestedIterator +JuMP.Containers.@container ``` In the [`@variable`](@ref) (resp. [`@constraint`](@ref)) macro, containers of @@ -44,12 +49,12 @@ Each expression `index_set_i` can either be keyword arguments to be expressions depending on the `index_name`. The macro then creates the container using the -[`JuMP.Containers.generate_container`](@ref) function with the following +[`JuMP.Containers.container`](@ref) function with the following arguments: -1. `VariableRef` for the [`@variable`](@ref) macro and `ConstraintRef` for the - [`@constraint`](@ref) macro. -2. The index variables and arbitrary symbols for dimensions for which no - variable index is specified. -3. The index sets specified. -4. The value of the `keyword` argument if given or `:Auto`. +1. A function taking as argument the value of the indices and returning the + value to be stored in the container, e.g. a variable for the + [`@variable`](@ref) macro and a constraint for the [`@constraint`](@ref) + macro. +2. An iterator over the indices of the container. +4. The value of the `container` keyword argument if given. diff --git a/src/Containers/container.jl b/src/Containers/container.jl index 5a9ac5ddb9e..19223e6f233 100644 --- a/src/Containers/container.jl +++ b/src/Containers/container.jl @@ -1,5 +1,70 @@ -const ArrayIndices{N} = VectorizedProductIterator{NTuple{N, Base.OneTo{Int}}} +# Copyright 2017, Iain Dunning, Joey Huchette, Miles Lubin, and contributors +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, You can obtain one at http://mozilla.org/MPL/2.0/. + +""" + default_container(indices) + +If `indices` is a [`NestedIterator`](@ref), return a +[`SparseAxisArray`](@ref). Otherwise, `indices` should be +a `VectorizedProductIterator` and the function returns +`Array` if all iterators of the product are `Base.OneTo` and retunrs +[`DenseAxisArray`](@ref) otherwise. +""" +function default_container end + +""" + container(f::Function, indices, ::Type{C}) + +Create a container of type `C` with indices `indices` and values at given +indices given by `f`. + + container(f::Function, indices) + +Create a container with indices `indices` and values at given indices given by +`f`. The type of container used is determined by [`default_container`](@ref). + +## Examples + +```@jldoctest +julia> Containers.container((i, j) -> i + j, Containers.vectorized_product(Base.OneTo(3), Base.OneTo(3))) +3×3 Array{Int64,2}: + 2 3 4 + 3 4 5 + 4 5 6 + +julia> Containers.container((i, j) -> i + j, Containers.vectorized_product(1:3, 1:3)) +2-dimensional DenseAxisArray{Int64,2,...} with index sets: + Dimension 1, 1:3 + Dimension 2, 1:3 +And data, a 3×3 Array{Int64,2}: + 2 3 4 + 3 4 5 + 4 5 6 + +julia> Containers.container((i, j) -> i + j, Containers.vectorized_product(2:3, Base.OneTo(3))) +2-dimensional DenseAxisArray{Int64,2,...} with index sets: + Dimension 1, 2:3 + Dimension 2, Base.OneTo(3) +And data, a 2×3 Array{Int64,2}: + 3 4 5 + 4 5 6 + +julia> Containers.container((i, j) -> i + j, Containers.nested(() -> 1:3, i -> i:3, condition = (i, j) -> isodd(i) || isodd(j))) +SparseAxisArray{Int64,2,Tuple{Int64,Int64}} with 5 entries: + [1, 2] = 3 + [2, 3] = 5 + [3, 3] = 6 + [1, 1] = 2 + [1, 3] = 4 +``` +""" +function container end + container(f::Function, indices) = container(f, indices, default_container(indices)) + +const ArrayIndices{N} = VectorizedProductIterator{NTuple{N, Base.OneTo{Int}}} default_container(::ArrayIndices) = Array function container(f::Function, indices::ArrayIndices, ::Type{Array}) return map(I -> f(I...), indices) diff --git a/src/Containers/macro.jl b/src/Containers/macro.jl index 68df9f63d66..2533fca8d80 100644 --- a/src/Containers/macro.jl +++ b/src/Containers/macro.jl @@ -1,5 +1,21 @@ +# Copyright 2017, Iain Dunning, Joey Huchette, Miles Lubin, and contributors +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, You can obtain one at http://mozilla.org/MPL/2.0/. + using Base.Meta +_get_name(c::Symbol) = c +_get_name(c::Nothing) = () +_get_name(c::AbstractString) = c +function _get_name(c::Expr) + if c.head == :string + return c + else + return c.args[1] + end +end + """ _extract_kw_args(args) @@ -65,8 +81,8 @@ function _parse_ref_sets(_error::Function, expr::Expr) c = copy(expr) idxvars = Any[] idxsets = Any[] - # On 0.7, :(t[i;j]) is a :ref, while t[i,j;j] is a :typed_vcat. - # In both cases :t is the first arg. + # `:(t[i,j;k])` is a :ref, while `:(t[i;j])` is a :typed_vcat. + # In both cases `:t` is the first arg. if isexpr(c, :typed_vcat) || isexpr(c, :ref) popfirst!(c.args) end @@ -176,12 +192,71 @@ function parse_container(_error, var, value, requested_container) return container_code(idxvars, indices, value, requested_container) end +""" + @container([i=..., j=..., ...], expr) + +Create a container with indices `i`, `j`, ... and values given by `expr` that +may depend on the value of the indices. + + @container(ref[i=..., j=..., ...], expr) + +Same as above but the container is assigned to the variable of name `ref`. + +The type of container can be controlled by the `container` keyword. See +[Containers in macros](@ref). Note that when the index set is explicitly +given as `1:n` for any expression `n`, it is transformed to `Base.OneTo(n)` +before being given to [`container`](@ref). + +## Examples + +```jldoctest +julia> Containers.@container([i = 1:3, j = 1:3], i + j) +3×3 Array{Int64,2}: + 2 3 4 + 3 4 5 + 4 5 6 + +julia> I = 1:3 +1:3 + +julia> Containers.@container([i = I, j = I], i + j) +2-dimensional DenseAxisArray{Int64,2,...} with index sets: + Dimension 1, 1:3 + Dimension 2, 1:3 +And data, a 3×3 Array{Int64,2}: + 2 3 4 + 3 4 5 + 4 5 6 + +julia> Containers.@container([i = 2:3, j = 1:3], i + j) +2-dimensional DenseAxisArray{Int64,2,...} with index sets: + Dimension 1, 2:3 + Dimension 2, Base.OneTo(3) +And data, a 2×3 Array{Int64,2}: + 3 4 5 + 4 5 6 + +julia> Containers.@container([i = 1:3, j = 1:3; i <= j], i + j) +JuMP.Containers.SparseAxisArray{Int64,2,Tuple{Int64,Int64}} with 6 entries: + [1, 2] = 3 + [2, 3] = 5 + [3, 3] = 6 + [2, 2] = 4 + [1, 1] = 2 + [1, 3] = 4 +``` +""" macro container(args...) args, kw_args, requested_container = _extract_kw_args(args) @assert length(args) == 2 @assert isempty(kw_args) var, value = args - name = var.args[1] code = parse_container(error, var, esc(value), requested_container) - return :($(esc(name)) = $code) + anonvar = isexpr(var, :vect) || isexpr(var, :vcat) + if anonvar + return code + else + name = Containers._get_name(var) + return :($(esc(name)) = $code) + end end diff --git a/src/Containers/nested_iterator.jl b/src/Containers/nested_iterator.jl index 165f0b5bb65..6c42067b625 100644 --- a/src/Containers/nested_iterator.jl +++ b/src/Containers/nested_iterator.jl @@ -1,3 +1,8 @@ +# Copyright 2017, Iain Dunning, Joey Huchette, Miles Lubin, and contributors +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, You can obtain one at http://mozilla.org/MPL/2.0/. + """ struct NestedIterator{T} iterators::T # Tuple of functions diff --git a/src/Containers/vectorized_product_iterator.jl b/src/Containers/vectorized_product_iterator.jl index 2e23960fd00..dba27a25023 100644 --- a/src/Containers/vectorized_product_iterator.jl +++ b/src/Containers/vectorized_product_iterator.jl @@ -1,10 +1,16 @@ +# Copyright 2017, Iain Dunning, Joey Huchette, Miles Lubin, and contributors +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, You can obtain one at http://mozilla.org/MPL/2.0/. + """ struct VectorizedProductIterator{T} prod::Iterators.ProductIterator{T} end -Same as `Base.Iterators.ProductIterator` except that it is independent -on the `IteratorSize` of the elements of `prod.iterators`. +Cartesian product of the iterators `prod.iterators`. It is the same iterator as +`Base.Iterators.ProductIterator` except that it is independent of the +`IteratorSize` of the elements of `prod.iterators`. For instance: * `size(Iterators.product(1, 2))` is `tuple()` while `size(VectorizedProductIterator(1, 2))` is `(1, 1)`. diff --git a/src/macros.jl b/src/macros.jl index 0dc89e969e0..580b2136c91 100644 --- a/src/macros.jl +++ b/src/macros.jl @@ -41,17 +41,6 @@ function _add_kw_args(call, kw_args) end end -_get_name(c::Symbol) = c -_get_name(c::Nothing) = () -_get_name(c::AbstractString) = c -function _get_name(c::Expr) - if c.head == :string - return c - else - return c.args[1] - end -end - _valid_model(m::AbstractModel, name) = nothing _valid_model(m, name) = error("Expected $name to be a JuMP model, but it has type ", typeof(m)) @@ -383,7 +372,7 @@ function _constraint_macro(args, macro_name::Symbol, parsefun::Function) anonvar = isexpr(c, :vect) || isexpr(c, :vcat) || length(extra) != 1 variable = gensym() - name = _get_name(c) + name = Containers._get_name(c) base_name = anonvar ? "" : string(name) # TODO: support the base_name keyword argument @@ -841,7 +830,7 @@ macro expression(args...) if anonvar macro_code = _macro_return(code) else - macro_code = _macro_assign_and_return(code, variable, _get_name(c), + macro_code = _macro_assign_and_return(code, variable, Containers._get_name(c), model_for_registering = m) end return _assert_valid_model(m, macro_code) @@ -1167,7 +1156,7 @@ macro variable(args...) anonvar && explicit_comparison && _error("Cannot use explicit bounds via >=, <= with an anonymous variable") variable = gensym() # TODO: Should we generate non-empty default names for variables? - name = _get_name(var) + name = Containers._get_name(var) if isempty(base_name_kw_args) base_name = anonvar ? "" : string(name) else @@ -1348,7 +1337,7 @@ macro NLconstraint(m, x, args...) macro_code = _macro_return(creation_code) else macro_code = _macro_assign_and_return(creation_code, variable, - _get_name(c), + Containers._get_name(c), model_for_registering = esc_m) end return _assert_valid_model(esc_m, macro_code) @@ -1397,7 +1386,7 @@ macro NLexpression(args...) macro_code = _macro_return(creation_code) else macro_code = _macro_assign_and_return(creation_code, variable, - _get_name(c), + Containers._get_name(c), model_for_registering = esc(m)) end return _assert_valid_model(esc(m), macro_code) @@ -1466,6 +1455,6 @@ macro NLparameter(m, ex, extra...) # TODO: NLparameters are not registered in the model because we don't yet # have an anonymous version. macro_code = _macro_assign_and_return(creation_code, variable, - _get_name(c)) + Containers._get_name(c)) return _assert_valid_model(esc_m, macro_code) end diff --git a/test/Containers/Containers.jl b/test/Containers/Containers.jl index 2c5e48ce094..d9f369330ba 100644 --- a/test/Containers/Containers.jl +++ b/test/Containers/Containers.jl @@ -6,5 +6,7 @@ using JuMP.Containers include("DenseAxisArray.jl") include("SparseAxisArray.jl") include("generate_container.jl") + include("nested_iterator.jl") + include("nested_iterator.jl") include("macro.jl") end diff --git a/test/Containers/macro.jl b/test/Containers/macro.jl index 7fe774a8170..ed40a07ea21 100644 --- a/test/Containers/macro.jl +++ b/test/Containers/macro.jl @@ -6,7 +6,7 @@ using JuMP.Containers @testset "Array" begin Containers.@container(x[i = 1:3], i^2) @test x isa Vector{Int} - Containers.@container(x[i = 1:3, j = 1:3], i^2) + x = Containers.@container([i = 1:3, j = 1:3], i^2) @test x isa Matrix{Int} end @testset "DenseAxisArray" begin diff --git a/test/Containers/nested_iterator.jl b/test/Containers/nested_iterator.jl new file mode 100644 index 00000000000..bb6e35942bb --- /dev/null +++ b/test/Containers/nested_iterator.jl @@ -0,0 +1,14 @@ +@testset "Nested Iterator" begin + iterators = (() -> 1:3, i -> 1:i) + condition(i, j) = j > i + @test isempty(Containers.nested(iterators..., condition=condition)) + @test isempty(collect(Containers.nested(iterators..., condition=condition))) + condition(i, j) = isodd(i) || isodd(j) + @test collect(Containers.nested(iterators..., condition=condition)) == [ + (1, 1) + (2, 1) + (3, 1) + (3, 2) + (3, 3) + ] +end diff --git a/test/Containers/vectorized_product_iterator.jl b/test/Containers/vectorized_product_iterator.jl new file mode 100644 index 00000000000..f4109d9b495 --- /dev/null +++ b/test/Containers/vectorized_product_iterator.jl @@ -0,0 +1,9 @@ +@testset "Vectorized Product Iterator" begin + I = [1 2 + 3 4] + @test isempty(Containers.vectorized_product(2, I, 1:0)) + @test isempty(collect(Containers.vectorized_product(2, I, 1:0))) + @test collect(Containers.vectorized_product(2, I)) == [ + (2, 1) (2, 3) (2, 2), (2, 4) + ] +end From 61d5e1a3175d7aa72b43a57e911b08dfe98e3d0d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Legat?= Date: Wed, 2 Oct 2019 10:36:53 +0200 Subject: [PATCH 6/7] Add a longer comment --- src/Containers/vectorized_product_iterator.jl | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/src/Containers/vectorized_product_iterator.jl b/src/Containers/vectorized_product_iterator.jl index dba27a25023..7cb33f832e1 100644 --- a/src/Containers/vectorized_product_iterator.jl +++ b/src/Containers/vectorized_product_iterator.jl @@ -3,6 +3,30 @@ # License, v. 2.0. If a copy of the MPL was not distributed with this # file, You can obtain one at http://mozilla.org/MPL/2.0/. +# With `Iterators.ProductIterator`, everything works except when the +# `IteratorSize` of an iterator is not `Base.HasShape{1}` or `Base.HasLength` +# which is notably the case for scalars for it is `Base.HasShape{0}` and +# multidimensional arrays for which it is `Base.HasShape{N}` where `N` is the +# dimension. For instance: +# ```julia +# julia> collect(Iterators.product(2, 3)) +# 0-dimensional Array{Tuple{Int64,Int64},0}: +# (2, 3) +# ``` +# while we could like it to be a `3`-dimensional array of size `(1, 1)`. +# When the user does `@container([2, 3], 1)`, a `DenseAxisArray` of size +# `(1, 1)`. Another example: +# ```julia +# julia> collect(Iterators.product([1 2; 3 4])) +# 2×2 Array{Tuple{Int64},2}: +# (1,) (2,) +# (3,) (4,) +# ``` +# while we need the size to be `(4,)`, not `(2, 2)` when the user does +# `@container([i = [1, 2; 3 4]], i^2)`. +# Long story short, we want to tried everything as a interator without shape +# while `Iterators.ProductIterator` does care about preserving the shape +# when doing the cartesian product. """ struct VectorizedProductIterator{T} prod::Iterators.ProductIterator{T} From bc881f725a4662c7c2b700f01b56e34edd9a9d64 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Legat?= Date: Wed, 2 Oct 2019 16:00:07 +0200 Subject: [PATCH 7/7] Add axis_constraints.jl benchmark --- src/Containers/container.jl | 1 - test/perf/axis_constraints.jl | 57 +++++++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+), 1 deletion(-) create mode 100644 test/perf/axis_constraints.jl diff --git a/src/Containers/container.jl b/src/Containers/container.jl index 19223e6f233..f703e695cf5 100644 --- a/src/Containers/container.jl +++ b/src/Containers/container.jl @@ -99,7 +99,6 @@ function container(f::Function, indices, end push!(unique_indices, index) end - # TODO compute idx error("Repeated index ", duplicate, ". Index sets must have unique elements.") end return SparseAxisArray(Dict(data)) diff --git a/test/perf/axis_constraints.jl b/test/perf/axis_constraints.jl new file mode 100644 index 00000000000..2e70e591e1b --- /dev/null +++ b/test/perf/axis_constraints.jl @@ -0,0 +1,57 @@ +using JuMP +using BenchmarkTools + +function sum_iterate(con_refs) + x = 0.0 + for con_ref in con_refs + x += dual(con_ref) + end + return x +end + +function sum_index(con_refs) + x = 0.0 + for i in eachindex(con_refs) + x += dual(con_refs[i]) + end + return x +end + +function dense_axis_constraints(n) + model = Model() + mock = MOIU.MockOptimizer(MOIU.Model{Float64}(), + eval_variable_constraint_dual=false) + MOIU.set_mock_optimize!(mock, + mock -> MOIU.mock_optimize!(mock, zeros(n), + (MOI.SingleVariable, MOI.EqualTo{Float64}) => ones(n - 1))) + MOIU.reset_optimizer(model, mock) + + @variable(model, x[1:n]) + set = MOI.EqualTo(0.0) + @time @constraint(model, con_refs[i = 2:n], x[i] in set) + optimize!(model) + @assert sum_iterate(con_refs) == n - 1 + @btime sum_iterate($con_refs) + @assert sum_index(con_refs) == n - 1 + @btime sum_index($con_refs) + return +end +function sparse_axis_constraints(n) + model = Model() + mock = MOIU.MockOptimizer(MOIU.Model{Float64}(), + eval_variable_constraint_dual=false) + MOIU.set_mock_optimize!(mock, + mock -> MOIU.mock_optimize!(mock, zeros(n), + (MOI.SingleVariable, MOI.EqualTo{Float64}) => ones(div(n, 2)))) + MOIU.reset_optimizer(model, mock) + + @variable(model, x[1:n]) + set = MOI.EqualTo(0.0) + @time @constraint(model, con_refs[i = 1:n; iseven(i)], x[i] in set) + optimize!(model) + @assert sum_iterate(con_refs) == div(n, 2) + @btime sum_iterate($con_refs) + @assert sum_index(con_refs) == div(n, 2) + @btime sum_index($con_refs) + return +end