From 873d21922703614a5effc28533468146f4fb60cc Mon Sep 17 00:00:00 2001 From: Oscar Dowson Date: Tue, 31 Oct 2023 08:39:18 +1300 Subject: [PATCH] [Bridges] fix some getters of ConstraintFunction and add better tests (#2328) --- .../Constraint/bridges/all_different.jl | 2 +- .../Constraint/bridges/all_different_reif.jl | 2 +- src/Bridges/Constraint/bridges/bin_packing.jl | 2 +- src/Bridges/Constraint/bridges/circuit.jl | 2 +- .../Constraint/bridges/count_at_least.jl | 2 +- .../Constraint/bridges/count_belongs.jl | 2 +- .../Constraint/bridges/count_distinct.jl | 2 +- .../Constraint/bridges/count_distinct_reif.jl | 2 +- .../Constraint/bridges/count_greater_than.jl | 2 +- src/Bridges/Constraint/bridges/interval.jl | 2 +- .../bridges/soc_to_nonconvex_quad.jl | 2 +- src/Bridges/Constraint/bridges/table.jl | 2 +- src/Test/test_basic_constraint.jl | 35 +++++++++++++++++++ src/Utilities/vector_of_constraints.jl | 2 +- test/Bridges/bridge_optimizer.jl | 3 +- test/Utilities/model.jl | 2 +- 16 files changed, 51 insertions(+), 15 deletions(-) diff --git a/src/Bridges/Constraint/bridges/all_different.jl b/src/Bridges/Constraint/bridges/all_different.jl index 654bd14278..c7f454a6b6 100644 --- a/src/Bridges/Constraint/bridges/all_different.jl +++ b/src/Bridges/Constraint/bridges/all_different.jl @@ -126,7 +126,7 @@ function MOI.get( ::MOI.ConstraintFunction, bridge::AllDifferentToCountDistinctBridge, ) - return bridge.f + return copy(bridge.f) end function MOI.get( diff --git a/src/Bridges/Constraint/bridges/all_different_reif.jl b/src/Bridges/Constraint/bridges/all_different_reif.jl index 21f374bd6b..a882a5e81f 100644 --- a/src/Bridges/Constraint/bridges/all_different_reif.jl +++ b/src/Bridges/Constraint/bridges/all_different_reif.jl @@ -139,7 +139,7 @@ function MOI.get( ::MOI.ConstraintFunction, bridge::ReifiedAllDifferentToCountDistinctBridge, ) - return bridge.f + return copy(bridge.f) end function MOI.get( diff --git a/src/Bridges/Constraint/bridges/bin_packing.jl b/src/Bridges/Constraint/bridges/bin_packing.jl index 70803c2f49..413e92c9b7 100644 --- a/src/Bridges/Constraint/bridges/bin_packing.jl +++ b/src/Bridges/Constraint/bridges/bin_packing.jl @@ -117,7 +117,7 @@ function MOI.get( ::MOI.ConstraintFunction, bridge::BinPackingToMILPBridge, ) - return bridge.f + return copy(bridge.f) end function MOI.get( diff --git a/src/Bridges/Constraint/bridges/circuit.jl b/src/Bridges/Constraint/bridges/circuit.jl index 84475204fb..68da9d7287 100644 --- a/src/Bridges/Constraint/bridges/circuit.jl +++ b/src/Bridges/Constraint/bridges/circuit.jl @@ -142,7 +142,7 @@ function MOI.get( ::MOI.ConstraintFunction, bridge::CircuitToMILPBridge, ) - return bridge.f + return copy(bridge.f) end function MOI.get( diff --git a/src/Bridges/Constraint/bridges/count_at_least.jl b/src/Bridges/Constraint/bridges/count_at_least.jl index f01deb3de7..36664d292c 100644 --- a/src/Bridges/Constraint/bridges/count_at_least.jl +++ b/src/Bridges/Constraint/bridges/count_at_least.jl @@ -112,7 +112,7 @@ function MOI.get( ::MOI.ConstraintFunction, bridge::CountAtLeastToCountBelongsBridge, ) - return bridge.f + return copy(bridge.f) end function MOI.get( diff --git a/src/Bridges/Constraint/bridges/count_belongs.jl b/src/Bridges/Constraint/bridges/count_belongs.jl index dc28b16781..51604a3db9 100644 --- a/src/Bridges/Constraint/bridges/count_belongs.jl +++ b/src/Bridges/Constraint/bridges/count_belongs.jl @@ -121,7 +121,7 @@ function MOI.get( ::MOI.ConstraintFunction, bridge::CountBelongsToMILPBridge, ) - return bridge.f + return copy(bridge.f) end function MOI.get( diff --git a/src/Bridges/Constraint/bridges/count_distinct.jl b/src/Bridges/Constraint/bridges/count_distinct.jl index b1c2ddb73d..c4b4081f34 100644 --- a/src/Bridges/Constraint/bridges/count_distinct.jl +++ b/src/Bridges/Constraint/bridges/count_distinct.jl @@ -142,7 +142,7 @@ function MOI.get( ::MOI.ConstraintFunction, bridge::CountDistinctToMILPBridge, ) - return bridge.f + return copy(bridge.f) end function MOI.get( diff --git a/src/Bridges/Constraint/bridges/count_distinct_reif.jl b/src/Bridges/Constraint/bridges/count_distinct_reif.jl index f4538b8ae9..49c5dc6683 100644 --- a/src/Bridges/Constraint/bridges/count_distinct_reif.jl +++ b/src/Bridges/Constraint/bridges/count_distinct_reif.jl @@ -154,7 +154,7 @@ function MOI.get( ::MOI.ConstraintFunction, bridge::ReifiedCountDistinctToMILPBridge, ) - return bridge.f + return copy(bridge.f) end function MOI.get( diff --git a/src/Bridges/Constraint/bridges/count_greater_than.jl b/src/Bridges/Constraint/bridges/count_greater_than.jl index c76881e039..5146b896e4 100644 --- a/src/Bridges/Constraint/bridges/count_greater_than.jl +++ b/src/Bridges/Constraint/bridges/count_greater_than.jl @@ -96,7 +96,7 @@ function MOI.get( ::MOI.ConstraintFunction, bridge::CountGreaterThanToMILPBridge, ) - return bridge.f + return copy(bridge.f) end function MOI.get( diff --git a/src/Bridges/Constraint/bridges/interval.jl b/src/Bridges/Constraint/bridges/interval.jl index f9f35224a4..0c9e501671 100644 --- a/src/Bridges/Constraint/bridges/interval.jl +++ b/src/Bridges/Constraint/bridges/interval.jl @@ -429,7 +429,7 @@ function MOI.get( if bridge.upper !== nothing return MOI.get(model, attr, bridge.upper) end - return bridge.func + return copy(bridge.func) end function MOI.get( diff --git a/src/Bridges/Constraint/bridges/soc_to_nonconvex_quad.jl b/src/Bridges/Constraint/bridges/soc_to_nonconvex_quad.jl index 5b82391510..86ba47d1a1 100644 --- a/src/Bridges/Constraint/bridges/soc_to_nonconvex_quad.jl +++ b/src/Bridges/Constraint/bridges/soc_to_nonconvex_quad.jl @@ -11,7 +11,7 @@ function MOI.get( ::MOI.ConstraintFunction, b::_AbstractSOCtoNonConvexQuadBridge{T}, ) where {T} - return MOI.VectorOfVariables(b.vars) + return MOI.VectorOfVariables(copy(b.vars)) end function MOI.Bridges.added_constrained_variable_types( diff --git a/src/Bridges/Constraint/bridges/table.jl b/src/Bridges/Constraint/bridges/table.jl index c159a7f9d3..764c891b2a 100644 --- a/src/Bridges/Constraint/bridges/table.jl +++ b/src/Bridges/Constraint/bridges/table.jl @@ -114,7 +114,7 @@ function MOI.get( ::MOI.ConstraintFunction, bridge::TableToMILPBridge, ) - return bridge.f + return copy(bridge.f) end function MOI.get( diff --git a/src/Test/test_basic_constraint.jl b/src/Test/test_basic_constraint.jl index 32fd3d698d..6ac1f44b7d 100644 --- a/src/Test/test_basic_constraint.jl +++ b/src/Test/test_basic_constraint.jl @@ -172,6 +172,40 @@ function _set(::Type{T}, ::Type{MOI.HyperRectangle}) where {T} return MOI.HyperRectangle(zeros(T, 3), ones(T, 3)) end +function _test_function_modification( + model::MOI.ModelLike, + config::Config{T}, + c::MOI.ConstraintIndex{F}, + f::F, +) where {T,F<:Union{MOI.ScalarAffineFunction{T},MOI.ScalarQuadraticFunction{T}}} + MOI.Utilities.modify_function!(f, MOI.ScalarConstantChange(f.constant + 1)) + g = MOI.get(model, MOI.ConstraintFunction(), c) + @test !≈(f.constant, g.constant, config) + return +end + +function _test_function_modification( + model::MOI.ModelLike, + config::Config{T}, + c::MOI.ConstraintIndex{F}, + f::F, +) where {T,F<:Union{MOI.VectorAffineFunction{T},MOI.VectorQuadraticFunction{T}}} + new_constants = f.constants .+ one(T) + MOI.Utilities.modify_function!(f, MOI.VectorConstantChange(new_constants)) + g = MOI.get(model, MOI.ConstraintFunction(), c) + @test !≈(f.constants, g.constants, config) + return +end + +function _test_function_modification( + ::MOI.ModelLike, + ::Config{T}, + c::MOI.ConstraintIndex{F}, + ::F, +) where {T,F<:MOI.AbstractFunction} + return +end + function _basic_constraint_test_helper( model::MOI.ModelLike, config::Config{T}, @@ -233,6 +267,7 @@ function _basic_constraint_test_helper( ) _test_attribute_value_type(model, MOI.ConstraintFunction(), c) _test_attribute_value_type(model, MOI.CanonicalConstraintFunction(), c) + _test_function_modification(model, config, c, f) end ### ### Test MOI.ConstraintSet diff --git a/src/Utilities/vector_of_constraints.jl b/src/Utilities/vector_of_constraints.jl index 3a0b272ac7..a2b9182ba6 100644 --- a/src/Utilities/vector_of_constraints.jl +++ b/src/Utilities/vector_of_constraints.jl @@ -103,7 +103,7 @@ function MOI.get( ) where {F,S} MOI.throw_if_not_valid(v, ci) f, _ = v.constraints[ci]::Tuple{F,S} - return f + return copy(f) end function MOI.get( diff --git a/test/Bridges/bridge_optimizer.jl b/test/Bridges/bridge_optimizer.jl index 2647bf947d..f0f98fde8e 100644 --- a/test/Bridges/bridge_optimizer.jl +++ b/test/Bridges/bridge_optimizer.jl @@ -739,7 +739,7 @@ function test_recursive_model_constraint(::Type{T} = Int) where {T} @test MOI.get(b, MOI.ConstraintFunction(), c) ≈ func new_func = T(2) * x MOI.set(b, MOI.ConstraintFunction(), c, new_func) - @test MOI.get(b, MOI.ConstraintFunction(), c) == new_func + @test MOI.get(b, MOI.ConstraintFunction(), c) ≈ new_func MOI.modify(b, c, MOI.ScalarCoefficientChange(x, T(3))) @test MOI.get(b, MOI.ConstraintFunction(), c) ≈ T(3) * x MOI.modify(b, c, MOI.ScalarConstantChange(T(-1))) @@ -753,6 +753,7 @@ function test_recursive_model_constraint(::Type{T} = Int) where {T} @test MOI.is_valid(b, c) MOI.delete(b, c) @test !MOI.is_valid(b, c) + return end function test_recursive_model_objective(::Type{T} = Int) where {T} diff --git a/test/Utilities/model.jl b/test/Utilities/model.jl index d4deafd2e6..fb755c4322 100644 --- a/test/Utilities/model.jl +++ b/test/Utilities/model.jl @@ -301,7 +301,7 @@ function test_quadratic_functions() @test MOI.Utilities.is_canonical(F1) F3 = MOI.get(model, MOI.CanonicalConstraintFunction(), c3) @test F3 ≈ f3 - @test F3 === MOI.get(model, MOI.ConstraintFunction(), c3) + @test F3 ≈ MOI.get(model, MOI.ConstraintFunction(), c3) @test MOI.Utilities.is_canonical(F3) @test MOI.get(model, MOI.CanonicalConstraintFunction(), c1) ≈ f3 @test MOI.Utilities.is_canonical(