From 7a1e728fb5a08bbdeb9de0f501f0d8dd5245f9a2 Mon Sep 17 00:00:00 2001 From: odow Date: Mon, 6 Dec 2021 12:24:46 +1300 Subject: [PATCH 1/6] Fix logic in cost_of_bridging --- src/Utilities/copy.jl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Utilities/copy.jl b/src/Utilities/copy.jl index b92dd6b335..ee0f5a716a 100644 --- a/src/Utilities/copy.jl +++ b/src/Utilities/copy.jl @@ -348,7 +348,7 @@ function _cost_of_bridging( ::Type{S}, ) where {S<:MOI.AbstractScalarSet} return ( - MOI.get(dest, MOI.VariableBridgingCost{S}()) - + MOI.get(dest, MOI.VariableBridgingCost{S}()) + MOI.get(dest, MOI.ConstraintBridgingCost{MOI.VariableIndex,S}()), # In case of ties, we give priority to vector sets. See issue #987. false, @@ -360,7 +360,7 @@ function _cost_of_bridging( ::Type{S}, ) where {S<:MOI.AbstractVectorSet} return ( - MOI.get(dest, MOI.VariableBridgingCost{S}()) - + MOI.get(dest, MOI.VariableBridgingCost{S}()) + MOI.get(dest, MOI.ConstraintBridgingCost{MOI.VectorOfVariables,S}()), # In case of ties, we give priority to vector sets. See issue #987 true, From cc07e80d1a792062752f79761c49acfe7a0504a7 Mon Sep 17 00:00:00 2001 From: odow Date: Mon, 6 Dec 2021 14:10:13 +1300 Subject: [PATCH 2/6] Update --- src/Utilities/copy.jl | 20 +++++++---------- test/Utilities/copy.jl | 51 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 12 deletions(-) diff --git a/src/Utilities/copy.jl b/src/Utilities/copy.jl index ee0f5a716a..a29d36fcf5 100644 --- a/src/Utilities/copy.jl +++ b/src/Utilities/copy.jl @@ -347,24 +347,20 @@ function _cost_of_bridging( dest::MOI.ModelLike, ::Type{S}, ) where {S<:MOI.AbstractScalarSet} - return ( - MOI.get(dest, MOI.VariableBridgingCost{S}()) + - MOI.get(dest, MOI.ConstraintBridgingCost{MOI.VariableIndex,S}()), - # In case of ties, we give priority to vector sets. See issue #987. - false, - ) + x = MOI.get(dest, MOI.VariableBridgingCost{S}()) + y = MOI.get(dest, MOI.ConstraintBridgingCost{MOI.VariableIndex,S}()) + # In case of ties, we give priority to vector sets. See issue #987. + return (x - y, false, x + y) end function _cost_of_bridging( dest::MOI.ModelLike, ::Type{S}, ) where {S<:MOI.AbstractVectorSet} - return ( - MOI.get(dest, MOI.VariableBridgingCost{S}()) + - MOI.get(dest, MOI.ConstraintBridgingCost{MOI.VectorOfVariables,S}()), - # In case of ties, we give priority to vector sets. See issue #987 - true, - ) + x = MOI.get(dest, MOI.VariableBridgingCost{S}()) + y = MOI.get(dest, MOI.ConstraintBridgingCost{MOI.VectorOfVariables,S}()) + # In case of ties, we give priority to vector sets. See issue #987 + return (x - y, true, x + y) end """ diff --git a/test/Utilities/copy.jl b/test/Utilities/copy.jl index 74720b0381..9f7bf309a3 100644 --- a/test/Utilities/copy.jl +++ b/test/Utilities/copy.jl @@ -896,6 +896,57 @@ function test_copy_of_constraints_passed_as_copy_accross_layers() return end +struct Optimizer1698_1 <: MOI.AbstractOptimizer end + +function MOI.supports_constraint( + ::Optimizer1698_1, + ::Type{MOI.VariableIndex}, + ::Type{MOI.Integer}, +) + return true +end + +function MOI.supports_constraint( + ::Optimizer1698_1, + ::Type{<:Union{MOI.VectorOfVariables,MOI.VectorAffineFunction{Float64}}}, + ::Type{<:Union{MOI.Nonnegatives,MOI.SecondOrderCone}}, +) + return true +end + +function test_sorted_variable_sets_by_cost_1() + src = MOI.Utilities.Model{Float64}() + x = MOI.add_variable(src) + y = MOI.add_variables(src, 2) + MOI.add_constraint(src, x, MOI.GreaterThan(1.0)) + MOI.add_constraint(src, x, MOI.Integer()) + MOI.add_constraint(src, y, MOI.SecondOrderCone(2)) + dest = MOI.Bridges.full_bridge_optimizer(Optimizer1698_1(), Float64) + @test MOI.Utilities.sorted_variable_sets_by_cost(dest, src) == + [MOI.Integer, MOI.GreaterThan{Float64}, MOI.SecondOrderCone] + return +end + +struct Optimizer1698_2 <: MOI.AbstractOptimizer end + +function MOI.supports_constraint( + ::Optimizer1698_2, + ::Type{<:Union{MOI.VectorOfVariables,MOI.VectorAffineFunction{Float64}}}, + ::Type{<:Union{MOI.Nonnegatives,MOI.Nonpositives}}, +) + return true +end + +function test_sorted_variable_sets_by_cost_2() + src = MOI.Utilities.Model{Float64}() + MOI.add_constrained_variables(src, MOI.Nonnegatives(2)) + MOI.add_constrained_variables(src, MOI.Zeros(2)) + dest = MOI.Bridges.full_bridge_optimizer(Optimizer1698_2(), Float64) + @test MOI.Utilities.sorted_variable_sets_by_cost(dest, src) == + [MOI.Nonnegatives, MOI.Zeros] + return +end + end # module TestCopy.runtests() From 6bb250bbb757cf331f4cec37e3062d11943e19bb Mon Sep 17 00:00:00 2001 From: odow Date: Tue, 7 Dec 2021 17:09:06 +1300 Subject: [PATCH 3/6] Updates --- src/Utilities/copy.jl | 24 +++++++++++++++++++----- test/Utilities/copy.jl | 10 +++++----- 2 files changed, 24 insertions(+), 10 deletions(-) diff --git a/src/Utilities/copy.jl b/src/Utilities/copy.jl index a29d36fcf5..a9520dc39d 100644 --- a/src/Utilities/copy.jl +++ b/src/Utilities/copy.jl @@ -349,8 +349,7 @@ function _cost_of_bridging( ) where {S<:MOI.AbstractScalarSet} x = MOI.get(dest, MOI.VariableBridgingCost{S}()) y = MOI.get(dest, MOI.ConstraintBridgingCost{MOI.VariableIndex,S}()) - # In case of ties, we give priority to vector sets. See issue #987. - return (x - y, false, x + y) + return !iszero(x), x - y, true end function _cost_of_bridging( @@ -359,15 +358,30 @@ function _cost_of_bridging( ) where {S<:MOI.AbstractVectorSet} x = MOI.get(dest, MOI.VariableBridgingCost{S}()) y = MOI.get(dest, MOI.ConstraintBridgingCost{MOI.VectorOfVariables,S}()) - # In case of ties, we give priority to vector sets. See issue #987 - return (x - y, true, x + y) + return !iszero(x), x - y, false end """ sorted_variable_sets_by_cost(dest::MOI.ModelLike, src::MOI.ModelLike) -Returns a `Vector{Type}` of the set types corresponding to `VariableIndex` and +Return a `Vector{Type}` of the set types corresponding to `VariableIndex` and `VectorOfVariables` constraints in the order in which they should be added. + +The order is important because some solvers require variables to be added in +particular order, and the order can also impact the bridging decisions. + +The sorting happens in the `_cost_of_bridging` function and has three main +considerations: + +1. First add sets for which the `VariableBridgingCost` is `0`. This ensures that + we minimize the number of variable bridges that get added. (Variable bridges + are bad because they produce an expression that needs substituting into the + remainder of the model.) +2. Then add sets for which the VariableBridgingCost is smaller than the + `ConstraintBridgingCost` (again avoiding variable bridges). +3. Finally, break any remaining ties in favor of `AbstractVectorSet`s. This + ensures we attempt to add large blocks of variables (e.g., such as PSD + matrices) before we add things like variable bounds. """ function sorted_variable_sets_by_cost(dest::MOI.ModelLike, src::MOI.ModelLike) constraint_types = MOI.get(src, MOI.ListOfConstraintTypesPresent()) diff --git a/test/Utilities/copy.jl b/test/Utilities/copy.jl index 9f7bf309a3..e41523b57f 100644 --- a/test/Utilities/copy.jl +++ b/test/Utilities/copy.jl @@ -449,7 +449,7 @@ function test_create_variables_using_supports_add_constrained_variable() dest = OrderConstrainedVariablesModel() bridged_dest = MOI.Bridges.full_bridge_optimizer(dest, Float64) @test MOIU.sorted_variable_sets_by_cost(bridged_dest, src) == - Type[MOI.Zeros, MOI.Nonnegatives, MOI.Nonpositives] + Type[MOI.Nonnegatives, MOI.Zeros, MOI.Nonpositives] @test MOI.supports_add_constrained_variables(bridged_dest, MOI.Nonnegatives) @test MOI.get(bridged_dest, MOI.VariableBridgingCost{MOI.Nonnegatives}()) == 0.0 @@ -486,12 +486,12 @@ function test_create_variables_using_supports_add_constrained_variable() MOI.ConstraintBridgingCost{MOI.VectorOfVariables,MOI.Zeros}(), ) == 2.0 index_map = MOI.copy_to(bridged_dest, src) - @test length(dest.constraintIndices) == 4 + @test length(dest.constraintIndices) == 6 dest = ReverseOrderConstrainedVariablesModel() bridged_dest = MOI.Bridges.full_bridge_optimizer(dest, Float64) @test MOIU.sorted_variable_sets_by_cost(bridged_dest, src) == - Type[MOI.Zeros, MOI.Nonpositives, MOI.Nonnegatives] + Type[MOI.Nonpositives, MOI.Zeros, MOI.Nonnegatives] @test MOI.supports_add_constrained_variables(bridged_dest, MOI.Nonnegatives) @test MOI.get(bridged_dest, MOI.VariableBridgingCost{MOI.Nonnegatives}()) == 2.0 @@ -528,7 +528,7 @@ function test_create_variables_using_supports_add_constrained_variable() MOI.ConstraintBridgingCost{MOI.VectorOfVariables,MOI.Zeros}(), ) == 3.0 index_map = MOI.copy_to(bridged_dest, src) - @test length(dest.constraintIndices) == 4 + @test length(dest.constraintIndices) == 6 # With single variables src = MOIU.Model{Float64}() @@ -923,7 +923,7 @@ function test_sorted_variable_sets_by_cost_1() MOI.add_constraint(src, y, MOI.SecondOrderCone(2)) dest = MOI.Bridges.full_bridge_optimizer(Optimizer1698_1(), Float64) @test MOI.Utilities.sorted_variable_sets_by_cost(dest, src) == - [MOI.Integer, MOI.GreaterThan{Float64}, MOI.SecondOrderCone] + [MOI.SecondOrderCone, MOI.Integer, MOI.GreaterThan{Float64}] return end From 64df5470db07a92f467381371d747dd074ecc425 Mon Sep 17 00:00:00 2001 From: Oscar Dowson Date: Wed, 8 Dec 2021 09:46:06 +1300 Subject: [PATCH 4/6] Update copy.jl --- src/Utilities/copy.jl | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/Utilities/copy.jl b/src/Utilities/copy.jl index a9520dc39d..40be65757d 100644 --- a/src/Utilities/copy.jl +++ b/src/Utilities/copy.jl @@ -374,11 +374,10 @@ The sorting happens in the `_cost_of_bridging` function and has three main considerations: 1. First add sets for which the `VariableBridgingCost` is `0`. This ensures that - we minimize the number of variable bridges that get added. (Variable bridges - are bad because they produce an expression that needs substituting into the - remainder of the model.) + we minimize the number of variable bridges that get added. 2. Then add sets for which the VariableBridgingCost is smaller than the - `ConstraintBridgingCost` (again avoiding variable bridges). + `ConstraintBridgingCost` so they can get added with + `add_constrained_variable(s)`. 3. Finally, break any remaining ties in favor of `AbstractVectorSet`s. This ensures we attempt to add large blocks of variables (e.g., such as PSD matrices) before we add things like variable bounds. From fc6656c36c18d612e0534c6642641a5c1279c250 Mon Sep 17 00:00:00 2001 From: odow Date: Thu, 9 Dec 2021 10:35:21 +1300 Subject: [PATCH 5/6] Update docstring --- src/Utilities/copy.jl | 51 +++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 49 insertions(+), 2 deletions(-) diff --git a/src/Utilities/copy.jl b/src/Utilities/copy.jl index 40be65757d..5e25e27450 100644 --- a/src/Utilities/copy.jl +++ b/src/Utilities/copy.jl @@ -367,8 +367,7 @@ end Return a `Vector{Type}` of the set types corresponding to `VariableIndex` and `VectorOfVariables` constraints in the order in which they should be added. -The order is important because some solvers require variables to be added in -particular order, and the order can also impact the bridging decisions. +## How the order is computed The sorting happens in the `_cost_of_bridging` function and has three main considerations: @@ -381,6 +380,54 @@ considerations: 3. Finally, break any remaining ties in favor of `AbstractVectorSet`s. This ensures we attempt to add large blocks of variables (e.g., such as PSD matrices) before we add things like variable bounds. + +## Why the order is important + +The order is important because some solvers require variables to be added in +particular order, and the order can also impact the bridging decisions. + +We favor adding first variables that won't use variables bridges because then +the variable constraints on the same variable can still be added as +`VariableIndex` or `VectorOfVariables` constraints. + +If a variable does need variable bridges and is part of another variable +constraint, then the other variable constraint will be force-bridged into affine +constraints, so there is a hidden cost in terms of number of additional number +of bridges that will need to be used. + +In fact, if the order does matter (in the sense that changing the order of the +vector returned by this function leads to a different formulation), it means the +variable _is_ in at least in one other variable constraint. Thus, in a sense we +could do `x - y + sign(x)`` but `!iszero(x), x - y` is fine + +## Example + +A key example is Pajarito. It supports `VariableIndex`-in-`Integer` and +`VectorAffine`-in-`Nonnegatives`. If the user writes: +```julia +@variable(model, x >= 1, Int) +``` +then we need to add two variable-related constraints: + * `VariableIndex`-in-`Integer` + * `VariableIndex`-in-`GreaterThan` +The first is natively supported and the variable and constraint bridging cost is +0. The second must be bridged to `VectorAffineFunction`-in-`Nonnegatives` via +`x - 1 in Nonnegatives(1)`, and the variable and constraint bridging cost is +`1` in both cases. + +If the order is `[Integer, GreaterThan]`, then we add `x ∈ Integer` and +`x - 1 in Nonnegatives`. Both are natively supported and it only requires a +single constraint bridge. + +If the order is `[GreaterThan, Integer]`, then we add a new variable constrained +to `y ∈ Nonnegatives` and end up with an expression from the variable bridge of +`x = y + 1`. Then when we add the Integer constraint, we get `y + 1 in Integer`, +which is not natively supported. Therefore, we need to add `y + 1 - z ∈ Zeros` +and `z ∈ Integer`. Oops! This cost an extra variable, a variable bridge of +`x = y + 1`and a `Zeros` constraint. + +Unfortunately, we don't have a good way of computing the updated costs for other +constraints if a variable bridge is chosen. """ function sorted_variable_sets_by_cost(dest::MOI.ModelLike, src::MOI.ModelLike) constraint_types = MOI.get(src, MOI.ListOfConstraintTypesPresent()) From 0a647e806268b5ce5c9eae1a72191da47fe450f0 Mon Sep 17 00:00:00 2001 From: Oscar Dowson Date: Thu, 9 Dec 2021 11:10:37 +1300 Subject: [PATCH 6/6] Update copy.jl --- src/Utilities/copy.jl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Utilities/copy.jl b/src/Utilities/copy.jl index 5e25e27450..78b1c3d9eb 100644 --- a/src/Utilities/copy.jl +++ b/src/Utilities/copy.jl @@ -397,8 +397,8 @@ of bridges that will need to be used. In fact, if the order does matter (in the sense that changing the order of the vector returned by this function leads to a different formulation), it means the -variable _is_ in at least in one other variable constraint. Thus, in a sense we -could do `x - y + sign(x)`` but `!iszero(x), x - y` is fine +variable is in at least one other variable constraint. Thus, in a sense we +could do `x - y + sign(x)`` but `!iszero(x), x - y` is fine. ## Example