From 2e16c4305b4a0d7bae5169c3fc0cdc4601028f72 Mon Sep 17 00:00:00 2001 From: Thibaut Cuvelier Date: Fri, 31 Jul 2020 00:50:04 +0200 Subject: [PATCH 01/10] When copying a model, propose to only keep conflict constraints. --- src/Utilities/copy.jl | 48 ++++++++++++++++++++++++++++++++----------- 1 file changed, 36 insertions(+), 12 deletions(-) diff --git a/src/Utilities/copy.jl b/src/Utilities/copy.jl index 91736a80fa..d8b6682573 100644 --- a/src/Utilities/copy.jl +++ b/src/Utilities/copy.jl @@ -10,11 +10,12 @@ Use [`Utilities.supports_default_copy_to`](@ref) and apply the copy operation. """ function automatic_copy_to(dest::MOI.ModelLike, src::MOI.ModelLike; - copy_names::Bool=true) + copy_names::Bool=true, only_conflict::Bool=false) if supports_default_copy_to(dest, copy_names) - default_copy_to(dest, src, copy_names) + default_copy_to(dest, src, copy_names, only_conflict) elseif supports_allocate_load(dest, copy_names) - allocate_load(dest, src, copy_names) + @assert !only_conflict # TODO: implement this. + allocate_load(dest, src, copy_names, only_conflict) else error("Model $(typeof(dest)) does not support copy", copy_names ? " with names" : "", ".") @@ -242,7 +243,8 @@ end """ copy_constraints(dest::MOI.ModelLike, src::MOI.ModelLike, idxmap::IndexMap, - cis_src::Vector{<:MOI.ConstraintIndex}) + cis_src::Vector{<:MOI.ConstraintIndex}, + only_conflict::Bool=false) Copy the constraints `cis_src` from the model `src` to the model `dest` and fill `idxmap` accordingly. Note that the attributes are not copied; call @@ -250,8 +252,20 @@ Copy the constraints `cis_src` from the model `src` to the model `dest` and fill """ function copy_constraints(dest::MOI.ModelLike, src::MOI.ModelLike, idxmap::IndexMap, - cis_src::Vector{<:MOI.ConstraintIndex}) + cis_src::Vector{<:MOI.ConstraintIndex}, + only_conflict::Bool=false) + # Retrieve the constraints to copy. f_src = MOI.get(src, MOI.ConstraintFunction(), cis_src) + + # If need be, filter this set of constraints. + if only_conflict + filter!(cis_src) do cidx + status = MOI.get(src, MOI.ConstraintConflictStatus(), cidx) + status != MOI.NOT_IN_CONFLICT + end + end + + # Copy the constraints into the new model and build the map. f_dest = map_indices.(Ref(idxmap), f_src) s = MOI.get(src, MOI.ConstraintSet(), cis_src) cis_dest = MOI.add_constraints(dest, f_dest, s) @@ -264,7 +278,8 @@ function pass_constraints( dest::MOI.ModelLike, src::MOI.ModelLike, copy_names::Bool, idxmap::IndexMap, single_variable_types, single_variable_indices, vector_of_variables_types, vector_of_variables_indices, - pass_cons=copy_constraints, pass_attr=MOI.set + pass_cons=copy_constraints, pass_attr=MOI.set, + only_conflict::Bool=false ) for (S, cis_src) in zip(single_variable_types, single_variable_indices) @@ -317,14 +332,16 @@ function default_copy_to(dest::MOI.ModelLike, src::MOI.ModelLike) end """ - default_copy_to(dest::MOI.ModelLike, src::MOI.ModelLike, copy_names::Bool) + default_copy_to(dest::MOI.ModelLike, src::MOI.ModelLike, copy_names::Bool, + only_conflict::Bool) Implements `MOI.copy_to(dest, src)` by adding the variables and then the constraints and attributes incrementally. The function [`supports_default_copy_to`](@ref) can be used to check whether `dest` supports the copying a model incrementally. """ -function default_copy_to(dest::MOI.ModelLike, src::MOI.ModelLike, copy_names::Bool) +function default_copy_to(dest::MOI.ModelLike, src::MOI.ModelLike, + copy_names::Bool, only_conflict::Bool=false) MOI.empty!(dest) vis_src = MOI.get(src, MOI.ListOfVariableIndices()) @@ -335,6 +352,11 @@ function default_copy_to(dest::MOI.ModelLike, src::MOI.ModelLike, copy_names::Bo # The `NLPBlock` assumes that the order of variables does not change (#849) if MOI.NLPBlock() in MOI.get(src, MOI.ListOfModelAttributesSet()) + if only_conflict + error("Copying only the constraints participating in a conflict ", + "is not possible for NLP models.") + end + single_variable_types = [S for (F, S) in constraint_types if F == MOI.SingleVariable] vector_of_variables_types = [S for (F, S) in constraint_types @@ -378,7 +400,8 @@ function default_copy_to(dest::MOI.ModelLike, src::MOI.ModelLike, copy_names::Bo # Copy constraints pass_constraints(dest, src, copy_names, idxmap, single_variable_types, single_variable_not_added, - vector_of_variables_types, vector_of_variables_not_added) + vector_of_variables_types, vector_of_variables_not_added, + only_conflict=only_conflict) return idxmap end @@ -684,7 +707,8 @@ Implements `MOI.copy_to(dest, src)` using the Allocate-Load API. The function [`supports_allocate_load`](@ref) can be used to check whether `dest` supports the Allocate-Load API. """ -function allocate_load(dest::MOI.ModelLike, src::MOI.ModelLike, copy_names::Bool) +function allocate_load(dest::MOI.ModelLike, src::MOI.ModelLike, copy_names::Bool, + only_conflict::Bool=false) MOI.empty!(dest) vis_src = MOI.get(src, MOI.ListOfVariableIndices()) @@ -722,7 +746,7 @@ function allocate_load(dest::MOI.ModelLike, src::MOI.ModelLike, copy_names::Bool pass_constraints(dest, src, copy_names, idxmap, single_variable_types, single_variable_not_allocated, vector_of_variables_types, vector_of_variables_not_allocated, - allocate_constraints, allocate) + allocate_constraints, allocate, only_conflict=only_conflict) # Load variables load_variables(dest, length(vis_src)) @@ -743,7 +767,7 @@ function allocate_load(dest::MOI.ModelLike, src::MOI.ModelLike, copy_names::Bool pass_constraints(dest, src, copy_names, idxmap, single_variable_types, single_variable_not_allocated, vector_of_variables_types, vector_of_variables_not_allocated, - load_constraints, load) + load_constraints, load, only_conflict=only_conflict) return idxmap end From ae3ea4a329d9029cce93938e8f4da336944d64a9 Mon Sep 17 00:00:00 2001 From: Thibaut Cuvelier Date: Wed, 12 Aug 2020 02:56:26 +0200 Subject: [PATCH 02/10] Filtering when copying: more generic implementation. --- src/Utilities/copy.jl | 57 +++++++++++++++++++++---------------------- 1 file changed, 28 insertions(+), 29 deletions(-) diff --git a/src/Utilities/copy.jl b/src/Utilities/copy.jl index d8b6682573..baba34c4c3 100644 --- a/src/Utilities/copy.jl +++ b/src/Utilities/copy.jl @@ -2,7 +2,8 @@ """ automatic_copy_to(dest::MOI.ModelLike, src::MOI.ModelLike; - copy_names::Bool=true) + copy_names::Bool=true, + filter_constraints::Function=(() -> true)) Use [`Utilities.supports_default_copy_to`](@ref) and [`Utilities.supports_allocate_load`](@ref) to automatically choose between @@ -10,12 +11,12 @@ Use [`Utilities.supports_default_copy_to`](@ref) and apply the copy operation. """ function automatic_copy_to(dest::MOI.ModelLike, src::MOI.ModelLike; - copy_names::Bool=true, only_conflict::Bool=false) + copy_names::Bool=true, + filter_constraints::Function=((x) -> true)) if supports_default_copy_to(dest, copy_names) - default_copy_to(dest, src, copy_names, only_conflict) + default_copy_to(dest, src, copy_names, filter_constraints) elseif supports_allocate_load(dest, copy_names) - @assert !only_conflict # TODO: implement this. - allocate_load(dest, src, copy_names, only_conflict) + allocate_load(dest, src, copy_names, filter_constraints) else error("Model $(typeof(dest)) does not support copy", copy_names ? " with names" : "", ".") @@ -109,7 +110,7 @@ Base.haskey(idxmap::IndexMap, vi::MOI.VariableIndex) = haskey(idxmap.varmap, vi) Base.keys(idxmap::IndexMap) = Iterators.flatten((keys(idxmap.varmap), keys(idxmap.conmap))) Base.length(idxmap::IndexMap) = length(idxmap.varmap) + length(idxmap.conmap) -Base.iterate(idxmap::MOIU.IndexMap, args...) = iterate(Base.Iterators.flatten((idxmap.varmap, idxmap.conmap)), args...) +Base.iterate(idxmap::IndexMap, args...) = iterate(Base.Iterators.flatten((idxmap.varmap, idxmap.conmap)), args...) """ pass_attributes(dest::MOI.ModelLike, src::MOI.ModelLike, copy_names::Bool, idxmap::IndexMap, pass_attr::Function=MOI.set) @@ -244,26 +245,25 @@ end copy_constraints(dest::MOI.ModelLike, src::MOI.ModelLike, idxmap::IndexMap, cis_src::Vector{<:MOI.ConstraintIndex}, - only_conflict::Bool=false) + filter_constraints::Function=(() -> true)) Copy the constraints `cis_src` from the model `src` to the model `dest` and fill `idxmap` accordingly. Note that the attributes are not copied; call [`pass_attributes`] to copy the constraint attributes. + +If the `filter_constraints` arguments is given, only the constraints for which +this function returns `true` will be copied. This function is given a +constraint index as argument. """ function copy_constraints(dest::MOI.ModelLike, src::MOI.ModelLike, idxmap::IndexMap, cis_src::Vector{<:MOI.ConstraintIndex}, - only_conflict::Bool=false) + filter_constraints::Function=((x) -> true)) # Retrieve the constraints to copy. f_src = MOI.get(src, MOI.ConstraintFunction(), cis_src) - # If need be, filter this set of constraints. - if only_conflict - filter!(cis_src) do cidx - status = MOI.get(src, MOI.ConstraintConflictStatus(), cidx) - status != MOI.NOT_IN_CONFLICT - end - end + # Filter this set of constraints. + filter!(filter_constraints, cis_src) # Copy the constraints into the new model and build the map. f_dest = map_indices.(Ref(idxmap), f_src) @@ -279,7 +279,7 @@ function pass_constraints( single_variable_types, single_variable_indices, vector_of_variables_types, vector_of_variables_indices, pass_cons=copy_constraints, pass_attr=MOI.set, - only_conflict::Bool=false + filter_constraints::Function=((x) -> true) ) for (S, cis_src) in zip(single_variable_types, single_variable_indices) @@ -306,7 +306,7 @@ function pass_constraints( for (F, S) in nonvariable_constraint_types cis_src = MOI.get(src, MOI.ListOfConstraintIndices{F, S}()) # do the rest in `pass_cons` which is type stable - pass_cons(dest, src, idxmap, cis_src) + pass_cons(dest, src, idxmap, cis_src, filter_constraints=filter_constraints) pass_attributes(dest, src, copy_names, idxmap, cis_src, pass_attr) end end @@ -333,7 +333,7 @@ end """ default_copy_to(dest::MOI.ModelLike, src::MOI.ModelLike, copy_names::Bool, - only_conflict::Bool) + filter_constraints::Function=((x) -> true)) Implements `MOI.copy_to(dest, src)` by adding the variables and then the constraints and attributes incrementally. The function @@ -341,7 +341,8 @@ constraints and attributes incrementally. The function the copying a model incrementally. """ function default_copy_to(dest::MOI.ModelLike, src::MOI.ModelLike, - copy_names::Bool, only_conflict::Bool=false) + copy_names::Bool, + filter_constraints::Function=((x) -> true)) MOI.empty!(dest) vis_src = MOI.get(src, MOI.ListOfVariableIndices()) @@ -352,11 +353,6 @@ function default_copy_to(dest::MOI.ModelLike, src::MOI.ModelLike, # The `NLPBlock` assumes that the order of variables does not change (#849) if MOI.NLPBlock() in MOI.get(src, MOI.ListOfModelAttributesSet()) - if only_conflict - error("Copying only the constraints participating in a conflict ", - "is not possible for NLP models.") - end - single_variable_types = [S for (F, S) in constraint_types if F == MOI.SingleVariable] vector_of_variables_types = [S for (F, S) in constraint_types @@ -401,7 +397,7 @@ function default_copy_to(dest::MOI.ModelLike, src::MOI.ModelLike, pass_constraints(dest, src, copy_names, idxmap, single_variable_types, single_variable_not_added, vector_of_variables_types, vector_of_variables_not_added, - only_conflict=only_conflict) + filter_constraints=filter_constraints) return idxmap end @@ -701,14 +697,16 @@ function load_constraints(dest::MOI.ModelLike, src::MOI.ModelLike, end """ - allocate_load(dest::MOI.ModelLike, src::MOI.ModelLike) + allocate_load(dest::MOI.ModelLike, src::MOI.ModelLike, + filter_constraints::Function=((x) -> true) + ) Implements `MOI.copy_to(dest, src)` using the Allocate-Load API. The function [`supports_allocate_load`](@ref) can be used to check whether `dest` supports the Allocate-Load API. """ function allocate_load(dest::MOI.ModelLike, src::MOI.ModelLike, copy_names::Bool, - only_conflict::Bool=false) + filter_constraints::Function=((x) -> true)) MOI.empty!(dest) vis_src = MOI.get(src, MOI.ListOfVariableIndices()) @@ -746,7 +744,8 @@ function allocate_load(dest::MOI.ModelLike, src::MOI.ModelLike, copy_names::Bool pass_constraints(dest, src, copy_names, idxmap, single_variable_types, single_variable_not_allocated, vector_of_variables_types, vector_of_variables_not_allocated, - allocate_constraints, allocate, only_conflict=only_conflict) + allocate_constraints, allocate, + filter_constraints=filter_constraints) # Load variables load_variables(dest, length(vis_src)) @@ -767,7 +766,7 @@ function allocate_load(dest::MOI.ModelLike, src::MOI.ModelLike, copy_names::Bool pass_constraints(dest, src, copy_names, idxmap, single_variable_types, single_variable_not_allocated, vector_of_variables_types, vector_of_variables_not_allocated, - load_constraints, load, only_conflict=only_conflict) + load_constraints, load, filter_constraints=filter_constraints) return idxmap end From 82baad75587f843281020425b350c575926fe2c8 Mon Sep 17 00:00:00 2001 From: Thibaut Cuvelier Date: Wed, 12 Aug 2020 03:33:33 +0200 Subject: [PATCH 03/10] Add test. --- src/Utilities/copy.jl | 9 +++++++-- test/Utilities/copy.jl | 20 ++++++++++++++++++++ 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/src/Utilities/copy.jl b/src/Utilities/copy.jl index baba34c4c3..5928fcd62f 100644 --- a/src/Utilities/copy.jl +++ b/src/Utilities/copy.jl @@ -278,24 +278,28 @@ function pass_constraints( dest::MOI.ModelLike, src::MOI.ModelLike, copy_names::Bool, idxmap::IndexMap, single_variable_types, single_variable_indices, vector_of_variables_types, vector_of_variables_indices, - pass_cons=copy_constraints, pass_attr=MOI.set, + pass_cons=copy_constraints, pass_attr=MOI.set; filter_constraints::Function=((x) -> true) ) for (S, cis_src) in zip(single_variable_types, single_variable_indices) + filter!(filter_constraints, cis_src) # do the rest in `pass_cons` which is type stable pass_cons(dest, src, idxmap, cis_src) cis_src = MOI.get(src, MOI.ListOfConstraintIndices{ MOI.SingleVariable, S}()) + filter!(filter_constraints, cis_src) pass_attributes(dest, src, copy_names, idxmap, cis_src, pass_attr) end for (S, cis_src) in zip(vector_of_variables_types, vector_of_variables_indices) + filter!(filter_constraints, cis_src) # do the rest in `pass_cons` which is type stable pass_cons(dest, src, idxmap, cis_src) cis_src = MOI.get(src, MOI.ListOfConstraintIndices{ MOI.VectorOfVariables, S}()) + filter!(filter_constraints, cis_src) pass_attributes(dest, src, copy_names, idxmap, cis_src, pass_attr) end @@ -305,8 +309,9 @@ function pass_constraints( ] for (F, S) in nonvariable_constraint_types cis_src = MOI.get(src, MOI.ListOfConstraintIndices{F, S}()) + filter!(filter_constraints, cis_src) # do the rest in `pass_cons` which is type stable - pass_cons(dest, src, idxmap, cis_src, filter_constraints=filter_constraints) + pass_cons(dest, src, idxmap, cis_src)#, filter_constraints=filter_constraints) pass_attributes(dest, src, copy_names, idxmap, cis_src, pass_attr) end end diff --git a/test/Utilities/copy.jl b/test/Utilities/copy.jl index 549668d9a8..f5988713cc 100644 --- a/test/Utilities/copy.jl +++ b/test/Utilities/copy.jl @@ -256,3 +256,23 @@ MOI.supports_add_constrained_variable(::ReverseOrderConstrainedVariablesModel, : @test typeof(c1) == typeof(dest.constraintIndices[1]) @test typeof(c2) == typeof(dest.constraintIndices[2]) end + +@testset "Filtering copy" begin + # Create a basic model. + src = MOIU.Model{Float64}() + x = MOI.add_variable(src) + c1 = MOI.add_constraint(src, x, MOI.LessThan{Float64}(1.0)) + c2 = MOI.add_constraint(src, x, MOI.GreaterThan{Float64}(0.0)) + + # Filtering function: the default case where this function always returns + # true is already well-tested by the above cases. + # Only keep the constraint c1. + f = (cidx) -> cidx == c1 + + # Perform the copy. + dst = OrderConstrainedVariablesModel() + index_map = MOI.copy_to(dst, src, filter_constraints=f) + + @test typeof(c1) == typeof(dst.constraintIndices[1]) + @test length(dst.constraintIndices) == 1 +end From 8192220c7d2d20782de025810f9dfdf5f0dca53b Mon Sep 17 00:00:00 2001 From: Thibaut Cuvelier Date: Wed, 12 Aug 2020 03:40:21 +0200 Subject: [PATCH 04/10] Filter less often. Add a bit more doc. --- src/Utilities/copy.jl | 58 +++++++++++++++++++++++++++++++------------ 1 file changed, 42 insertions(+), 16 deletions(-) diff --git a/src/Utilities/copy.jl b/src/Utilities/copy.jl index 5928fcd62f..9517891ba6 100644 --- a/src/Utilities/copy.jl +++ b/src/Utilities/copy.jl @@ -3,16 +3,20 @@ """ automatic_copy_to(dest::MOI.ModelLike, src::MOI.ModelLike; copy_names::Bool=true, - filter_constraints::Function=(() -> true)) + filter_constraints::Union{Nothing, Function}=nothing) Use [`Utilities.supports_default_copy_to`](@ref) and [`Utilities.supports_allocate_load`](@ref) to automatically choose between [`Utilities.default_copy_to`](@ref) or [`Utilities.allocate_load`](@ref) to apply the copy operation. + +If the `filter_constraints` arguments is given, only the constraints for which +this function returns `true` will be copied. This function is given a +constraint index as argument. """ function automatic_copy_to(dest::MOI.ModelLike, src::MOI.ModelLike; copy_names::Bool=true, - filter_constraints::Function=((x) -> true)) + filter_constraints::Union{Nothing, Function}=nothing) if supports_default_copy_to(dest, copy_names) default_copy_to(dest, src, copy_names, filter_constraints) elseif supports_allocate_load(dest, copy_names) @@ -258,12 +262,14 @@ constraint index as argument. function copy_constraints(dest::MOI.ModelLike, src::MOI.ModelLike, idxmap::IndexMap, cis_src::Vector{<:MOI.ConstraintIndex}, - filter_constraints::Function=((x) -> true)) + filter_constraints::Union{Nothing, Function}=nothing) # Retrieve the constraints to copy. f_src = MOI.get(src, MOI.ConstraintFunction(), cis_src) - # Filter this set of constraints. - filter!(filter_constraints, cis_src) + # Filter this set of constraints if need before. + if filter_constraints !== nothing + filter!(filter_constraints, cis_src) + end # Copy the constraints into the new model and build the map. f_dest = map_indices.(Ref(idxmap), f_src) @@ -279,27 +285,37 @@ function pass_constraints( single_variable_types, single_variable_indices, vector_of_variables_types, vector_of_variables_indices, pass_cons=copy_constraints, pass_attr=MOI.set; - filter_constraints::Function=((x) -> true) + filter_constraints::Union{Nothing, Function}=nothing ) for (S, cis_src) in zip(single_variable_types, single_variable_indices) - filter!(filter_constraints, cis_src) + if filter_constraints !== nothing + filter!(filter_constraints, cis_src) + end # do the rest in `pass_cons` which is type stable pass_cons(dest, src, idxmap, cis_src) + cis_src = MOI.get(src, MOI.ListOfConstraintIndices{ MOI.SingleVariable, S}()) - filter!(filter_constraints, cis_src) + if filter_constraints !== nothing + filter!(filter_constraints, cis_src) + end pass_attributes(dest, src, copy_names, idxmap, cis_src, pass_attr) end for (S, cis_src) in zip(vector_of_variables_types, vector_of_variables_indices) - filter!(filter_constraints, cis_src) + if filter_constraints !== nothing + filter!(filter_constraints, cis_src) + end # do the rest in `pass_cons` which is type stable pass_cons(dest, src, idxmap, cis_src) + cis_src = MOI.get(src, MOI.ListOfConstraintIndices{ MOI.VectorOfVariables, S}()) - filter!(filter_constraints, cis_src) + if filter_constraints !== nothing + filter!(filter_constraints, cis_src) + end pass_attributes(dest, src, copy_names, idxmap, cis_src, pass_attr) end @@ -309,9 +325,11 @@ function pass_constraints( ] for (F, S) in nonvariable_constraint_types cis_src = MOI.get(src, MOI.ListOfConstraintIndices{F, S}()) - filter!(filter_constraints, cis_src) + if filter_constraints !== nothing + filter!(filter_constraints, cis_src) + end # do the rest in `pass_cons` which is type stable - pass_cons(dest, src, idxmap, cis_src)#, filter_constraints=filter_constraints) + pass_cons(dest, src, idxmap, cis_src) pass_attributes(dest, src, copy_names, idxmap, cis_src, pass_attr) end end @@ -338,16 +356,20 @@ end """ default_copy_to(dest::MOI.ModelLike, src::MOI.ModelLike, copy_names::Bool, - filter_constraints::Function=((x) -> true)) + filter_constraints::Union{Nothing, Function}=nothing) Implements `MOI.copy_to(dest, src)` by adding the variables and then the constraints and attributes incrementally. The function [`supports_default_copy_to`](@ref) can be used to check whether `dest` supports the copying a model incrementally. + +If the `filter_constraints` arguments is given, only the constraints for which +this function returns `true` will be copied. This function is given a +constraint index as argument. """ function default_copy_to(dest::MOI.ModelLike, src::MOI.ModelLike, copy_names::Bool, - filter_constraints::Function=((x) -> true)) + filter_constraints::Union{Nothing, Function}=nothing) MOI.empty!(dest) vis_src = MOI.get(src, MOI.ListOfVariableIndices()) @@ -703,15 +725,19 @@ end """ allocate_load(dest::MOI.ModelLike, src::MOI.ModelLike, - filter_constraints::Function=((x) -> true) + filter_constraints::Union{Nothing, Function}=nothing ) Implements `MOI.copy_to(dest, src)` using the Allocate-Load API. The function [`supports_allocate_load`](@ref) can be used to check whether `dest` supports the Allocate-Load API. + +If the `filter_constraints` arguments is given, only the constraints for which +this function returns `true` will be copied. This function is given a +constraint index as argument. """ function allocate_load(dest::MOI.ModelLike, src::MOI.ModelLike, copy_names::Bool, - filter_constraints::Function=((x) -> true)) + filter_constraints::Union{Nothing, Function}=nothing) MOI.empty!(dest) vis_src = MOI.get(src, MOI.ListOfVariableIndices()) From 1342f9cd5c4bd43a1d6fd7d3a7589c00d6ab45e3 Mon Sep 17 00:00:00 2001 From: Thibaut Cuvelier Date: Wed, 12 Aug 2020 04:00:03 +0200 Subject: [PATCH 05/10] Add benchmark for copy. --- src/Benchmarks/Benchmarks.jl | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/src/Benchmarks/Benchmarks.jl b/src/Benchmarks/Benchmarks.jl index 9efa017405..a31ae9f86c 100644 --- a/src/Benchmarks/Benchmarks.jl +++ b/src/Benchmarks/Benchmarks.jl @@ -232,4 +232,25 @@ end return model end +@add_benchmark function copy_model(new_model) + model = new_model() + index = MOI.add_variables(model, 1_000) + cons = Vector{ + MOI.ConstraintIndex{MOI.ScalarAffineFunction{Float64}, MOI.LessThan{Float64}} + }(undef, 1_000) + for (i, x) in enumerate(index) + cons[i] = MOI.add_constraint( + model, + MOI.ScalarAffineFunction([MOI.ScalarAffineTerm(1.0, x)], 0.0), + MOI.LessThan(1.0 * i) + ) + end + + model2 = new_model() + MOI.copy_to(model2, model) + # MOI.copy_to(model2, model, filter_constraints=(x) -> x in cons[1:500]) + + return model2 +end + end From aa7c31198d0cdab0f073d76c555d1a9f86ca6c6a Mon Sep 17 00:00:00 2001 From: Thibaut Cuvelier Date: Fri, 21 Aug 2020 22:36:35 +0200 Subject: [PATCH 06/10] Fix prototype. --- src/Utilities/copy.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Utilities/copy.jl b/src/Utilities/copy.jl index 9517891ba6..d9cbb419e1 100644 --- a/src/Utilities/copy.jl +++ b/src/Utilities/copy.jl @@ -249,7 +249,7 @@ end copy_constraints(dest::MOI.ModelLike, src::MOI.ModelLike, idxmap::IndexMap, cis_src::Vector{<:MOI.ConstraintIndex}, - filter_constraints::Function=(() -> true)) + filter_constraints::Union{Nothing, Function}=nothing) Copy the constraints `cis_src` from the model `src` to the model `dest` and fill `idxmap` accordingly. Note that the attributes are not copied; call From d8c416002a13f13265d8c47d1bbad7e0e28dd4b4 Mon Sep 17 00:00:00 2001 From: Thibaut Cuvelier Date: Fri, 21 Aug 2020 22:36:57 +0200 Subject: [PATCH 07/10] Typo. --- src/Utilities/copy.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Utilities/copy.jl b/src/Utilities/copy.jl index d9cbb419e1..e9c5b93a88 100644 --- a/src/Utilities/copy.jl +++ b/src/Utilities/copy.jl @@ -266,7 +266,7 @@ function copy_constraints(dest::MOI.ModelLike, src::MOI.ModelLike, # Retrieve the constraints to copy. f_src = MOI.get(src, MOI.ConstraintFunction(), cis_src) - # Filter this set of constraints if need before. + # Filter this set of constraints if needed before. if filter_constraints !== nothing filter!(filter_constraints, cis_src) end From 27d3de78707fd5c424643cb563599935beabb443 Mon Sep 17 00:00:00 2001 From: Thibaut Cuvelier Date: Fri, 21 Aug 2020 22:41:06 +0200 Subject: [PATCH 08/10] Comment about double copies. --- src/Utilities/copy.jl | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/Utilities/copy.jl b/src/Utilities/copy.jl index e9c5b93a88..7172bbec32 100644 --- a/src/Utilities/copy.jl +++ b/src/Utilities/copy.jl @@ -287,6 +287,9 @@ function pass_constraints( pass_cons=copy_constraints, pass_attr=MOI.set; filter_constraints::Union{Nothing, Function}=nothing ) + # copy_constraints can also take a filter_constraints argument; however, filtering + # is performed within this function (because it also calls MOI.set on the constraints). + # Don't pass this argument to copy_constraints/pass_cons to avoid a double filtering. for (S, cis_src) in zip(single_variable_types, single_variable_indices) if filter_constraints !== nothing From 2fa662cd1b705d08b83b22b634b92d136f733618 Mon Sep 17 00:00:00 2001 From: Thibaut Cuvelier Date: Fri, 21 Aug 2020 23:52:42 +0200 Subject: [PATCH 09/10] Add test: unsupported constraints are not copied when explicitly filtered for. --- test/Utilities/copy.jl | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/test/Utilities/copy.jl b/test/Utilities/copy.jl index f5988713cc..caa3ded676 100644 --- a/test/Utilities/copy.jl +++ b/test/Utilities/copy.jl @@ -276,3 +276,40 @@ end @test typeof(c1) == typeof(dst.constraintIndices[1]) @test length(dst.constraintIndices) == 1 end + +mutable struct BoundModel <: MOI.ModelLike + # Type of model that only supports ≤ bound constraints. In particular, it does not support integrality constraints. + inner::MOIU.Model{Float64} + BoundModel() = new(MOIU.Model{Float64}()) +end + +MOI.add_variables(model::BoundModel, n) = MOI.add_variables(model.inner, n) +MOI.add_variable(model::BoundModel) = MOI.add_variable(model.inner) +MOI.add_constraint(model::BoundModel, f::F, s::S) where {F<:MathOptInterface.AbstractFunction, S<:MathOptInterface.AbstractSet} = + MOI.add_constraint(model.inner, f, s) +MOI.supports_constraint(::BoundModel, ::Type{MOI.SingleVariable}, ::Type{MOI.LessThan{Float64}}) = true +MOI.supports_constraint(::BoundModel, ::Type{MOI.SingleVariable}, ::Type{MOI.Integer}) = false + +MOIU.supports_default_copy_to(::BoundModel, ::Bool) = true +MOI.copy_to(dest::BoundModel, src::MOI.ModelLike; kws...) = MOIU.automatic_copy_to(dest, src; kws...) +MOI.empty!(model::BoundModel) = MOI.empty!(model.inner) + +MOI.supports(::BoundModel, ::Type{MOI.NumberOfConstraints}) = true +MOI.get(model::BoundModel, attr::MOI.NumberOfConstraints) = MOI.get(model.inner, attr) + +@testset "Filtering copy" begin + # Create a basic model. + src = MOIU.Model{Float64}() + x = MOI.add_variable(src) + c1 = MOI.add_constraint(src, x, MOI.LessThan{Float64}(10.0)) + c2 = MOI.add_constraint(src, x, MOI.Integer()) + + # Filtering function: get rid of integrality constraint. + f = (cidx) -> MOI.get(src, MOI.ConstraintSet(), cidx) != MOI.Integer() + + # Perform the copy. This should not throw an error. + dst = BoundModel() + MOI.copy_to(dst, src, filter_constraints=f) + @test MOI.get(dst, MOI.NumberOfConstraints{MOI.SingleVariable, MOI.LessThan{Float64}}()) == 1 + @test MOI.get(dst, MOI.NumberOfConstraints{MOI.SingleVariable, MOI.Integer}()) == 0 +end From b002a4a20fa2e724a9ef7c5e4291f702a27ff77d Mon Sep 17 00:00:00 2001 From: Thibaut Cuvelier Date: Fri, 28 Aug 2020 17:00:22 +0200 Subject: [PATCH 10/10] Add a test specifically for BoundModel. --- test/Utilities/copy.jl | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/test/Utilities/copy.jl b/test/Utilities/copy.jl index caa3ded676..ea79c4c2ca 100644 --- a/test/Utilities/copy.jl +++ b/test/Utilities/copy.jl @@ -257,7 +257,7 @@ MOI.supports_add_constrained_variable(::ReverseOrderConstrainedVariablesModel, : @test typeof(c2) == typeof(dest.constraintIndices[2]) end -@testset "Filtering copy" begin +@testset "Filtering copy: check based on index" begin # Create a basic model. src = MOIU.Model{Float64}() x = MOI.add_variable(src) @@ -283,12 +283,9 @@ mutable struct BoundModel <: MOI.ModelLike BoundModel() = new(MOIU.Model{Float64}()) end -MOI.add_variables(model::BoundModel, n) = MOI.add_variables(model.inner, n) MOI.add_variable(model::BoundModel) = MOI.add_variable(model.inner) -MOI.add_constraint(model::BoundModel, f::F, s::S) where {F<:MathOptInterface.AbstractFunction, S<:MathOptInterface.AbstractSet} = - MOI.add_constraint(model.inner, f, s) -MOI.supports_constraint(::BoundModel, ::Type{MOI.SingleVariable}, ::Type{MOI.LessThan{Float64}}) = true -MOI.supports_constraint(::BoundModel, ::Type{MOI.SingleVariable}, ::Type{MOI.Integer}) = false +MOI.add_constraint(model::BoundModel, f::MOI.AbstractFunction, s::MOI.LessThan{Float64}) = MOI.add_constraint(model.inner, f, s) +MOI.supports_constraint(::BoundModel, ::Type{MOI.SingleVariable}, ::MOI.LessThan{Float64}) = true MOIU.supports_default_copy_to(::BoundModel, ::Bool) = true MOI.copy_to(dest::BoundModel, src::MOI.ModelLike; kws...) = MOIU.automatic_copy_to(dest, src; kws...) @@ -297,7 +294,7 @@ MOI.empty!(model::BoundModel) = MOI.empty!(model.inner) MOI.supports(::BoundModel, ::Type{MOI.NumberOfConstraints}) = true MOI.get(model::BoundModel, attr::MOI.NumberOfConstraints) = MOI.get(model.inner, attr) -@testset "Filtering copy" begin +@testset "Filtering copy: check based on constraint type" begin # Create a basic model. src = MOIU.Model{Float64}() x = MOI.add_variable(src) @@ -307,7 +304,12 @@ MOI.get(model::BoundModel, attr::MOI.NumberOfConstraints) = MOI.get(model.inner, # Filtering function: get rid of integrality constraint. f = (cidx) -> MOI.get(src, MOI.ConstraintSet(), cidx) != MOI.Integer() - # Perform the copy. This should not throw an error. + # Perform the unfiltered copy. This should throw an error (i.e. the implementation of BoundModel + # should be correct). + dst = BoundModel() + @test_throws MOI.UnsupportedConstraint{MOI.SingleVariable, MOI.Integer} MOI.copy_to(dst, src) + + # Perform the filtered copy. This should not throw an error. dst = BoundModel() MOI.copy_to(dst, src, filter_constraints=f) @test MOI.get(dst, MOI.NumberOfConstraints{MOI.SingleVariable, MOI.LessThan{Float64}}()) == 1