From 7ee7b2febd25f49c6aae105ac05772c8de030cd0 Mon Sep 17 00:00:00 2001 From: odow Date: Mon, 9 Aug 2021 11:43:51 +1200 Subject: [PATCH 1/5] [breaking] remove automatic_copy_to --- docs/src/submodules/Utilities/reference.md | 1 - docs/src/tutorials/implementing.md | 2 +- src/Bridges/bridge_optimizer.jl | 2 +- src/Utilities/copy.jl | 41 +++++----------------- src/Utilities/mockoptimizer.jl | 6 ++-- src/Utilities/model.jl | 2 +- src/Utilities/universalfallback.jl | 2 +- test/Utilities/cachingoptimizer.jl | 2 +- test/Utilities/copy.jl | 12 +++---- 9 files changed, 22 insertions(+), 48 deletions(-) diff --git a/docs/src/submodules/Utilities/reference.md b/docs/src/submodules/Utilities/reference.md index fcd88befeb..25054cc243 100644 --- a/docs/src/submodules/Utilities/reference.md +++ b/docs/src/submodules/Utilities/reference.md @@ -64,7 +64,6 @@ Utilities.latex_formulation ## Copy utilities ```@docs -Utilities.automatic_copy_to Utilities.default_copy_to Utilities.IndexMap Utilities.identity_index_map diff --git a/docs/src/tutorials/implementing.md b/docs/src/tutorials/implementing.md index 432a12be0e..2560d4dddc 100644 --- a/docs/src/tutorials/implementing.md +++ b/docs/src/tutorials/implementing.md @@ -470,7 +470,7 @@ MOI.supports_incremental_interface(::Optimizer, copy_names::Bool) = true MOI.supports_incremental_interface(::Optimizer, copy_names::Bool) = !copy_names function MOI.copy_to(dest::Optimizer, src::MOI.ModelLike; kwargs...) - return MOI.Utilities.automatic_copy_to(dest, src; kwargs...) + return MOI.Utilities.default_copy_to(dest, src; kwargs...) end ``` See [`supports_incremental_interface`](@ref) for more details on whether to diff --git a/src/Bridges/bridge_optimizer.jl b/src/Bridges/bridge_optimizer.jl index 7da990a94b..da2b09574f 100644 --- a/src/Bridges/bridge_optimizer.jl +++ b/src/Bridges/bridge_optimizer.jl @@ -427,7 +427,7 @@ function MOIU.pass_nonvariable_constraints( end function MOI.copy_to(b::AbstractBridgeOptimizer, src::MOI.ModelLike; kwargs...) - return MOIU.automatic_copy_to(b, src; kwargs...) + return MOIU.default_copy_to(b, src; kwargs...) end function MOI.supports_incremental_interface( diff --git a/src/Utilities/copy.jl b/src/Utilities/copy.jl index 09c0262441..7178bb5bda 100644 --- a/src/Utilities/copy.jl +++ b/src/Utilities/copy.jl @@ -1,39 +1,7 @@ # This file contains default implementations for the `MOI.copy_to` function that # can be used by a model. -""" - automatic_copy_to( - dest::MOI.ModelLike, - src::MOI.ModelLike; - copy_names::Bool=true, - filter_constraints::Union{Nothing,Function} = nothing, - ) - -A default fallback for [`MOI.copy_to`](@ref). - -To use this method, define -[`MathOptInterface.supports_incremental_interface`](@ref). - -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::Union{Nothing,Function} = nothing, -) - if !MOI.supports_incremental_interface(dest, copy_names) - error( - "Model $(typeof(dest)) does not support copy", - copy_names ? " with names" : "", - ".", - ) - end - return default_copy_to(dest, src, copy_names, filter_constraints) -end - +@deprecate automatic_copy_to default_copy_to @deprecate supports_default_copy_to MOI.supports_incremental_interface include("copy/index_map.jl") @@ -598,6 +566,13 @@ function default_copy_to( copy_names::Bool, filter_constraints::Union{Nothing,Function} = nothing, ) + if !MOI.supports_incremental_interface(dest, copy_names) + error( + "Model $(typeof(dest)) does not support copy", + copy_names ? " with names" : "", + ".", + ) + end MOI.empty!(dest) vis_src = MOI.get(src, MOI.ListOfVariableIndices()) diff --git a/src/Utilities/mockoptimizer.jl b/src/Utilities/mockoptimizer.jl index a661af2507..1386fb1f04 100644 --- a/src/Utilities/mockoptimizer.jl +++ b/src/Utilities/mockoptimizer.jl @@ -11,8 +11,8 @@ struct MockConstraintAttribute <: MOI.AbstractConstraintAttribute end """ MockOptimizer -`MockOptimizer` is a fake optimizer especially useful for testing. Its main -feature is that it can store the values that should be returned for each +`MockOptimizer` is a fake optimizer especially useful for testing. Its main +feature is that it can store the values that should be returned for each attribute. """ mutable struct MockOptimizer{MT<:MOI.ModelLike} <: MOI.AbstractOptimizer @@ -921,7 +921,7 @@ function MOI.supports_add_constrained_variables( end function MOI.copy_to(mock::MockOptimizer, src::MOI.ModelLike; kws...) - return automatic_copy_to(mock, src; kws...) + return default_copy_to(mock, src; kws...) end function MOI.supports_incremental_interface( diff --git a/src/Utilities/model.jl b/src/Utilities/model.jl index 56a136cc73..62e54eea12 100644 --- a/src/Utilities/model.jl +++ b/src/Utilities/model.jl @@ -576,7 +576,7 @@ function pass_nonvariable_constraints( end function MOI.copy_to(dest::AbstractModel, src::MOI.ModelLike; kws...) - return automatic_copy_to(dest, src; kws...) + return default_copy_to(dest, src; kws...) end MOI.supports_incremental_interface(::AbstractModel, ::Bool) = true function final_touch(model::AbstractModel, index_map) diff --git a/src/Utilities/universalfallback.jl b/src/Utilities/universalfallback.jl index bf55f26307..5199f50a3b 100644 --- a/src/Utilities/universalfallback.jl +++ b/src/Utilities/universalfallback.jl @@ -129,7 +129,7 @@ function pass_nonvariable_constraints( end function MOI.copy_to(uf::UniversalFallback, src::MOI.ModelLike; kws...) - return MOIU.automatic_copy_to(uf, src; kws...) + return MOIU.default_copy_to(uf, src; kws...) end function MOI.supports_incremental_interface( diff --git a/test/Utilities/cachingoptimizer.jl b/test/Utilities/cachingoptimizer.jl index 4b70ac4905..1c49c78dde 100644 --- a/test/Utilities/cachingoptimizer.jl +++ b/test/Utilities/cachingoptimizer.jl @@ -85,7 +85,7 @@ end MOI.supports_incremental_interface(::NoFreeVariables, names::Bool) = !names function MOI.copy_to(dest::NoFreeVariables, src::MOI.ModelLike; kwargs...) - return MOI.Utilities.automatic_copy_to(dest, src; kwargs...) + return MOI.Utilities.default_copy_to(dest, src; kwargs...) end ### diff --git a/test/Utilities/copy.jl b/test/Utilities/copy.jl index fd4bd8145f..c54128b713 100644 --- a/test/Utilities/copy.jl +++ b/test/Utilities/copy.jl @@ -124,12 +124,12 @@ end function test_AUTOMATIC() src = DummyModel() dest = DummyModel() - @test_throws ErrorException MOIU.automatic_copy_to(dest, src) + @test_throws ErrorException MOIU.default_copy_to(dest, src) try - @test_throws ErrorException MOIU.automatic_copy_to(dest, src) + @test_throws ErrorException MOIU.default_copy_to(dest, src) catch err @test sprint(showerror, err) == - "Model DummyModel does not" * " support copy with names." + "Model DummyModel does not support copy with names." end end @@ -188,7 +188,7 @@ function MOI.copy_to( src::MOI.ModelLike; kws..., ) - return MOIU.automatic_copy_to(dest, src; kws...) + return MOIU.default_copy_to(dest, src; kws...) end function MOI.add_variables(model::ConstrainedVariablesModel, n) @@ -283,7 +283,7 @@ function MOI.copy_to( src::MOI.ModelLike; kwargs..., ) - return MOIU.automatic_copy_to(dest, src; kwargs...) + return MOIU.default_copy_to(dest, src; kwargs...) end function MOI.supports_incremental_interface( @@ -670,7 +670,7 @@ end MOI.supports_incremental_interface(::BoundModel, ::Bool) = true function MOI.copy_to(dest::BoundModel, src::MOI.ModelLike; kws...) - return MOIU.automatic_copy_to(dest, src; kws...) + return MOIU.default_copy_to(dest, src; kws...) end MOI.empty!(model::BoundModel) = MOI.empty!(model.inner) From 7f9d68ae33c393d3b6c38086dc926fd411a993fb Mon Sep 17 00:00:00 2001 From: odow Date: Mon, 9 Aug 2021 13:29:53 +1200 Subject: [PATCH 2/5] Fix deprecation --- src/Bridges/bridge_optimizer.jl | 8 +++++-- src/Utilities/copy.jl | 35 ++++++++++++++++++++---------- src/Utilities/mockoptimizer.jl | 4 ++-- src/Utilities/model.jl | 5 +++-- src/Utilities/universalfallback.jl | 4 ++-- test/Utilities/copy.jl | 14 ++++++------ test/deprecate.jl | 7 ++++++ test/dummy.jl | 4 +++- 8 files changed, 53 insertions(+), 28 deletions(-) diff --git a/src/Bridges/bridge_optimizer.jl b/src/Bridges/bridge_optimizer.jl index da2b09574f..7e814d0a38 100644 --- a/src/Bridges/bridge_optimizer.jl +++ b/src/Bridges/bridge_optimizer.jl @@ -426,8 +426,12 @@ function MOIU.pass_nonvariable_constraints( return end -function MOI.copy_to(b::AbstractBridgeOptimizer, src::MOI.ModelLike; kwargs...) - return MOIU.default_copy_to(b, src; kwargs...) +function MOI.copy_to( + dest::AbstractBridgeOptimizer, + src::MOI.ModelLike; + kwargs..., +) + return MOIU.default_copy_to(dest, src; kwargs...) end function MOI.supports_incremental_interface( diff --git a/src/Utilities/copy.jl b/src/Utilities/copy.jl index 7178bb5bda..3018665df6 100644 --- a/src/Utilities/copy.jl +++ b/src/Utilities/copy.jl @@ -454,14 +454,6 @@ function copy_free_variables( return end -function default_copy_to(dest::MOI.ModelLike, src::MOI.ModelLike) - Base.depwarn( - "default_copy_to(dest, src) is deprecated, use default_copy_to(dest, src, true) instead or default_copy_to(dest, src, false) if you do not want to copy names.", - :default_copy_to, - ) - return default_copy_to(dest, src, true) -end - function sorted_variable_sets_by_cost(dest::MOI.ModelLike, src::MOI.ModelLike) constraint_types = MOI.get(src, MOI.ListOfConstraintTypesPresent()) single_or_vector_variables_types = [ @@ -543,11 +535,30 @@ only once all the model information is gathered. """ function final_touch(::MOI.ModelLike, idxmap) end +function default_copy_to( + dest::MOI.ModelLike, + src::MOI.ModelLike, + copy_names::Bool, + filter_constraints::Union{Nothing,Function} = nothing, +) + @warn( + "The `copy_names` and `filter_constraints` arguments to " * + "`default_copy_to` are now keyword arguments.", + maxlog = 1, + ) + return default_copy_to( + dest, + src; + copy_names = copy_names, + filter_constraints = filter_constraints, + ) +end + """ default_copy_to( dest::MOI.ModelLike, - src::MOI.ModelLike, - copy_names::Bool, + src::MOI.ModelLike; + copy_names::Bool = true, filter_constraints::Union{Nothing,Function} = nothing, ) @@ -562,8 +573,8 @@ constraint index as argument. """ function default_copy_to( dest::MOI.ModelLike, - src::MOI.ModelLike, - copy_names::Bool, + src::MOI.ModelLike; + copy_names::Bool = false, filter_constraints::Union{Nothing,Function} = nothing, ) if !MOI.supports_incremental_interface(dest, copy_names) diff --git a/src/Utilities/mockoptimizer.jl b/src/Utilities/mockoptimizer.jl index 1386fb1f04..b0fb34a730 100644 --- a/src/Utilities/mockoptimizer.jl +++ b/src/Utilities/mockoptimizer.jl @@ -920,8 +920,8 @@ function MOI.supports_add_constrained_variables( return MOI.supports_add_constrained_variables(mock.inner_model, MOI.Reals) end -function MOI.copy_to(mock::MockOptimizer, src::MOI.ModelLike; kws...) - return default_copy_to(mock, src; kws...) +function MOI.copy_to(dest::MockOptimizer, src::MOI.ModelLike; kwargs...) + return default_copy_to(dest, src; kwargs...) end function MOI.supports_incremental_interface( diff --git a/src/Utilities/model.jl b/src/Utilities/model.jl index 62e54eea12..a9ef5d47ec 100644 --- a/src/Utilities/model.jl +++ b/src/Utilities/model.jl @@ -575,9 +575,10 @@ function pass_nonvariable_constraints( ) end -function MOI.copy_to(dest::AbstractModel, src::MOI.ModelLike; kws...) - return default_copy_to(dest, src; kws...) +function MOI.copy_to(dest::AbstractModel, src::MOI.ModelLike; kwargs...) + return default_copy_to(dest, src; kwargs...) end + MOI.supports_incremental_interface(::AbstractModel, ::Bool) = true function final_touch(model::AbstractModel, index_map) return final_touch(model.constraints, index_map) diff --git a/src/Utilities/universalfallback.jl b/src/Utilities/universalfallback.jl index 5199f50a3b..073b4f9d7f 100644 --- a/src/Utilities/universalfallback.jl +++ b/src/Utilities/universalfallback.jl @@ -128,8 +128,8 @@ function pass_nonvariable_constraints( ) end -function MOI.copy_to(uf::UniversalFallback, src::MOI.ModelLike; kws...) - return MOIU.default_copy_to(uf, src; kws...) +function MOI.copy_to(dest::UniversalFallback, src::MOI.ModelLike; kwargs...) + return MOIU.default_copy_to(dest, src; kwargs...) end function MOI.supports_incremental_interface( diff --git a/test/Utilities/copy.jl b/test/Utilities/copy.jl index c54128b713..e63d1bd71c 100644 --- a/test/Utilities/copy.jl +++ b/test/Utilities/copy.jl @@ -30,9 +30,9 @@ function MOI.empty!(::AbstractDummyModel) end function MOI.copy_to( dest::AbstractDummyModel, src::MOI.ModelLike; - copy_names = true, + copy_names::Bool = true, ) - return MOIU.default_copy_to(dest, src, copy_names) + return MOIU.default_copy_to(dest, src; copy_names = copy_names) end MOI.supports(::AbstractDummyModel, ::MOI.ObjectiveSense) = true @@ -151,7 +151,7 @@ function test_issue_849() ) MOI.set(model, MOI.NLPBlock(), nlp_data) copy = MOIU.UniversalFallback(MOIU.Model{Float64}()) - index_map = MOIU.default_copy_to(copy, model, true) + index_map = MOIU.default_copy_to(copy, model; copy_names = true) for vi in [a, b, c, x, y[1]] @test index_map[vi] == vi end @@ -186,9 +186,9 @@ end function MOI.copy_to( dest::ConstrainedVariablesModel, src::MOI.ModelLike; - kws..., + kwargs..., ) - return MOIU.default_copy_to(dest, src; kws...) + return MOIU.default_copy_to(dest, src; kwargs...) end function MOI.add_variables(model::ConstrainedVariablesModel, n) @@ -669,8 +669,8 @@ end MOI.supports_incremental_interface(::BoundModel, ::Bool) = true -function MOI.copy_to(dest::BoundModel, src::MOI.ModelLike; kws...) - return MOIU.default_copy_to(dest, src; kws...) +function MOI.copy_to(dest::BoundModel, src::MOI.ModelLike; kwargs...) + return MOIU.default_copy_to(dest, src; kwargs...) end MOI.empty!(model::BoundModel) = MOI.empty!(model.inner) diff --git a/test/deprecate.jl b/test/deprecate.jl index 803660c1b1..47d2b9877f 100644 --- a/test/deprecate.jl +++ b/test/deprecate.jl @@ -70,6 +70,13 @@ function test_RawOptimizerAttribute() @test_logs (:warn,) MOI.RawParameter(:a) == MOI.RawOptimizerAttribute("a") end +function test_default_copy_to() + dest = MOI.Utilities.Model{Float64}() + src = MOI.Utilities.Model{Float64}() + @test_logs (:warn,) MOI.Utilities.default_copy_to(dest, src, true) + return +end + function runtests() for name in names(@__MODULE__; all = true) if startswith("$name", "test_") diff --git a/test/dummy.jl b/test/dummy.jl index 7a5bc8ff04..0853387168 100644 --- a/test/dummy.jl +++ b/test/dummy.jl @@ -5,13 +5,15 @@ const MOIU = MOI.Utilities abstract type AbstractDummyModel <: MOI.ModelLike end function MOI.empty!(::AbstractDummyModel) end + function MOI.copy_to( dest::AbstractDummyModel, src::MOI.ModelLike; - copy_names = true, + copy_names::Bool = true, ) return MOIU.default_copy_to(dest, src, copy_names) end + MOI.supports(::AbstractDummyModel, ::MOI.ObjectiveSense) = true function MOI.supports( ::AbstractDummyModel, From deb98cc77f7d04212bc84490d1d16463820e6f8b Mon Sep 17 00:00:00 2001 From: odow Date: Mon, 9 Aug 2021 14:17:38 +1200 Subject: [PATCH 3/5] Fix default copy_names --- 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 3018665df6..a85b7c8e69 100644 --- a/src/Utilities/copy.jl +++ b/src/Utilities/copy.jl @@ -574,7 +574,7 @@ constraint index as argument. function default_copy_to( dest::MOI.ModelLike, src::MOI.ModelLike; - copy_names::Bool = false, + copy_names::Bool = true, filter_constraints::Union{Nothing,Function} = nothing, ) if !MOI.supports_incremental_interface(dest, copy_names) From 50c088d6625ba2fdf05c2b94b0f362e57cb9444e Mon Sep 17 00:00:00 2001 From: odow Date: Mon, 9 Aug 2021 15:30:37 +1200 Subject: [PATCH 4/5] Fix failing test --- test/DeprecatedTest/modellike.jl | 2 +- test/dummy.jl | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/test/DeprecatedTest/modellike.jl b/test/DeprecatedTest/modellike.jl index caafc10f3e..efae198a32 100644 --- a/test/DeprecatedTest/modellike.jl +++ b/test/DeprecatedTest/modellike.jl @@ -2,7 +2,7 @@ using Test import MathOptInterface const MOI = MathOptInterface const MOIT = MOI.DeprecatedTest -const MOU = MOI.Utilities +const MOIU = MOI.Utilities @testset "Start value attributes and nothing" begin model = MOIU.MockOptimizer(MOIU.UniversalFallback(MOIU.Model{Float64}())) diff --git a/test/dummy.jl b/test/dummy.jl index 0853387168..b6a8fa18c6 100644 --- a/test/dummy.jl +++ b/test/dummy.jl @@ -14,7 +14,10 @@ function MOI.copy_to( return MOIU.default_copy_to(dest, src, copy_names) end +MOI.supports_incremental_interface(::AbstractDummyModel, ::Bool) = true + MOI.supports(::AbstractDummyModel, ::MOI.ObjectiveSense) = true + function MOI.supports( ::AbstractDummyModel, ::MOI.ConstraintPrimalStart, From a70c32fee242fbe5ef40edc3c60bda99c5aa81ba Mon Sep 17 00:00:00 2001 From: odow Date: Mon, 9 Aug 2021 16:32:10 +1200 Subject: [PATCH 5/5] Improve docstring --- src/Utilities/copy.jl | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/Utilities/copy.jl b/src/Utilities/copy.jl index a85b7c8e69..27cdb7cb9f 100644 --- a/src/Utilities/copy.jl +++ b/src/Utilities/copy.jl @@ -562,14 +562,13 @@ end 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 -[`MathOptInterface.supports_incremental_interface`](@ref) can be used to check -whether `dest` supports the copying a model incrementally. +A default implementation of `MOI.copy_to(dest, src)` for models that implement +the incremental interface, i.e., [`MOI.supports_incremental_interface`](@ref) +returns `true`. -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. +If `filter_constraints` is a `Function`, only constraints for which +`filter_constraints(ci)` returns `true` will be copied, where `ci` is the +[`MOI.ConstraintIndex`](@ref) of the constraint. """ function default_copy_to( dest::MOI.ModelLike,