From fe8a4bbea3e58e848133245d0b9491e1272ff484 Mon Sep 17 00:00:00 2001 From: odow Date: Thu, 4 Apr 2024 15:06:05 +1300 Subject: [PATCH 01/10] Add test for issue #2452 --- test/Bridges/bridge_optimizer.jl | 54 ++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/test/Bridges/bridge_optimizer.jl b/test/Bridges/bridge_optimizer.jl index 97fe163f03..5af2795138 100644 --- a/test/Bridges/bridge_optimizer.jl +++ b/test/Bridges/bridge_optimizer.jl @@ -1187,6 +1187,60 @@ function test_cannot_unbridge_variable_function() return end +MOI.Utilities.@model( + Model2452, + (), + (), + (MOI.Nonnegatives, MOI.Zeros), + (), + (), + (), + (MOI.VectorOfVariables,), + (MOI.VectorAffineFunction,) +) + +function MOI.supports_constraint( + ::Model2452{T}, + ::Type{MOI.VariableIndex}, + ::Type{ + <:Union{ + MOI.GreaterThan{T}, + MOI.LessThan{T}, + MOI.EqualTo{T}, + MOI.Interval{T}, + MOI.ZeroOne, + MOI.Integer, + }, + }, +) where {T} + return false +end + +function MOI.supports_constraint( + ::Model2452{T}, + ::Type{MOI.VectorOfVariables}, + ::Type{MOI.Reals}, +) where {T} + return false +end + +function test_issue_2452() + src = MOI.Utilities.UniversalFallback(MOI.Utilities.Model{Float64}()) + x = MOI.add_variable(src) + MOI.add_constraint(src, x, MOI.GreaterThan(1.0)) + c = MOI.add_constraint(src, 2.0 * x, MOI.EqualTo(3.0)) + dest = MOI.instantiate(Model2452{Float64}; with_bridge_type = Float64) + index_map = MOI.copy_to(dest, src) + set = MOI.get(dest, MOI.ConstraintSet(), index_map[c]) + @test_broken set == MOI.EqualTo(3.0) + MOI.set(dest, MOI.ConstraintSet(), index_map[c], set) + @test_broken MOI.get(dest, MOI.ConstraintSet(), index_map[c]) == set + new_set = MOI.EqualTo(2.0) + MOI.set(dest, MOI.ConstraintSet(), index_map[c], new_set) + @test_broken MOI.get(dest, MOI.ConstraintSet(), index_map[c]) == new_set + return +end + end # module TestBridgeOptimizer.runtests() From 60c0a94b8565936ef5bfc1d6e78876e46b21f3fb Mon Sep 17 00:00:00 2001 From: odow Date: Thu, 4 Apr 2024 16:12:51 +1300 Subject: [PATCH 02/10] Update --- src/Bridges/bridge_optimizer.jl | 4 +++- test/Bridges/bridge_optimizer.jl | 6 +++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/Bridges/bridge_optimizer.jl b/src/Bridges/bridge_optimizer.jl index a5b5a363b0..491f9b8e32 100644 --- a/src/Bridges/bridge_optimizer.jl +++ b/src/Bridges/bridge_optimizer.jl @@ -1477,7 +1477,9 @@ function MOI.get( MOI.get(b.model, MOI.ConstraintFunction(), ci) end f = unbridged_function(b, func) - return MOI.Utilities.shift_constant(set, -MOI.constant(f)) + g = bridged_function(b, f) + offset = MOI.constant(g) - MOI.constant(f) + return MOI.Utilities.shift_constant(set, offset) end ## Other constraint attributes diff --git a/test/Bridges/bridge_optimizer.jl b/test/Bridges/bridge_optimizer.jl index 5af2795138..cecd67dd01 100644 --- a/test/Bridges/bridge_optimizer.jl +++ b/test/Bridges/bridge_optimizer.jl @@ -1232,12 +1232,12 @@ function test_issue_2452() dest = MOI.instantiate(Model2452{Float64}; with_bridge_type = Float64) index_map = MOI.copy_to(dest, src) set = MOI.get(dest, MOI.ConstraintSet(), index_map[c]) - @test_broken set == MOI.EqualTo(3.0) + @test set == MOI.EqualTo(3.0) MOI.set(dest, MOI.ConstraintSet(), index_map[c], set) - @test_broken MOI.get(dest, MOI.ConstraintSet(), index_map[c]) == set + @test MOI.get(dest, MOI.ConstraintSet(), index_map[c]) == set new_set = MOI.EqualTo(2.0) MOI.set(dest, MOI.ConstraintSet(), index_map[c], new_set) - @test_broken MOI.get(dest, MOI.ConstraintSet(), index_map[c]) == new_set + @test MOI.get(dest, MOI.ConstraintSet(), index_map[c]) == new_set return end From 7d1772f2a31d9137d9419b378d4b8be128569ce6 Mon Sep 17 00:00:00 2001 From: odow Date: Thu, 4 Apr 2024 19:06:53 +1300 Subject: [PATCH 03/10] Add test --- src/Test/test_modification.jl | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/Test/test_modification.jl b/src/Test/test_modification.jl index 76b58e5da2..3277711460 100644 --- a/src/Test/test_modification.jl +++ b/src/Test/test_modification.jl @@ -1061,3 +1061,18 @@ function test_modification_constraint_scalarquadraticcoefficientchange( @test ≈(MOI.get(model, MOI.ConstraintFunction(), c), g, config) return end + +function test_modification_mathoptinterface_issue_2452( + model::MOI.ModelLike, + config::Config{T}, +) where {T} + F, S = MOI.ScalarAffineFunction{T}, MOI.EqualTo{T} + @requires MOI.supports_constraint(model, F, S) + x, _ = MOI.add_constrained_variable(model, MOI.GreaterThan(T(1))) + c = MOI.add_constraint(model, T(2) * x, MOI.EqualTo(T(3))) + @test ≈(MOI.get(dest, MOI.ConstraintFunction(), c), T(2) * x, config) + @test MOI.get(dest, MOI.ConstraintSet(), c) == MOI.EqualTo(T(3)) + MOI.set(dest, MOI.ConstraintSet(), c, MOI.EqualTo(T(2))) + @test MOI.get(dest, MOI.ConstraintSet(), c) == MOI.EqualTo(T(2)) + return +end From 6bb3afcafaeb072eafdba78dc016fcc611236522 Mon Sep 17 00:00:00 2001 From: Oscar Dowson Date: Thu, 4 Apr 2024 19:15:07 +1300 Subject: [PATCH 04/10] Update test_modification.jl --- src/Test/test_modification.jl | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Test/test_modification.jl b/src/Test/test_modification.jl index 3277711460..5589662fe2 100644 --- a/src/Test/test_modification.jl +++ b/src/Test/test_modification.jl @@ -1070,9 +1070,9 @@ function test_modification_mathoptinterface_issue_2452( @requires MOI.supports_constraint(model, F, S) x, _ = MOI.add_constrained_variable(model, MOI.GreaterThan(T(1))) c = MOI.add_constraint(model, T(2) * x, MOI.EqualTo(T(3))) - @test ≈(MOI.get(dest, MOI.ConstraintFunction(), c), T(2) * x, config) - @test MOI.get(dest, MOI.ConstraintSet(), c) == MOI.EqualTo(T(3)) - MOI.set(dest, MOI.ConstraintSet(), c, MOI.EqualTo(T(2))) - @test MOI.get(dest, MOI.ConstraintSet(), c) == MOI.EqualTo(T(2)) + @test ≈(MOI.get(model, MOI.ConstraintFunction(), c), T(2) * x, config) + @test MOI.get(model, MOI.ConstraintSet(), c) == MOI.EqualTo(T(3)) + MOI.set(model, MOI.ConstraintSet(), c, MOI.EqualTo(T(2))) + @test MOI.get(model, MOI.ConstraintSet(), c) == MOI.EqualTo(T(2)) return end From 77b5d4a22c7e85eef0f86de64df7349a38d54481 Mon Sep 17 00:00:00 2001 From: odow Date: Mon, 8 Apr 2024 08:54:40 +1200 Subject: [PATCH 05/10] Add test for supports_shift_constant --- src/Bridges/bridge_optimizer.jl | 11 ++++++++--- test/Bridges/bridge_optimizer.jl | 13 ++++++++++++- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/src/Bridges/bridge_optimizer.jl b/src/Bridges/bridge_optimizer.jl index 491f9b8e32..a3ac6908a5 100644 --- a/src/Bridges/bridge_optimizer.jl +++ b/src/Bridges/bridge_optimizer.jl @@ -1456,8 +1456,8 @@ end function MOI.get( b::AbstractBridgeOptimizer, attr::MOI.ConstraintSet, - ci::MOI.ConstraintIndex{<:MOI.AbstractScalarFunction}, -) + ci::MOI.ConstraintIndex{<:MOI.AbstractScalarFunction,S}, +) where {S} set = if is_bridged(b, ci) MOI.throw_if_not_valid(b, ci) call_in_context(MOI.get, b, ci, attr) @@ -1466,7 +1466,11 @@ function MOI.get( end # This is a scalar function, so if there are variable bridges, it might # contain constants that have been moved into the set. - if !Variable.has_bridges(Variable.bridges(b)) + if !MOI.Utilities.supports_shift_constant(S) + # If it doesn't support shift_constant, then return the set + return set + elseif !Variable.has_bridges(Variable.bridges(b)) + # and the same if there are no variable bridges return set end # The function constant of the bridged function was moved to the set, @@ -1478,6 +1482,7 @@ function MOI.get( end f = unbridged_function(b, func) g = bridged_function(b, f) + # But it's really the difference of the bridged and unbridged functions. offset = MOI.constant(g) - MOI.constant(f) return MOI.Utilities.shift_constant(set, offset) end diff --git a/test/Bridges/bridge_optimizer.jl b/test/Bridges/bridge_optimizer.jl index cecd67dd01..b4c034d813 100644 --- a/test/Bridges/bridge_optimizer.jl +++ b/test/Bridges/bridge_optimizer.jl @@ -1209,7 +1209,6 @@ function MOI.supports_constraint( MOI.EqualTo{T}, MOI.Interval{T}, MOI.ZeroOne, - MOI.Integer, }, }, ) where {T} @@ -1241,6 +1240,18 @@ function test_issue_2452() return end +function test_issue_2452_integer() + src = MOI.Utilities.UniversalFallback(MOI.Utilities.Model{Float64}()) + x = MOI.add_variable(src) + MOI.add_constraint(src, x, MOI.GreaterThan(1.0)) + y = MOI.add_variable(src) + c = MOI.add_constraint(src, 1.0 * y, MOI.Integer()) + dest = MOI.instantiate(Model2452{Float64}; with_bridge_type = Float64) + index_map = MOI.copy_to(dest, src) + @test MOI.get(dest, MOI.ConstraintSet(), index_map[c]) == MOI.Integer() + return +end + end # module TestBridgeOptimizer.runtests() From 465c27ff85e2a34bccabcff68b0d7cc1b1499100 Mon Sep 17 00:00:00 2001 From: odow Date: Mon, 8 Apr 2024 12:32:44 +1200 Subject: [PATCH 06/10] Update --- src/Bridges/bridge_optimizer.jl | 27 ++++++++++++--------------- test/Bridges/bridge_optimizer.jl | 13 +++++++++++++ 2 files changed, 25 insertions(+), 15 deletions(-) diff --git a/src/Bridges/bridge_optimizer.jl b/src/Bridges/bridge_optimizer.jl index a3ac6908a5..4f4acce4b9 100644 --- a/src/Bridges/bridge_optimizer.jl +++ b/src/Bridges/bridge_optimizer.jl @@ -1466,25 +1466,22 @@ function MOI.get( end # This is a scalar function, so if there are variable bridges, it might # contain constants that have been moved into the set. - if !MOI.Utilities.supports_shift_constant(S) - # If it doesn't support shift_constant, then return the set + if !Variable.has_bridges(Variable.bridges(b)) + # If there are no variable bridges, return the set. return set - elseif !Variable.has_bridges(Variable.bridges(b)) - # and the same if there are no variable bridges + elseif !MOI.Utilities.supports_shift_constant(S) + # If it doesn't support shift_constant, then return the set return set end - # The function constant of the bridged function was moved to the set, - # we need to remove it. - func = if is_bridged(b, ci) - call_in_context(MOI.get, b, ci, MOI.ConstraintFunction()) - else - MOI.get(b.model, MOI.ConstraintFunction(), ci) - end - f = unbridged_function(b, func) + # The function constant of the bridged function (if it exists) was moved to + # the set, we need to remove it. + f = MOI.get(b, MOI.ConstraintFunction(), ci) + # f is the "un-bridged" function in terms of the user-variables. We need to + # substitute the variable bridges: g = bridged_function(b, f) - # But it's really the difference of the bridged and unbridged functions. - offset = MOI.constant(g) - MOI.constant(f) - return MOI.Utilities.shift_constant(set, offset) + # g is now the "bridged" function. Add its constant to recover the user's + # original set (since the constant was shifted by -constant(g) when adding). + return MOI.Utilities.shift_constant(set, MOI.constant(g)) end ## Other constraint attributes diff --git a/test/Bridges/bridge_optimizer.jl b/test/Bridges/bridge_optimizer.jl index b4c034d813..e020b9227a 100644 --- a/test/Bridges/bridge_optimizer.jl +++ b/test/Bridges/bridge_optimizer.jl @@ -1240,6 +1240,19 @@ function test_issue_2452() return end +function test_issue_2452_with_constant() + src = MOI.Utilities.UniversalFallback(MOI.Utilities.Model{Float64}()) + x = MOI.add_variable(src) + MOI.add_constraint(src, x, MOI.GreaterThan(1.0)) + MOI.add_constraint(src, 2.0 * x + 1.0, MOI.EqualTo(3.0)) + dest = MOI.instantiate(Model2452{Float64}; with_bridge_type = Float64) + @test_throws( + MOI.ScalarFunctionConstantNotZero, + MOI.copy_to(dest, src), + ) + return +end + function test_issue_2452_integer() src = MOI.Utilities.UniversalFallback(MOI.Utilities.Model{Float64}()) x = MOI.add_variable(src) From d5b5d57bd1981633c3bb8774cb3cd64e0d248440 Mon Sep 17 00:00:00 2001 From: odow Date: Mon, 8 Apr 2024 12:44:25 +1200 Subject: [PATCH 07/10] Fix formatting --- test/Bridges/bridge_optimizer.jl | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/test/Bridges/bridge_optimizer.jl b/test/Bridges/bridge_optimizer.jl index e020b9227a..336c2d4379 100644 --- a/test/Bridges/bridge_optimizer.jl +++ b/test/Bridges/bridge_optimizer.jl @@ -1246,10 +1246,7 @@ function test_issue_2452_with_constant() MOI.add_constraint(src, x, MOI.GreaterThan(1.0)) MOI.add_constraint(src, 2.0 * x + 1.0, MOI.EqualTo(3.0)) dest = MOI.instantiate(Model2452{Float64}; with_bridge_type = Float64) - @test_throws( - MOI.ScalarFunctionConstantNotZero, - MOI.copy_to(dest, src), - ) + @test_throws MOI.ScalarFunctionConstantNotZero MOI.copy_to(dest, src) return end From 25ae2b1bc09790c3a549f20a5092ab130aea6e67 Mon Sep 17 00:00:00 2001 From: Oscar Dowson Date: Mon, 8 Apr 2024 20:31:46 +1200 Subject: [PATCH 08/10] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Benoît Legat --- src/Bridges/bridge_optimizer.jl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Bridges/bridge_optimizer.jl b/src/Bridges/bridge_optimizer.jl index 4f4acce4b9..a5beb322e6 100644 --- a/src/Bridges/bridge_optimizer.jl +++ b/src/Bridges/bridge_optimizer.jl @@ -1479,8 +1479,8 @@ function MOI.get( # f is the "un-bridged" function in terms of the user-variables. We need to # substitute the variable bridges: g = bridged_function(b, f) - # g is now the "bridged" function. Add its constant to recover the user's - # original set (since the constant was shifted by -constant(g) when adding). + # `g` is now the "bridged" function. Add its constant to recover the user's + # original set (since the constant was shifted by `-constant(g)` when adding). return MOI.Utilities.shift_constant(set, MOI.constant(g)) end From 587bdb7f458799f933a13d9d2c25061124894471 Mon Sep 17 00:00:00 2001 From: odow Date: Mon, 8 Apr 2024 20:59:13 +1200 Subject: [PATCH 09/10] Add test for multiple bridges --- test/Bridges/bridge_optimizer.jl | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/test/Bridges/bridge_optimizer.jl b/test/Bridges/bridge_optimizer.jl index 336c2d4379..cd2a227578 100644 --- a/test/Bridges/bridge_optimizer.jl +++ b/test/Bridges/bridge_optimizer.jl @@ -1223,6 +1223,23 @@ function MOI.supports_constraint( return false end +function test_issue_2452_multiple_variable_bridges() + src = MOI.Utilities.UniversalFallback(MOI.Utilities.Model{Float64}()) + x = MOI.add_variable(src) + MOI.add_constraint(src, x, MOI.LessThan(1.0)) + c = MOI.add_constraint(src, 2.0 * x, MOI.EqualTo(3.0)) + dest = MOI.instantiate(Model2452{Float64}; with_bridge_type = Float64) + index_map = MOI.copy_to(dest, src) + set = MOI.get(dest, MOI.ConstraintSet(), index_map[c]) + @test set == MOI.EqualTo(3.0) + MOI.set(dest, MOI.ConstraintSet(), index_map[c], set) + @test MOI.get(dest, MOI.ConstraintSet(), index_map[c]) == set + new_set = MOI.EqualTo(2.0) + MOI.set(dest, MOI.ConstraintSet(), index_map[c], new_set) + @test MOI.get(dest, MOI.ConstraintSet(), index_map[c]) == new_set + return +end + function test_issue_2452() src = MOI.Utilities.UniversalFallback(MOI.Utilities.Model{Float64}()) x = MOI.add_variable(src) From 91384818cda55af0bec209141120a5d80b858a59 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Legat?= Date: Mon, 8 Apr 2024 11:55:33 +0200 Subject: [PATCH 10/10] Add comments --- src/Bridges/bridge_optimizer.jl | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/src/Bridges/bridge_optimizer.jl b/src/Bridges/bridge_optimizer.jl index a5beb322e6..666ecebb10 100644 --- a/src/Bridges/bridge_optimizer.jl +++ b/src/Bridges/bridge_optimizer.jl @@ -1473,14 +1473,24 @@ function MOI.get( # If it doesn't support shift_constant, then return the set return set end - # The function constant of the bridged function (if it exists) was moved to - # the set, we need to remove it. + # When the constraint is added with function `f` and set `set_f`, + # the function is bridged into `g` with set `set` by + # `g, set = bridged_constraint_function(b, f, set_f)`. + # By doing so, the function constant of the bridged function (if it exists) + # was moved to `set`, we need to remove it to recover `set_f`. + # The function `f` contains the variables in the context of `ci` + # (which should match `Variable.bridges(b).current_context` since no code + # outside of that context has references to `ci`) and + # the constraint `g` contains the variables of `b.model`. + # The following line recovers `f` in the context of + # `Variables.map(b).current_context`. f = MOI.get(b, MOI.ConstraintFunction(), ci) - # f is the "un-bridged" function in terms of the user-variables. We need to - # substitute the variable bridges: + # We need to substitute the variable bridges to recover the function `g` + # that was given at the creation of the bridge. g = bridged_function(b, f) - # `g` is now the "bridged" function. Add its constant to recover the user's - # original set (since the constant was shifted by `-constant(g)` when adding). + # Since `bridged_constraint_function(b, f, set_f)` used + # `set = shift_constant(set_f, -MOI.constant(g))`, we need + # to do the opposite to recover `set_f`. return MOI.Utilities.shift_constant(set, MOI.constant(g)) end