From ad45651ef3507f7d63a39c4e84872158cf9694c5 Mon Sep 17 00:00:00 2001 From: Oscar Dowson Date: Sat, 22 Jun 2024 11:26:21 +1200 Subject: [PATCH] [Utilities] maintain order of variables in default_copy_to (#2495) --- docs/src/submodules/Utilities/overview.md | 18 +- src/Utilities/copy.jl | 279 +++++++++++++--------- test/Utilities/copy.jl | 46 ++++ 3 files changed, 220 insertions(+), 123 deletions(-) diff --git a/docs/src/submodules/Utilities/overview.md b/docs/src/submodules/Utilities/overview.md index 113e60ae3c..e7a95a63bf 100644 --- a/docs/src/submodules/Utilities/overview.md +++ b/docs/src/submodules/Utilities/overview.md @@ -503,24 +503,24 @@ julia> A.m julia> A.colptr 4-element Vector{Int32}: 0 - 1 - 3 + 2 + 4 5 julia> A.rowval 5-element Vector{Int32}: 0 1 + 1 2 0 - 1 julia> A.nzval 5-element Vector{Float64}: + -4.0 1.0 1.0 2.0 - -4.0 1.0 ``` The lower and upper row bounds: @@ -543,15 +543,15 @@ The lower and upper variable bounds: ```jldoctest matrixofconstraints julia> dest.variables.lower 3-element Vector{Float64}: - 5.0 - -Inf 0.0 + -Inf + 5.0 julia> dest.variables.upper 3-element Vector{Float64}: - 5.0 - 10.0 1.0 + 10.0 + 5.0 ``` Because of larger variations between solvers, the objective can be queried using the standard MOI methods: @@ -563,7 +563,7 @@ julia> F = MOI.get(dest, MOI.ObjectiveFunctionType()) MathOptInterface.ScalarAffineFunction{Float64} julia> F = MOI.get(dest, MOI.ObjectiveFunction{F}()) -0.0 + 1.0 MOI.VariableIndex(3) + 2.0 MOI.VariableIndex(2) - 3.1 MOI.VariableIndex(1) +0.0 + 1.0 MOI.VariableIndex(1) + 2.0 MOI.VariableIndex(2) - 3.1 MOI.VariableIndex(3) ``` Thus, Clp.jl implements [`copy_to`](@ref) methods similar to the following: diff --git a/src/Utilities/copy.jl b/src/Utilities/copy.jl index 37a9e376d0..29d4b8d31b 100644 --- a/src/Utilities/copy.jl +++ b/src/Utilities/copy.jl @@ -154,85 +154,6 @@ function _pass_attribute( return end -""" - _try_constrain_variables_on_creation( - dest::MOI.ModelLike, - src::MOI.ModelLike, - index_map::IndexMap, - ::Type{S}, - ) where {S<:MOI.AbstractVectorSet} - -Copy the constraints of type `MOI.VectorOfVariables`-in-`S` from the model `src` -to the model `dest` and fill `index_map` accordingly. The copy is only done when -the variables to be copied are not already keys of `index_map`. - -It returns a list of the constraints that were not added. -""" -function _try_constrain_variables_on_creation( - dest::MOI.ModelLike, - src::MOI.ModelLike, - index_map::IndexMap, - ::Type{S}, -) where {S<:MOI.AbstractVectorSet} - not_added = MOI.ConstraintIndex{MOI.VectorOfVariables,S}[] - for ci_src in - MOI.get(src, MOI.ListOfConstraintIndices{MOI.VectorOfVariables,S}()) - f_src = MOI.get(src, MOI.ConstraintFunction(), ci_src) - if !allunique(f_src.variables) - # Can't add it because there are duplicate variables - push!(not_added, ci_src) - elseif any(vi -> haskey(index_map, vi), f_src.variables) - # Can't add it because it contains a variable previously added - push!(not_added, ci_src) - else - set = MOI.get(src, MOI.ConstraintSet(), ci_src)::S - vis_dest, ci_dest = MOI.add_constrained_variables(dest, set) - index_map[ci_src] = ci_dest - for (vi_src, vi_dest) in zip(f_src.variables, vis_dest) - index_map[vi_src] = vi_dest - end - end - end - return not_added -end - -""" - _try_constrain_variables_on_creation( - dest::MOI.ModelLike, - src::MOI.ModelLike, - index_map::IndexMap, - ::Type{S}, - ) where {S<:MOI.AbstractScalarSet} - -Copy the constraints of type `MOI.VariableIndex`-in-`S` from the model `src` to -the model `dest` and fill `index_map` accordingly. The copy is only done when the -variables to be copied are not already keys of `index_map`. - -It returns a list of the constraints that were not added. -""" -function _try_constrain_variables_on_creation( - dest::MOI.ModelLike, - src::MOI.ModelLike, - index_map::IndexMap, - ::Type{S}, -) where {S<:MOI.AbstractScalarSet} - not_added = MOI.ConstraintIndex{MOI.VariableIndex,S}[] - for ci_src in - MOI.get(src, MOI.ListOfConstraintIndices{MOI.VariableIndex,S}()) - f_src = MOI.get(src, MOI.ConstraintFunction(), ci_src) - if haskey(index_map, f_src) - # Can't add it because it contains a variable previously added - push!(not_added, ci_src) - else - set = MOI.get(src, MOI.ConstraintSet(), ci_src)::S - vi_dest, ci_dest = MOI.add_constrained_variable(dest, set) - index_map[ci_src] = ci_dest - index_map[f_src] = vi_dest - end - end - return not_added -end - """ _copy_constraints( dest::MOI.ModelLike, @@ -344,22 +265,6 @@ function _pass_constraints( return end -function _copy_free_variables(dest::MOI.ModelLike, index_map::IndexMap, vis_src) - if length(vis_src) == length(index_map.var_map) - return # All variables already added - end - x = MOI.add_variables(dest, length(vis_src) - length(index_map.var_map)) - i = 1 - for vi in vis_src - if !haskey(index_map, vi) - index_map[vi] = x[i] - i += 1 - end - end - @assert i == length(x) + 1 - return -end - _is_variable_function(::Type{MOI.VariableIndex}) = true _is_variable_function(::Type{MOI.VectorOfVariables}) = true _is_variable_function(::Any) = false @@ -478,25 +383,8 @@ function default_copy_to(dest::MOI.ModelLike, src::MOI.ModelLike) error("Model $(typeof(dest)) does not support copy_to.") end MOI.empty!(dest) - vis_src = MOI.get(src, MOI.ListOfVariableIndices()) - index_map = IndexMap() - # The `NLPBlock` assumes that the order of variables does not change (#849) - # Therefore, all VariableIndex and VectorOfVariable constraints are added - # seprately, and no variables constrained-on-creation are added. - has_nlp = MOI.NLPBlock() in MOI.get(src, MOI.ListOfModelAttributesSet()) - constraints_not_added = if has_nlp - Any[ - MOI.get(src, MOI.ListOfConstraintIndices{F,S}()) for - (F, S) in MOI.get(src, MOI.ListOfConstraintTypesPresent()) if - _is_variable_function(F) - ] - else - Any[ - _try_constrain_variables_on_creation(dest, src, index_map, S) - for S in sorted_variable_sets_by_cost(dest, src) - ] - end - _copy_free_variables(dest, index_map, vis_src) + index_map, vis_src, constraints_not_added = + _copy_variables_with_set(dest, src) # Copy variable attributes pass_attributes(dest, src, index_map, vis_src) # Copy model attributes @@ -507,6 +395,169 @@ function default_copy_to(dest::MOI.ModelLike, src::MOI.ModelLike) return index_map end +struct _CopyVariablesWithSetCache + variable_to_column::Dict{MOI.VariableIndex,Int} + constraints_not_added::Vector{Any} + variables_with_domain::Set{MOI.VariableIndex} + variable_cones::Vector{Tuple{Vector{MOI.VariableIndex},Any}} + function _CopyVariablesWithSetCache() + return new( + Dict{MOI.VariableIndex,Int}(), + Any[], + Set{MOI.VariableIndex}(), + Tuple{Vector{MOI.VariableIndex},Any}[], + ) + end +end + +function _build_copy_variables_with_set_cache( + src::MOI.ModelLike, + cache::_CopyVariablesWithSetCache, + ::Type{S}, +) where {S<:MOI.AbstractScalarSet} + F = MOI.VariableIndex + indices = MOI.ConstraintIndex{F,S}[] + for ci in MOI.get(src, MOI.ListOfConstraintIndices{F,S}()) + x = MOI.get(src, MOI.ConstraintFunction(), ci) + if x in cache.variables_with_domain + # `x` is already assigned to a domain. Add this constraint via + # `add_constraint`. + push!(indices, ci) + else + # `x` is not assigned to a domain. Choose to add this constraint via + # `x, ci = add_constraint_variable(model, set)` + push!(cache.variables_with_domain, x) + push!(cache.variable_cones, ([x], ci)) + end + end + if !isempty(indices) + # If indices is not empty, then we have some constraints to add. + push!(cache.constraints_not_added, indices) + end + return +end + +# This function is a heuristic that checks whether `f` should be added via +# `MOI.add_constrained_variables`. +function _is_variable_cone( + cache::_CopyVariablesWithSetCache, + f::MOI.VectorOfVariables, +) + if isempty(f.variables) + # If the dimension is `0`, `f` cannot be added via + # `add_constrained_variables` + return false + end + offset = cache.variable_to_column[f.variables[1]] - 1 + for (i, xi) in enumerate(f.variables) + if xi in cache.variables_with_domain + # The function contains at least one element that is already + # assigned to a domain. We can't add `f` via + # `add_constrained_variables` + return false + elseif cache.variable_to_column[xi] != offset + i + # The variables in the function are not contiguous in their column + # ordering. In theory, we could add `f` via `add_constrained_variables`, + # but this would introduce a permutation so we choose not to. + return false + end + end + return true +end + +function _build_copy_variables_with_set_cache( + src::MOI.ModelLike, + cache::_CopyVariablesWithSetCache, + ::Type{S}, +) where {S<:MOI.AbstractVectorSet} + F = MOI.VectorOfVariables + indices = MOI.ConstraintIndex{F,S}[] + for ci in MOI.get(src, MOI.ListOfConstraintIndices{F,S}()) + f = MOI.get(src, MOI.ConstraintFunction(), ci) + if _is_variable_cone(cache, f) + for fi in f.variables + # We need to assign each variable in `f` to a domain + push!(cache.variables_with_domain, fi) + end + # And we need to add the variables via `add_constrained_variables`. + push!(cache.variable_cones, (f.variables, ci)) + else + # Not a variable cone, so add via `add_constraint`. + push!(indices, ci) + end + end + if !isempty(indices) + # If indices is not empty, then we have some constraints to add. + push!(cache.constraints_not_added, indices) + end + return +end + +function _add_variable_with_domain( + dest, + src, + index_map, + f, + ci::MOI.ConstraintIndex{MOI.VariableIndex,<:MOI.AbstractScalarSet}, +) + set = MOI.get(src, MOI.ConstraintSet(), ci) + dest_x, dest_ci = MOI.add_constrained_variable(dest, set) + index_map[only(f)] = dest_x + index_map[ci] = dest_ci + return +end + +function _add_variable_with_domain( + dest, + src, + index_map, + f, + ci::MOI.ConstraintIndex{MOI.VectorOfVariables,<:MOI.AbstractVectorSet}, +) + set = MOI.get(src, MOI.ConstraintSet(), ci) + dest_x, dest_ci = MOI.add_constrained_variables(dest, set) + for (fi, xi) in zip(f, dest_x) + index_map[fi] = xi + end + index_map[ci] = dest_ci + return +end + +function _copy_variables_with_set(dest, src) + index_map = IndexMap() + vis_src = MOI.get(src, MOI.ListOfVariableIndices()) + cache = _CopyVariablesWithSetCache() + for (i, v) in enumerate(vis_src) + cache.variable_to_column[v] = i + end + for S in sorted_variable_sets_by_cost(dest, src) + _build_copy_variables_with_set_cache(src, cache, S) + end + column(x::MOI.VariableIndex) = cache.variable_to_column[x] + start_column(x) = column(first(x[1])) + current_column = 0 + sort!(cache.variable_cones; by = start_column) + for (f, ci) in cache.variable_cones + offset = column(first(f)) - current_column - 1 + if offset > 0 + dest_x = MOI.add_variables(dest, offset) + for i in 1:offset + index_map[vis_src[current_column+i]] = dest_x[i] + end + end + _add_variable_with_domain(dest, src, index_map, f, ci) + current_column = column(last(f)) + end + offset = length(cache.variable_to_column) - current_column + if offset > 0 + dest_x = MOI.add_variables(dest, offset) + for i in 1:offset + index_map[vis_src[current_column+i]] = dest_x[i] + end + end + return index_map, vis_src, cache.constraints_not_added +end + """ ModelFilter(filter::Function, model::MOI.ModelLike) diff --git a/test/Utilities/copy.jl b/test/Utilities/copy.jl index 3257954d68..ce39a7e83d 100644 --- a/test/Utilities/copy.jl +++ b/test/Utilities/copy.jl @@ -957,6 +957,52 @@ function test_sorted_copy_to() return end +function test_copy_to_empty_VectorOfVariables_constraint() + src = MOI.Utilities.Model{Float64}() + x = MOI.add_variable(src) + F, S = MOI.VectorOfVariables, MOI.Zeros + MOI.add_constraint(src, F([]), S(0)) + dest = MOI.Utilities.Model{Float64}() + MOI.copy_to(dest, src) + @test (F, S) in MOI.get(dest, MOI.ListOfConstraintTypesPresent()) + @test MOI.get(dest, MOI.NumberOfConstraints{F,S}()) == 1 + ci = only(MOI.get(dest, MOI.ListOfConstraintIndices{F,S}())) + @test MOI.get(dest, MOI.ConstraintFunction(), ci) == F([]) + return +end + +function test_copy_to_sorted_sets() + src = MOI.Utilities.Model{Float64}() + # Free variable to start + x1 = MOI.add_variable(src) + x2, _ = MOI.add_constrained_variables(src, MOI.SecondOrderCone(3)) + x3 = MOI.add_variables(src, 2) + MOI.add_constraint(src, MOI.VectorOfVariables(x3), MOI.Nonnegatives(2)) + # A variable with multiple domain sets + x4, _ = MOI.add_constrained_variable(src, MOI.GreaterThan(0.0)) + _ = MOI.add_constraint(src, x4, MOI.LessThan(0.0)) + # A split function. Not added as a variable + MOI.add_constraint( + src, + MOI.VectorOfVariables([x1, x3[2]]), + MOI.Nonpositives(2), + ) + # Repeated set + _, _ = MOI.add_constrained_variables(src, MOI.Nonnegatives(3)) + # An empty constraint + MOI.add_constraint(src, MOI.VectorOfVariables([]), MOI.Zeros(0)) + # Two free variable to end + _ = MOI.add_variables(src, 2) + dest = MOI.Utilities.Model{Float64}() + index_map = MOI.copy_to(dest, src) + x_src = MOI.get(src, MOI.ListOfVariableIndices()) + x_dest = MOI.get(dest, MOI.ListOfVariableIndices()) + for (x, y) in zip(x_src, x_dest) + @test index_map[x] == y + end + return +end + end # module TestCopy.runtests()