From 9314e0114889d03a467947cbf9f395a9170edf0d Mon Sep 17 00:00:00 2001 From: odow Date: Wed, 5 May 2021 16:13:16 +1200 Subject: [PATCH 01/10] Lazily create storage in VectorOfConstraints --- src/Utilities/vector_of_constraints.jl | 114 ++++++++++++++++--------- 1 file changed, 76 insertions(+), 38 deletions(-) diff --git a/src/Utilities/vector_of_constraints.jl b/src/Utilities/vector_of_constraints.jl index dd6236079d..64890cf0f3 100644 --- a/src/Utilities/vector_of_constraints.jl +++ b/src/Utilities/vector_of_constraints.jl @@ -15,54 +15,72 @@ # vector, it readily gives the entries of `model.constrmap` that need to be # updated. -struct VectorOfConstraints{F<:MOI.AbstractFunction,S<:MOI.AbstractSet} <: - MOI.ModelLike - # FIXME: It is not ideal that we have `DataType` here, it might induce type - # instabilities. We should change `CleverDicts` so that we can just - # use `typeof(CleverDicts.index_to_key)` here. - constraints::CleverDicts.CleverDict{ - MOI.ConstraintIndex{F,S}, - Tuple{F,S}, - typeof(CleverDicts.key_to_index), - typeof(CleverDicts.index_to_key), - } - - function VectorOfConstraints{F,S}() where {F,S} - return new{F,S}( - CleverDicts.CleverDict{MOI.ConstraintIndex{F,S},Tuple{F,S}}(), - ) - end +const _VectorOfConstraintsCleverDict{F,S} = CleverDicts.CleverDict{ + MOI.ConstraintIndex{F,S}, + Tuple{F,S}, + typeof(CleverDicts.key_to_index), + typeof(CleverDicts.index_to_key), +} + +mutable struct VectorOfConstraints{ + F<:MOI.AbstractFunction, + S<:MOI.AbstractSet, +} <: MOI.ModelLike + # We create a lot of these objects (one for each function-set pair)! Since + # most users will probably only use a few of these, we can optimize our + # storage by using a `Union{Nothing,X}`. + constraints::Union{Nothing,_VectorOfConstraintsCleverDict{F,S}} + + VectorOfConstraints{F,S}() where {F,S} = new{F,S}(nothing) end -MOI.is_empty(v::VectorOfConstraints) = isempty(v.constraints) -MOI.empty!(v::VectorOfConstraints) = empty!(v.constraints) +MOI.is_empty(v::VectorOfConstraints) = v.constraints === nothing +MOI.empty!(v::VectorOfConstraints) = (v.constraints = nothing) function MOI.supports_constraint( - v::VectorOfConstraints{F,S}, + ::VectorOfConstraints{F,S}, ::Type{F}, ::Type{S}, ) where {F<:MOI.AbstractFunction,S<:MOI.AbstractSet} return true end + +""" + _constraints(v::VectorOfConstraints{F,S}) where {F,S} + +Return a type-stable reference to the constraints in `v`. Throws an error if the +field is `nothing`. +""" +function _constraints(v::VectorOfConstraints{F,S}) where {F,S} + return v.constraints::_VectorOfConstraintsCleverDict{F,S} +end + function MOI.add_constraint( v::VectorOfConstraints{F,S}, func::F, set::S, ) where {F<:MOI.AbstractFunction,S<:MOI.AbstractSet} + if MOI.is_empty(v) + v.constraints = + CleverDicts.CleverDict{MOI.ConstraintIndex{F,S},Tuple{F,S}}() + end # We canonicalize the constraint so that solvers can avoid having to # canonicalize it most of the time (they can check if they need to with # `is_canonical`. # Note that the canonicalization is not guaranteed if for instance # `modify` is called and adds a new term. # See https://github.com/jump-dev/MathOptInterface.jl/pull/1118 - return CleverDicts.add_item(v.constraints, (canonical(func), copy(set))) + return CleverDicts.add_item(_constraints(v), (canonical(func), copy(set))) end function MOI.is_valid( v::VectorOfConstraints{F,S}, ci::MOI.ConstraintIndex{F,S}, ) where {F,S} - return haskey(v.constraints, ci) + if MOI.is_empty(v) + return false + end + return haskey(_constraints(v), ci) end function MOI.delete( @@ -70,7 +88,11 @@ function MOI.delete( ci::MOI.ConstraintIndex{F,S}, ) where {F,S} MOI.throw_if_not_valid(v, ci) - delete!(v.constraints, ci) + cons = _constraints(v) + delete!(cons, ci) + if length(cons) == 0 + MOI.empty!(v) + end return end @@ -80,7 +102,7 @@ function MOI.get( ci::MOI.ConstraintIndex{F,S}, ) where {F,S} MOI.throw_if_not_valid(v, ci) - return v.constraints[ci][1] + return _constraints(v)[ci][1] end function MOI.get( @@ -89,7 +111,7 @@ function MOI.get( ci::MOI.ConstraintIndex{F,S}, ) where {F,S} MOI.throw_if_not_valid(v, ci) - return v.constraints[ci][2] + return _constraints(v)[ci][2] end function MOI.set( @@ -99,7 +121,8 @@ function MOI.set( func::F, ) where {F,S} MOI.throw_if_not_valid(v, ci) - v.constraints[ci] = (func, v.constraints[ci][2]) + cons = _constraints(v) + cons[ci] = (func, cons[ci][2]) return end @@ -110,7 +133,8 @@ function MOI.set( set::S, ) where {F,S} MOI.throw_if_not_valid(v, ci) - v.constraints[ci] = (v.constraints[ci][1], set) + cons = _constraints(v) + cons[ci] = (cons[ci][1], set) return end @@ -118,21 +142,21 @@ function MOI.get( v::VectorOfConstraints{F,S}, ::MOI.ListOfConstraintTypesPresent, )::Vector{Tuple{DataType,DataType}} where {F,S} - return isempty(v.constraints) ? [] : [(F, S)] + return MOI.is_empty(v) ? [] : [(F, S)] end function MOI.get( v::VectorOfConstraints{F,S}, ::MOI.NumberOfConstraints{F,S}, ) where {F,S} - return length(v.constraints) + return MOI.is_empty(v) ? 0 : length(_constraints(v)) end function MOI.get( v::VectorOfConstraints{F,S}, ::MOI.ListOfConstraintIndices{F,S}, ) where {F,S} - return keys(v.constraints) + return MOI.is_empty(v) ? MOI.ConstraintIndex{F,S}[] : keys(_constraints(v)) end function MOI.modify( @@ -140,21 +164,28 @@ function MOI.modify( ci::MOI.ConstraintIndex{F,S}, change::MOI.AbstractFunctionModification, ) where {F,S} - func, set = v.constraints[ci] - v.constraints[ci] = (modify_function(func, change), set) + cons = _constraints(v) + func, set = cons[ci] + cons[ci] = (modify_function(func, change), set) return end # Deletion of variables in vector of variables function _remove_variable(v::VectorOfConstraints, vi::MOI.VariableIndex) - CleverDicts.map_values!(v.constraints) do (f, s) + if MOI.is_empty(v) + return + end + CleverDicts.map_values!(_constraints(v)) do (f, s) return remove_variable(f, s, vi) end return end -function _filter_variables(keep::F, v::VectorOfConstraints) where {F<:Function} - CleverDicts.map_values!(v.constraints) do (f, s) +function _filter_variables(keep::Function, v::VectorOfConstraints) + if MOI.is_empty(v) + return + end + CleverDicts.map_values!(_constraints(v)) do (f, s) return filter_variables(keep, f, s) end return @@ -176,10 +207,10 @@ function _throw_if_cannot_delete( vis, fast_in_vis, ) where {S<:MOI.AbstractVectorSet} - if MOI.supports_dimension_update(S) + if MOI.supports_dimension_update(S) || MOI.is_empty(v) return end - for fs in values(v.constraints) + for fs in values(_constraints(v)) f = fs[1]::MOI.VectorOfVariables if length(f.variables) > 1 && f.variables != vis for vi in f.variables @@ -207,7 +238,11 @@ function _delete_variables( v::VectorOfConstraints{MOI.VectorOfVariables,<:MOI.AbstractVectorSet}, vis::Vector{MOI.VariableIndex}, ) where {F<:Function} - filter!(v.constraints) do p + if MOI.is_empty(v) + return + end + cons = _constraints(v) + filter!(cons) do p f = p.second[1] del = if length(f.variables) == 1 first(f.variables) in vis @@ -219,6 +254,9 @@ function _delete_variables( end return !del end + if length(cons) == 0 + MOI.empty!(v) + end return end From 86c0e0f476c4842ee3f99aa8f1bcdeee36357c44 Mon Sep 17 00:00:00 2001 From: odow Date: Thu, 6 May 2021 17:10:07 +1200 Subject: [PATCH 02/10] Use Nothing default for StructOfConstraints --- src/Utilities/struct_of_constraints.jl | 158 ++++++++++++------------- 1 file changed, 76 insertions(+), 82 deletions(-) diff --git a/src/Utilities/struct_of_constraints.jl b/src/Utilities/struct_of_constraints.jl index dd38494ea2..197baae977 100644 --- a/src/Utilities/struct_of_constraints.jl +++ b/src/Utilities/struct_of_constraints.jl @@ -2,8 +2,12 @@ abstract type StructOfConstraints <: MOI.ModelLike end function _throw_if_cannot_delete(model::StructOfConstraints, vis, fast_in_vis) broadcastcall(model) do constrs - return _throw_if_cannot_delete(constrs, vis, fast_in_vis) + if constrs !== nothing + _throw_if_cannot_delete(constrs, vis, fast_in_vis) + end + return end + return end function _deleted_constraints( callback::Function, @@ -11,24 +15,23 @@ function _deleted_constraints( vi, ) broadcastcall(model) do constrs - return _deleted_constraints(callback, constrs, vi) + if constrs !== nothing + _deleted_constraints(callback, constrs, vi) + end + return end + return end function MOI.add_constraint( model::StructOfConstraints, - func::MOI.AbstractFunction, - set::MOI.AbstractSet, -) - if MOI.supports_constraint(model, typeof(func), typeof(set)) - return MOI.add_constraint( - constraints(model, typeof(func), typeof(set)), - func, - set, - ) - else - throw(MOI.UnsupportedConstraint{typeof(func),typeof(set)}()) + func::F, + set::S, +) where {F<:MOI.AbstractFunction,S<:MOI.AbstractSet} + if !MOI.supports_constraint(model, F, S) + throw(MOI.UnsupportedConstraint{F,S}()) end + return MOI.add_constraint(constraints(model, F, S), func, set) end function constraints( @@ -49,18 +52,18 @@ function MOI.get( end function MOI.delete(model::StructOfConstraints, ci::MOI.ConstraintIndex) - return MOI.delete(constraints(model, ci), ci) + MOI.delete(constraints(model, ci), ci) + return end function MOI.is_valid( model::StructOfConstraints, ci::MOI.ConstraintIndex{F,S}, ) where {F,S} - if MOI.supports_constraint(model, F, S) - return MOI.is_valid(constraints(model, ci), ci) - else + if !MOI.supports_constraint(model, F, S) return false end + return MOI.is_valid(constraints(model, ci), ci) end function MOI.modify( @@ -68,7 +71,8 @@ function MOI.modify( ci::MOI.ConstraintIndex, change::MOI.AbstractFunctionModification, ) - return MOI.modify(constraints(model, ci), ci, change) + MOI.modify(constraints(model, ci), ci, change) + return end function MOI.set( @@ -77,45 +81,55 @@ function MOI.set( ci::MOI.ConstraintIndex, func_or_set, ) - return MOI.set(constraints(model, ci), attr, ci, func_or_set) + MOI.set(constraints(model, ci), attr, ci, func_or_set) + return end function MOI.get( model::StructOfConstraints, - loc::MOI.ListOfConstraintTypesPresent, -) where {T} - return broadcastvcat(model) do v - return MOI.get(v, loc) + attr::MOI.ListOfConstraintTypesPresent, +) + return broadcastvcat(model) do constrs + if constrs === nothing + return Tuple{DataType,DataType}[] + end + return MOI.get(constrs, attr) end end function MOI.get( model::StructOfConstraints, - noc::MOI.NumberOfConstraints{F,S}, + attr::MOI.NumberOfConstraints{F,S}, ) where {F,S} - if MOI.supports_constraint(model, F, S) - return MOI.get(constraints(model, F, S), noc) - else + if !MOI.supports_constraint(model, F, S) return 0 end + return MOI.get(constraints(model, F, S), attr) end function MOI.get( model::StructOfConstraints, - loc::MOI.ListOfConstraintIndices{F,S}, + attr::MOI.ListOfConstraintIndices{F,S}, ) where {F,S} - if MOI.supports_constraint(model, F, S) - return MOI.get(constraints(model, F, S), loc) - else + if !MOI.supports_constraint(model, F, S) return MOI.ConstraintIndex{F,S}[] end + return MOI.get(constraints(model, F, S), attr) end function MOI.is_empty(model::StructOfConstraints) - return mapreduce_constraints(MOI.is_empty, &, model, true) + return mapreduce_constraints(&, model, true) do constrs + return constrs === nothing || MOI.is_empty(constrs) + end end function MOI.empty!(model::StructOfConstraints) - return broadcastcall(MOI.empty!, model) + broadcastcall(model) do constrs + if constrs !== nothing + MOI.empty!(constrs) + end + return + end + return end # Can be used to access constraints of a model @@ -178,33 +192,13 @@ end # (MOI, :Zeros) -> :(MOI.Zeros) # (:Zeros) -> :(MOI.Zeros) -_set(s::SymbolSet) = esc(s.s) -_fun(s::SymbolFun) = esc(s.s) -function _typedset(s::SymbolSet) - if s.typed - T = esc(:T) - :($(_set(s)){$T}) - else - _set(s) - end -end -function _typedfun(s::SymbolFun) - if s.typed - T = esc(:T) - :($(_fun(s)){$T}) - else - _fun(s) - end -end +_typedset(s::SymbolSet) = s.typed ? Expr(:curly, esc(s.s), esc(:T)) : esc(s.s) +_typedfun(s::SymbolFun) = s.typed ? Expr(:curly, esc(s.s), esc(:T)) : esc(s.s) # Base.lowercase is moved to Unicode.lowercase in Julia v0.7 using Unicode _field(s::SymbolFS) = Symbol(replace(lowercase(string(s.s)), "." => "_")) - -_getC(s::SymbolSet) = :(VectorOfConstraints{F,$(_typedset(s))}) -_getC(s::SymbolFun) = _typedfun(s) - _callfield(f, s::SymbolFS) = :($f(model.$(_field(s)))) _broadcastfield(b, s::SymbolFS) = :($b(f, model.$(_field(s)))) _mapreduce_field(s::SymbolFS) = :(cur = op(cur, f(model.$(_field(s))))) @@ -223,62 +217,61 @@ If `types` is vector of `SymbolFun` (resp. `SymbolSet`) then the constraints of that function (resp. set) type are stored in the corresponding field. """ function struct_of_constraint_code(struct_name, types, field_types = nothing) - esc_struct_name = struct_name T = esc(:T) - typed_struct = :($(esc_struct_name){$T}) + typed_struct = :($(struct_name){$T}) type_parametrized = field_types === nothing if type_parametrized field_types = [Symbol("C$i") for i in eachindex(types)] append!(typed_struct.args, field_types) end + struct_def = :(mutable struct $typed_struct <: StructOfConstraints end) - struct_def = :(struct $typed_struct <: StructOfConstraints end) - - for (t, field_type) in zip(types, field_types) - field = _field(t) - push!(struct_def.args[3].args, :($field::$field_type)) - end code = quote - function $MOIU.broadcastcall(f::Function, model::$esc_struct_name) - return $(Expr(:block, _callfield.(Ref(:f), types)...)) + function $MOIU.broadcastcall(f::Function, model::$struct_name) + $(Expr(:block, _callfield.(Ref(:f), types)...)) + return end - function $MOIU.broadcastvcat(f::Function, model::$esc_struct_name) + + function $MOIU.broadcastvcat(f::Function, model::$struct_name) return vcat($(_callfield.(Ref(:f), types)...)) end + function $MOIU.mapreduce_constraints( f::Function, op::Function, - model::$esc_struct_name, + model::$struct_name, cur, ) return $(Expr(:block, _mapreduce_field.(types)...)) end end - for t in types - if t isa SymbolFun - fun = _fun(t) - set = :(MOI.AbstractSet) - else - fun = :(MOI.AbstractFunction) - set = _set(t) - end + for (t, field_type) in zip(types, field_types) field = _field(t) - code = quote - $code + push!(struct_def.args[3].args, :($field::Union{Nothing,$field_type})) + fun = t isa SymbolFun ? esc(t.s) : :(MOI.AbstractFunction) + set = t isa SymbolFun ? :(MOI.AbstractSet) : esc(t.s) + constraints_code = :( function $MOIU.constraints( - model::$esc_struct_name, + model::$typed_struct, ::Type{<:$fun}, ::Type{<:$set}, - ) + ) where {$T} + if model.$field === nothing + model.$field = $(field_type)() + end return model.$field end + ) + if type_parametrized + append!(constraints_code.args[1].args, field_types) end + push!(code.args, constraints_code) end supports_code = if eltype(types) <: SymbolFun quote function $MOI.supports_constraint( - model::$esc_struct_name{$T}, + model::$struct_name{$T}, ::Type{F}, ::Type{S}, ) where { @@ -293,7 +286,7 @@ function struct_of_constraint_code(struct_name, types, field_types = nothing) @assert eltype(types) <: SymbolSet quote function $MOI.supports_constraint( - model::$esc_struct_name{$T}, + model::$struct_name{$T}, ::Type{F}, ::Type{S}, ) where { @@ -307,7 +300,8 @@ function struct_of_constraint_code(struct_name, types, field_types = nothing) end expr = Expr(:block, struct_def, supports_code, code) if !isempty(field_types) - constructors = [:($field_type()) for field_type in field_types] + constructors = [:(nothing) for field_type in field_types] + # constructors = [:($field_type()) for field_type in field_types] # If there is no field type, the default constructor is sufficient and # adding this constructor will make a `StackOverflow`. constructor_code = :(function $typed_struct() where {$T} From 37da3c178630c7c9603c5e0f6079a8949f9aee82 Mon Sep 17 00:00:00 2001 From: odow Date: Thu, 6 May 2021 17:29:34 +1200 Subject: [PATCH 03/10] Minor tweaks --- src/Utilities/struct_of_constraints.jl | 15 +++++-------- src/Utilities/vector_of_constraints.jl | 31 +++++++++++++------------- 2 files changed, 21 insertions(+), 25 deletions(-) diff --git a/src/Utilities/struct_of_constraints.jl b/src/Utilities/struct_of_constraints.jl index 197baae977..b617a3807a 100644 --- a/src/Utilities/struct_of_constraints.jl +++ b/src/Utilities/struct_of_constraints.jl @@ -184,21 +184,16 @@ struct SymbolSet <: SymbolFS typed::Bool end -# QuoteNode prevents s from being interpolated and keeps it as a symbol -# Expr(:., MOI, s) would be MOI.s -# Expr(:., MOI, $s) would be Expr(:., MOI, EqualTo) -# Expr(:., MOI, :($s)) would be Expr(:., MOI, :EqualTo) -# Expr(:., MOI, :($(QuoteNode(s)))) is Expr(:., MOI, :(:EqualTo)) <- what we want - -# (MOI, :Zeros) -> :(MOI.Zeros) -# (:Zeros) -> :(MOI.Zeros) _typedset(s::SymbolSet) = s.typed ? Expr(:curly, esc(s.s), esc(:T)) : esc(s.s) _typedfun(s::SymbolFun) = s.typed ? Expr(:curly, esc(s.s), esc(:T)) : esc(s.s) # Base.lowercase is moved to Unicode.lowercase in Julia v0.7 -using Unicode +import Unicode + +function _field(s::SymbolFS) + return Symbol(replace(Unicode.lowercase(string(s.s)), "." => "_")) +end -_field(s::SymbolFS) = Symbol(replace(lowercase(string(s.s)), "." => "_")) _callfield(f, s::SymbolFS) = :($f(model.$(_field(s)))) _broadcastfield(b, s::SymbolFS) = :($b(f, model.$(_field(s)))) _mapreduce_field(s::SymbolFS) = :(cur = op(cur, f(model.$(_field(s))))) diff --git a/src/Utilities/vector_of_constraints.jl b/src/Utilities/vector_of_constraints.jl index 64890cf0f3..0b6f14abec 100644 --- a/src/Utilities/vector_of_constraints.jl +++ b/src/Utilities/vector_of_constraints.jl @@ -172,19 +172,16 @@ end # Deletion of variables in vector of variables +"This function assumes that v.constraints is not nothing." function _remove_variable(v::VectorOfConstraints, vi::MOI.VariableIndex) - if MOI.is_empty(v) - return - end CleverDicts.map_values!(_constraints(v)) do (f, s) return remove_variable(f, s, vi) end return end + +"This function assumes that v.constraints is not nothing." function _filter_variables(keep::Function, v::VectorOfConstraints) - if MOI.is_empty(v) - return - end CleverDicts.map_values!(_constraints(v)) do (f, s) return filter_variables(keep, f, s) end @@ -233,14 +230,12 @@ function _delete_variables( return # Nothing to do as it's not `VectorOfVariables` constraints end +"This function assumes that v.constraints is not nothing." function _delete_variables( - callback::F, + callback::Function, v::VectorOfConstraints{MOI.VectorOfVariables,<:MOI.AbstractVectorSet}, vis::Vector{MOI.VariableIndex}, -) where {F<:Function} - if MOI.is_empty(v) - return - end +) cons = _constraints(v) filter!(cons) do p f = p.second[1] @@ -261,10 +256,13 @@ function _delete_variables( end function _deleted_constraints( - callback::F, + callback::Function, v::VectorOfConstraints, vi::MOI.VariableIndex, -) where {F<:Function} +)s + if MOI.is_empty(v) + return + end vis = [vi] _delete_variables(callback, v, vis) _remove_variable(v, vi) @@ -272,10 +270,13 @@ function _deleted_constraints( end function _deleted_constraints( - callback::F, + callback::Function, v::VectorOfConstraints, vis::Vector{MOI.VariableIndex}, -) where {F<:Function} +) + if MOI.is_empty(v) + return + end removed = Set(vis) _delete_variables(callback, v, vis) _filter_variables(vi -> !(vi in removed), v) From 3f381a22a6bbf916c45d93ae41a9cfd8a30f575e Mon Sep 17 00:00:00 2001 From: odow Date: Thu, 6 May 2021 18:38:08 +1200 Subject: [PATCH 04/10] Fix typo --- src/Utilities/vector_of_constraints.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Utilities/vector_of_constraints.jl b/src/Utilities/vector_of_constraints.jl index 0b6f14abec..9e22630a87 100644 --- a/src/Utilities/vector_of_constraints.jl +++ b/src/Utilities/vector_of_constraints.jl @@ -259,7 +259,7 @@ function _deleted_constraints( callback::Function, v::VectorOfConstraints, vi::MOI.VariableIndex, -)s +) if MOI.is_empty(v) return end From f70b74729e9337127f5c7bf9d474199dc1b2781c Mon Sep 17 00:00:00 2001 From: odow Date: Thu, 6 May 2021 19:17:44 +1200 Subject: [PATCH 05/10] Fixes --- src/Utilities/vector_of_constraints.jl | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/Utilities/vector_of_constraints.jl b/src/Utilities/vector_of_constraints.jl index 9e22630a87..a22484a4e6 100644 --- a/src/Utilities/vector_of_constraints.jl +++ b/src/Utilities/vector_of_constraints.jl @@ -249,9 +249,6 @@ function _delete_variables( end return !del end - if length(cons) == 0 - MOI.empty!(v) - end return end @@ -263,8 +260,7 @@ function _deleted_constraints( if MOI.is_empty(v) return end - vis = [vi] - _delete_variables(callback, v, vis) + _delete_variables(callback, v, [vi]) _remove_variable(v, vi) return end From 6de3f2e5c5539680082bbc613f80f68b8dbdb060 Mon Sep 17 00:00:00 2001 From: odow Date: Thu, 6 May 2021 20:46:35 +1200 Subject: [PATCH 06/10] Fixes --- src/Utilities/vector_of_constraints.jl | 2 +- test/Utilities/model.jl | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Utilities/vector_of_constraints.jl b/src/Utilities/vector_of_constraints.jl index a22484a4e6..e4cbd689b2 100644 --- a/src/Utilities/vector_of_constraints.jl +++ b/src/Utilities/vector_of_constraints.jl @@ -142,7 +142,7 @@ function MOI.get( v::VectorOfConstraints{F,S}, ::MOI.ListOfConstraintTypesPresent, )::Vector{Tuple{DataType,DataType}} where {F,S} - return MOI.is_empty(v) ? [] : [(F, S)] + return (MOI.is_empty(v) || length(_constraints(v)) == 0) ? [] : [(F, S)] end function MOI.get( diff --git a/test/Utilities/model.jl b/test/Utilities/model.jl index ca6c62b217..c91e5f0aca 100644 --- a/test/Utilities/model.jl +++ b/test/Utilities/model.jl @@ -348,6 +348,7 @@ end push!(loc2, (F, S)) end end + _pushloc(::Nothing) = nothing function _pushloc(model::MOI.Utilities.StructOfConstraints) return MOIU.broadcastcall(_pushloc, model) end From fc331cbd0348c88609a1e82b0404bab2906f3ef5 Mon Sep 17 00:00:00 2001 From: odow Date: Thu, 6 May 2021 21:42:22 +1200 Subject: [PATCH 07/10] Fix fix fix --- src/Utilities/vector_of_constraints.jl | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/src/Utilities/vector_of_constraints.jl b/src/Utilities/vector_of_constraints.jl index e4cbd689b2..f4a8415348 100644 --- a/src/Utilities/vector_of_constraints.jl +++ b/src/Utilities/vector_of_constraints.jl @@ -142,7 +142,7 @@ function MOI.get( v::VectorOfConstraints{F,S}, ::MOI.ListOfConstraintTypesPresent, )::Vector{Tuple{DataType,DataType}} where {F,S} - return (MOI.is_empty(v) || length(_constraints(v)) == 0) ? [] : [(F, S)] + return MOI.is_empty(v) ? [] : [(F, S)] end function MOI.get( @@ -249,6 +249,9 @@ function _delete_variables( end return !del end + if length(cons) == 0 + MOI.empty!(v) + end return end @@ -261,6 +264,11 @@ function _deleted_constraints( return end _delete_variables(callback, v, [vi]) + # We need this second check in case deleting the variables also removes the + # last constraint! + if MOI.is_empty(v) + return + end _remove_variable(v, vi) return end @@ -273,8 +281,13 @@ function _deleted_constraints( if MOI.is_empty(v) return end - removed = Set(vis) _delete_variables(callback, v, vis) + # We need this second check in case deleting the variables also removes the + # last constraint! + if MOI.is_empty(v) + return + end + removed = Set(vis) _filter_variables(vi -> !(vi in removed), v) return end From 7fdb1f9a86c40c284dfeac9c9ae23809de7ec0da Mon Sep 17 00:00:00 2001 From: odow Date: Fri, 7 May 2021 10:44:07 +1200 Subject: [PATCH 08/10] Simplify the macro code --- src/Utilities/model.jl | 2 +- src/Utilities/struct_of_constraints.jl | 102 +++++++++---------------- 2 files changed, 39 insertions(+), 65 deletions(-) diff --git a/src/Utilities/model.jl b/src/Utilities/model.jl index 81de8d35f9..0251a95a0e 100644 --- a/src/Utilities/model.jl +++ b/src/Utilities/model.jl @@ -957,7 +957,7 @@ macro model( sets = vector_sets end voc = map(sets) do set - return :(VectorOfConstraints{$(_typedfun(funs[i])),$(_typedset(set))}) + return :(VectorOfConstraints{$(_typed(funs[i])),$(_typed(set))}) end return _struct_of_constraints_type(cname, voc, true) end diff --git a/src/Utilities/struct_of_constraints.jl b/src/Utilities/struct_of_constraints.jl index b617a3807a..fc3c272a86 100644 --- a/src/Utilities/struct_of_constraints.jl +++ b/src/Utilities/struct_of_constraints.jl @@ -9,6 +9,7 @@ function _throw_if_cannot_delete(model::StructOfConstraints, vis, fast_in_vis) end return end + function _deleted_constraints( callback::Function, model::StructOfConstraints, @@ -43,6 +44,7 @@ function constraints( end return constraints(model, F, S) end + function MOI.get( model::StructOfConstraints, attr::Union{MOI.ConstraintFunction,MOI.ConstraintSet}, @@ -122,6 +124,7 @@ function MOI.is_empty(model::StructOfConstraints) return constrs === nothing || MOI.is_empty(constrs) end end + function MOI.empty!(model::StructOfConstraints) broadcastcall(model) do constrs if constrs !== nothing @@ -134,58 +137,48 @@ end # Can be used to access constraints of a model """ -broadcastcall(f::Function, model::AbstractModel) - -Calls `f(contrs)` for every vector `constrs::Vector{ConstraintIndex{F, S}, F, S}` of the model. + broadcastcall(f::Function, model::StructOfConstraints) -# Examples - -To add all constraints of the model to a solver `solver`, one can do -```julia -_addcon(solver, ci, f, s) = MOI.add_constraint(solver, f, s) -function _addcon(solver, constrs::Vector) - for constr in constrs - _addcon(solver, constr...) - end -end -MOIU.broadcastcall(constrs -> _addcon(solver, constrs), model) -``` +Calls `f(contrs)` for every field in `model`. """ function broadcastcall end """ -broadcastvcat(f::Function, model::AbstractModel) + broadcastvcat(f::Function, model::StructOfConstraints) -Calls `f(contrs)` for every vector `constrs::Vector{ConstraintIndex{F, S}, F, S}` of the model and concatenate the results with `vcat` (this is used internally for `ListOfConstraintTypesPresent`). - -# Examples - -To get the list of all functions: -```julia -_getfun(ci, f, s) = f -_getfun(cindices::Tuple) = _getfun(cindices...) -_getfuns(constrs::Vector) = _getfun.(constrs) -MOIU.broadcastvcat(_getfuns, model) -``` +Calls `f(contrs)` for every field in `model` and `vcat`s the results. """ function broadcastvcat end +""" + mapreduce_constraints( + f::Function, + op::Function, + model::StructOfConstraints, + init, + ) + +Call `mapreduce` on every field of `model` given an initial value `init`. Each +element in the map is computed as `f(x)` and the elements are reduced using +`op`. +""" function mapreduce_constraints end # Macro code abstract type SymbolFS end + struct SymbolFun <: SymbolFS s::Union{Symbol,Expr} typed::Bool end + struct SymbolSet <: SymbolFS s::Union{Symbol,Expr} typed::Bool end -_typedset(s::SymbolSet) = s.typed ? Expr(:curly, esc(s.s), esc(:T)) : esc(s.s) -_typedfun(s::SymbolFun) = s.typed ? Expr(:curly, esc(s.s), esc(:T)) : esc(s.s) +_typed(s::SymbolFS) = s.typed ? Expr(:curly, esc(s.s), esc(:T)) : esc(s.s) # Base.lowercase is moved to Unicode.lowercase in Julia v0.7 import Unicode @@ -195,9 +188,8 @@ function _field(s::SymbolFS) end _callfield(f, s::SymbolFS) = :($f(model.$(_field(s)))) -_broadcastfield(b, s::SymbolFS) = :($b(f, model.$(_field(s)))) + _mapreduce_field(s::SymbolFS) = :(cur = op(cur, f(model.$(_field(s))))) -_mapreduce_constraints(s::SymbolFS) = :(cur = op(cur, f(model.$(_field(s))))) """ struct_of_constraint_code(struct_name, types, field_types = nothing) @@ -219,9 +211,9 @@ function struct_of_constraint_code(struct_name, types, field_types = nothing) field_types = [Symbol("C$i") for i in eachindex(types)] append!(typed_struct.args, field_types) end - struct_def = :(mutable struct $typed_struct <: StructOfConstraints end) - code = quote + mutable struct $typed_struct <: StructOfConstraints end + function $MOIU.broadcastcall(f::Function, model::$struct_name) $(Expr(:block, _callfield.(Ref(:f), types)...)) return @@ -243,7 +235,7 @@ function struct_of_constraint_code(struct_name, types, field_types = nothing) for (t, field_type) in zip(types, field_types) field = _field(t) - push!(struct_def.args[3].args, :($field::Union{Nothing,$field_type})) + push!(code.args[2].args[3].args, :($field::Union{Nothing,$field_type})) fun = t isa SymbolFun ? esc(t.s) : :(MOI.AbstractFunction) set = t isa SymbolFun ? :(MOI.AbstractSet) : esc(t.s) constraints_code = :( @@ -263,49 +255,31 @@ function struct_of_constraint_code(struct_name, types, field_types = nothing) end push!(code.args, constraints_code) end - supports_code = if eltype(types) <: SymbolFun - quote - function $MOI.supports_constraint( - model::$struct_name{$T}, - ::Type{F}, - ::Type{S}, - ) where { - $T, - F<:Union{$(_typedfun.(types)...)}, - S<:MOI.AbstractSet, - } - return $MOI.supports_constraint(constraints(model, F, S), F, S) - end - end - else - @assert eltype(types) <: SymbolSet - quote + is_func = eltype(types) <: SymbolFun + SuperF = is_func ? :(Union{$(_typed.(types)...)}) : :(MOI.AbstractFunction) + SuperS = is_func ? :(MOI.AbstractSet) : :(Union{$(_typed.(types)...)}) + push!( + code.args, + :( function $MOI.supports_constraint( model::$struct_name{$T}, ::Type{F}, ::Type{S}, - ) where { - $T, - F<:MOI.AbstractFunction, - S<:Union{$(_typedset.(types)...)}, - } + ) where {$T,F<:$SuperF,S<:$SuperS} return $MOI.supports_constraint(constraints(model, F, S), F, S) end - end - end - expr = Expr(:block, struct_def, supports_code, code) + ), + ) if !isempty(field_types) - constructors = [:(nothing) for field_type in field_types] - # constructors = [:($field_type()) for field_type in field_types] # If there is no field type, the default constructor is sufficient and # adding this constructor will make a `StackOverflow`. constructor_code = :(function $typed_struct() where {$T} - return $typed_struct($(constructors...)) + return $typed_struct($([:(nothing) for _ in field_types]...)) end) if type_parametrized append!(constructor_code.args[1].args, field_types) end - push!(expr.args, constructor_code) + push!(code.args, constructor_code) end - return expr + return code end From 5f20538fcd2b67da26e66dc8433017b3baa81f97 Mon Sep 17 00:00:00 2001 From: odow Date: Fri, 7 May 2021 14:53:42 +1200 Subject: [PATCH 09/10] Fix return type of constraints --- src/Utilities/struct_of_constraints.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Utilities/struct_of_constraints.jl b/src/Utilities/struct_of_constraints.jl index fc3c272a86..ccbadd6ffc 100644 --- a/src/Utilities/struct_of_constraints.jl +++ b/src/Utilities/struct_of_constraints.jl @@ -243,7 +243,7 @@ function struct_of_constraint_code(struct_name, types, field_types = nothing) model::$typed_struct, ::Type{<:$fun}, ::Type{<:$set}, - ) where {$T} + )::$(field_type) where {$T} if model.$field === nothing model.$field = $(field_type)() end From 233a557e0d0bc456201cd0e14a9bd2d164e05af6 Mon Sep 17 00:00:00 2001 From: odow Date: Wed, 12 May 2021 13:11:10 +1200 Subject: [PATCH 10/10] Revert VectorOfConstraint changes --- src/Utilities/vector_of_constraints.jl | 108 +++++++------------------ 1 file changed, 30 insertions(+), 78 deletions(-) diff --git a/src/Utilities/vector_of_constraints.jl b/src/Utilities/vector_of_constraints.jl index f4a8415348..35a9db648f 100644 --- a/src/Utilities/vector_of_constraints.jl +++ b/src/Utilities/vector_of_constraints.jl @@ -15,27 +15,26 @@ # vector, it readily gives the entries of `model.constrmap` that need to be # updated. -const _VectorOfConstraintsCleverDict{F,S} = CleverDicts.CleverDict{ - MOI.ConstraintIndex{F,S}, - Tuple{F,S}, - typeof(CleverDicts.key_to_index), - typeof(CleverDicts.index_to_key), -} - mutable struct VectorOfConstraints{ F<:MOI.AbstractFunction, S<:MOI.AbstractSet, } <: MOI.ModelLike - # We create a lot of these objects (one for each function-set pair)! Since - # most users will probably only use a few of these, we can optimize our - # storage by using a `Union{Nothing,X}`. - constraints::Union{Nothing,_VectorOfConstraintsCleverDict{F,S}} - - VectorOfConstraints{F,S}() where {F,S} = new{F,S}(nothing) + constraints::CleverDicts.CleverDict{ + MOI.ConstraintIndex{F,S}, + Tuple{F,S}, + typeof(CleverDicts.key_to_index), + typeof(CleverDicts.index_to_key), + } + + function VectorOfConstraints{F,S}() where {F,S} + return new{F,S}( + CleverDicts.CleverDict{MOI.ConstraintIndex{F,S},Tuple{F,S}}(), + ) + end end -MOI.is_empty(v::VectorOfConstraints) = v.constraints === nothing -MOI.empty!(v::VectorOfConstraints) = (v.constraints = nothing) +MOI.is_empty(v::VectorOfConstraints) = isempty(v.constraints) +MOI.empty!(v::VectorOfConstraints) = empty!(v.constraints) function MOI.supports_constraint( ::VectorOfConstraints{F,S}, @@ -45,42 +44,25 @@ function MOI.supports_constraint( return true end -""" - _constraints(v::VectorOfConstraints{F,S}) where {F,S} - -Return a type-stable reference to the constraints in `v`. Throws an error if the -field is `nothing`. -""" -function _constraints(v::VectorOfConstraints{F,S}) where {F,S} - return v.constraints::_VectorOfConstraintsCleverDict{F,S} -end - function MOI.add_constraint( v::VectorOfConstraints{F,S}, func::F, set::S, ) where {F<:MOI.AbstractFunction,S<:MOI.AbstractSet} - if MOI.is_empty(v) - v.constraints = - CleverDicts.CleverDict{MOI.ConstraintIndex{F,S},Tuple{F,S}}() - end # We canonicalize the constraint so that solvers can avoid having to # canonicalize it most of the time (they can check if they need to with # `is_canonical`. # Note that the canonicalization is not guaranteed if for instance # `modify` is called and adds a new term. # See https://github.com/jump-dev/MathOptInterface.jl/pull/1118 - return CleverDicts.add_item(_constraints(v), (canonical(func), copy(set))) + return CleverDicts.add_item(v.constraints, (canonical(func), copy(set))) end function MOI.is_valid( v::VectorOfConstraints{F,S}, ci::MOI.ConstraintIndex{F,S}, ) where {F,S} - if MOI.is_empty(v) - return false - end - return haskey(_constraints(v), ci) + return haskey(v.constraints, ci) end function MOI.delete( @@ -88,11 +70,7 @@ function MOI.delete( ci::MOI.ConstraintIndex{F,S}, ) where {F,S} MOI.throw_if_not_valid(v, ci) - cons = _constraints(v) - delete!(cons, ci) - if length(cons) == 0 - MOI.empty!(v) - end + delete!(v.constraints, ci) return end @@ -102,7 +80,7 @@ function MOI.get( ci::MOI.ConstraintIndex{F,S}, ) where {F,S} MOI.throw_if_not_valid(v, ci) - return _constraints(v)[ci][1] + return v.constraints[ci][1] end function MOI.get( @@ -111,7 +89,7 @@ function MOI.get( ci::MOI.ConstraintIndex{F,S}, ) where {F,S} MOI.throw_if_not_valid(v, ci) - return _constraints(v)[ci][2] + return v.constraints[ci][2] end function MOI.set( @@ -121,8 +99,7 @@ function MOI.set( func::F, ) where {F,S} MOI.throw_if_not_valid(v, ci) - cons = _constraints(v) - cons[ci] = (func, cons[ci][2]) + v.constraints[ci] = (func, v.constraints[ci][2]) return end @@ -133,8 +110,7 @@ function MOI.set( set::S, ) where {F,S} MOI.throw_if_not_valid(v, ci) - cons = _constraints(v) - cons[ci] = (cons[ci][1], set) + v.constraints[ci] = (v.constraints[ci][1], set) return end @@ -142,21 +118,21 @@ function MOI.get( v::VectorOfConstraints{F,S}, ::MOI.ListOfConstraintTypesPresent, )::Vector{Tuple{DataType,DataType}} where {F,S} - return MOI.is_empty(v) ? [] : [(F, S)] + return isempty(v.constraints) ? [] : [(F, S)] end function MOI.get( v::VectorOfConstraints{F,S}, ::MOI.NumberOfConstraints{F,S}, ) where {F,S} - return MOI.is_empty(v) ? 0 : length(_constraints(v)) + return length(v.constraints) end function MOI.get( v::VectorOfConstraints{F,S}, ::MOI.ListOfConstraintIndices{F,S}, ) where {F,S} - return MOI.is_empty(v) ? MOI.ConstraintIndex{F,S}[] : keys(_constraints(v)) + return keys(v.constraints) end function MOI.modify( @@ -164,25 +140,22 @@ function MOI.modify( ci::MOI.ConstraintIndex{F,S}, change::MOI.AbstractFunctionModification, ) where {F,S} - cons = _constraints(v) - func, set = cons[ci] - cons[ci] = (modify_function(func, change), set) + func, set = v.constraints[ci] + v.constraints[ci] = (modify_function(func, change), set) return end # Deletion of variables in vector of variables -"This function assumes that v.constraints is not nothing." function _remove_variable(v::VectorOfConstraints, vi::MOI.VariableIndex) - CleverDicts.map_values!(_constraints(v)) do (f, s) + CleverDicts.map_values!(v.constraints) do (f, s) return remove_variable(f, s, vi) end return end -"This function assumes that v.constraints is not nothing." function _filter_variables(keep::Function, v::VectorOfConstraints) - CleverDicts.map_values!(_constraints(v)) do (f, s) + CleverDicts.map_values!(v.constraints) do (f, s) return filter_variables(keep, f, s) end return @@ -207,7 +180,7 @@ function _throw_if_cannot_delete( if MOI.supports_dimension_update(S) || MOI.is_empty(v) return end - for fs in values(_constraints(v)) + for fs in values(v.constraints) f = fs[1]::MOI.VectorOfVariables if length(f.variables) > 1 && f.variables != vis for vi in f.variables @@ -230,14 +203,12 @@ function _delete_variables( return # Nothing to do as it's not `VectorOfVariables` constraints end -"This function assumes that v.constraints is not nothing." function _delete_variables( callback::Function, v::VectorOfConstraints{MOI.VectorOfVariables,<:MOI.AbstractVectorSet}, vis::Vector{MOI.VariableIndex}, ) - cons = _constraints(v) - filter!(cons) do p + filter!(v.constraints) do p f = p.second[1] del = if length(f.variables) == 1 first(f.variables) in vis @@ -249,9 +220,6 @@ function _delete_variables( end return !del end - if length(cons) == 0 - MOI.empty!(v) - end return end @@ -260,15 +228,7 @@ function _deleted_constraints( v::VectorOfConstraints, vi::MOI.VariableIndex, ) - if MOI.is_empty(v) - return - end _delete_variables(callback, v, [vi]) - # We need this second check in case deleting the variables also removes the - # last constraint! - if MOI.is_empty(v) - return - end _remove_variable(v, vi) return end @@ -278,15 +238,7 @@ function _deleted_constraints( v::VectorOfConstraints, vis::Vector{MOI.VariableIndex}, ) - if MOI.is_empty(v) - return - end _delete_variables(callback, v, vis) - # We need this second check in case deleting the variables also removes the - # last constraint! - if MOI.is_empty(v) - return - end removed = Set(vis) _filter_variables(vi -> !(vi in removed), v) return