From 38f940e0bd2c82934a725d3b33187081ba95c782 Mon Sep 17 00:00:00 2001 From: odow Date: Wed, 30 Oct 2024 15:10:58 +1300 Subject: [PATCH 1/7] [Utilities] fix checking supports_constraint in default_copy_to --- src/Utilities/copy.jl | 23 +++++++++++++++++++---- test/Utilities/copy.jl | 29 +++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 4 deletions(-) diff --git a/src/Utilities/copy.jl b/src/Utilities/copy.jl index 76a34dc75f..8beb2cfada 100644 --- a/src/Utilities/copy.jl +++ b/src/Utilities/copy.jl @@ -383,6 +383,21 @@ function default_copy_to(dest::MOI.ModelLike, src::MOI.ModelLike) error("Model $(typeof(dest)) does not support copy_to.") end MOI.empty!(dest) + for (F, S) in MOI.get(src, MOI.ListOfConstraintTypesPresent()) + if F == MOI.VariableIndex + if !MOI.supports_add_constrained_variable(dest, S) + throw(MOI.AddConstraintNotAllowed{F,S}()) + end + elseif F == MOI.VectorOfVariables + if !MOI.supports_add_constrained_variables(dest, S) + throw(MOI.AddConstraintNotAllowed{F,S}()) + end + else + if !MOI.supports_add_constrainet(dest, F, S) + throw(MOI.AddConstraintNotAllowed{F,S}()) + end + end + end index_map, vis_src, constraints_not_added = _copy_variables_with_set(dest, src) # Copy variable attributes @@ -498,8 +513,8 @@ function _add_variable_with_domain( src, index_map, f, - ci::MOI.ConstraintIndex{MOI.VariableIndex,<:MOI.AbstractScalarSet}, -) + ci::MOI.ConstraintIndex{MOI.VariableIndex,S}, +) where {S<: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 @@ -512,8 +527,8 @@ function _add_variable_with_domain( src, index_map, f, - ci::MOI.ConstraintIndex{MOI.VectorOfVariables,<:MOI.AbstractVectorSet}, -) + ci::MOI.ConstraintIndex{MOI.VectorOfVariables,S}, +) where {S<: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) diff --git a/test/Utilities/copy.jl b/test/Utilities/copy.jl index ce39a7e83d..b51624c51f 100644 --- a/test/Utilities/copy.jl +++ b/test/Utilities/copy.jl @@ -1003,6 +1003,35 @@ function test_copy_to_sorted_sets() return end +function test_add_constraint_not_allowed_scalar_variable() + F, S = MOI.VariableIndex, MOI.GreaterThan{Float64} + model = MOI.Utilities.Model{Float64}() + x, _ = MOI.add_constrained_variable(model, MOI.GreaterThan(1.0)) + dest = MOI.FileFormats.CBF.Model() + @test_throws(MOI.AddConstraintNotAllowed{F,S}, MOI.copy_to(dest, model)) + return +end + +function test_add_constraint_not_allowed_vector_variables() + F, S = MOI.VectorOfVariables, MOI.AllDifferent + model = MOI.Utilities.Model{Float64}() + x, _ = MOI.add_constrained_variables(model, MOI.AllDifferent(3)) + dest = MOI.FileFormats.CBF.Model() + @test_throws(MOI.AddConstraintNotAllowed{F,S}, MOI.copy_to(dest, model)) + return +end + +function test_add_constraint_not_allowed_scalar_function() + F, S = MOI.ScalarNonlinearFunction, MOI.EqualTo{Float64} + model = MOI.Utilities.Model{Float64}() + x = MOI.add_variable(model) + f = MOI.ScalarNonlinearFunction(:log, Any[x]) + MOI.add_constraint(model, f, MOI.EqualTo(0.0)) + dest = MOI.FileFormats.CBF.Model() + @test_throws(MOI.AddConstraintNotAllowed{F,S}, MOI.copy_to(dest, model)) + return +end + end # module TestCopy.runtests() From c70d979b70bbba671589f495161244d0a8fe65d4 Mon Sep 17 00:00:00 2001 From: odow Date: Wed, 30 Oct 2024 15:13:06 +1300 Subject: [PATCH 2/7] Update --- src/Utilities/copy.jl | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Utilities/copy.jl b/src/Utilities/copy.jl index 8beb2cfada..422255c688 100644 --- a/src/Utilities/copy.jl +++ b/src/Utilities/copy.jl @@ -513,8 +513,8 @@ function _add_variable_with_domain( src, index_map, f, - ci::MOI.ConstraintIndex{MOI.VariableIndex,S}, -) where {S<:MOI.AbstractScalarSet} + 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 @@ -527,8 +527,8 @@ function _add_variable_with_domain( src, index_map, f, - ci::MOI.ConstraintIndex{MOI.VectorOfVariables,S}, -) where {S<:MOI.AbstractVectorSet} + 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) From 072bdd4bf382a3d938f74e51c45e42bf18ef10eb Mon Sep 17 00:00:00 2001 From: Oscar Dowson Date: Wed, 30 Oct 2024 15:21:02 +1300 Subject: [PATCH 3/7] Update src/Utilities/copy.jl --- 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 422255c688..fa73eb71e2 100644 --- a/src/Utilities/copy.jl +++ b/src/Utilities/copy.jl @@ -393,7 +393,7 @@ function default_copy_to(dest::MOI.ModelLike, src::MOI.ModelLike) throw(MOI.AddConstraintNotAllowed{F,S}()) end else - if !MOI.supports_add_constrainet(dest, F, S) + if !MOI.supports_add_constraint(dest, F, S) throw(MOI.AddConstraintNotAllowed{F,S}()) end end From 5f6f995103698dff56fb62f4be39b3e2c606eaa5 Mon Sep 17 00:00:00 2001 From: Oscar Dowson Date: Wed, 30 Oct 2024 15:25:34 +1300 Subject: [PATCH 4/7] Update src/Utilities/copy.jl --- 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 fa73eb71e2..23629ca062 100644 --- a/src/Utilities/copy.jl +++ b/src/Utilities/copy.jl @@ -393,7 +393,7 @@ function default_copy_to(dest::MOI.ModelLike, src::MOI.ModelLike) throw(MOI.AddConstraintNotAllowed{F,S}()) end else - if !MOI.supports_add_constraint(dest, F, S) + if !MOI.supports_constraint(dest, F, S) throw(MOI.AddConstraintNotAllowed{F,S}()) end end From fe1fa970d44cc542013c728e81e3041e0b2344e5 Mon Sep 17 00:00:00 2001 From: odow Date: Wed, 30 Oct 2024 15:36:31 +1300 Subject: [PATCH 5/7] Update --- src/Utilities/copy.jl | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/Utilities/copy.jl b/src/Utilities/copy.jl index 23629ca062..bd98639c95 100644 --- a/src/Utilities/copy.jl +++ b/src/Utilities/copy.jl @@ -386,16 +386,14 @@ function default_copy_to(dest::MOI.ModelLike, src::MOI.ModelLike) for (F, S) in MOI.get(src, MOI.ListOfConstraintTypesPresent()) if F == MOI.VariableIndex if !MOI.supports_add_constrained_variable(dest, S) - throw(MOI.AddConstraintNotAllowed{F,S}()) + throw(MOI.UnsupportedConstraint{F,S}()) end elseif F == MOI.VectorOfVariables if !MOI.supports_add_constrained_variables(dest, S) - throw(MOI.AddConstraintNotAllowed{F,S}()) - end - else - if !MOI.supports_constraint(dest, F, S) - throw(MOI.AddConstraintNotAllowed{F,S}()) + throw(MOI.UnsupportedConstraint{F,S}()) end + elseif !MOI.supports_constraint(dest, F, S) + throw(MOI.UnsupportedConstraint{F,S}()) end end index_map, vis_src, constraints_not_added = From 6d5f51e9e3e24ad60fc160dc940c1d9ce209fd6d Mon Sep 17 00:00:00 2001 From: odow Date: Wed, 30 Oct 2024 16:14:52 +1300 Subject: [PATCH 6/7] Fix --- test/Utilities/copy.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/Utilities/copy.jl b/test/Utilities/copy.jl index b51624c51f..7d14f1b399 100644 --- a/test/Utilities/copy.jl +++ b/test/Utilities/copy.jl @@ -753,7 +753,7 @@ end function MOI.supports_constraint( ::BoundModel, ::Type{MOI.VariableIndex}, - ::MOI.LessThan{Float64}, + ::Type{MOI.LessThan{Float64}}, ) return true end From 8a11eba8af0110e2e72658b683293a698a8ebda6 Mon Sep 17 00:00:00 2001 From: odow Date: Wed, 30 Oct 2024 16:46:51 +1300 Subject: [PATCH 7/7] Update --- src/Utilities/copy.jl | 44 +++++++++++++++++++++++++------------ test/Utilities/copy.jl | 50 ++++++++++++++++++++++++++++-------------- 2 files changed, 64 insertions(+), 30 deletions(-) diff --git a/src/Utilities/copy.jl b/src/Utilities/copy.jl index bd98639c95..e30be80752 100644 --- a/src/Utilities/copy.jl +++ b/src/Utilities/copy.jl @@ -189,6 +189,9 @@ function _copy_constraints( index_map, cis_src::Vector{MOI.ConstraintIndex{F,S}}, ) where {F,S} + if !MOI.supports_constraint(dest, F, S) + throw(MOI.UnsupportedConstraint{F,S}()) + end return _copy_constraints(dest, src, index_map, index_map[F, S], cis_src) end @@ -383,19 +386,6 @@ function default_copy_to(dest::MOI.ModelLike, src::MOI.ModelLike) error("Model $(typeof(dest)) does not support copy_to.") end MOI.empty!(dest) - for (F, S) in MOI.get(src, MOI.ListOfConstraintTypesPresent()) - if F == MOI.VariableIndex - if !MOI.supports_add_constrained_variable(dest, S) - throw(MOI.UnsupportedConstraint{F,S}()) - end - elseif F == MOI.VectorOfVariables - if !MOI.supports_add_constrained_variables(dest, S) - throw(MOI.UnsupportedConstraint{F,S}()) - end - elseif !MOI.supports_constraint(dest, F, S) - throw(MOI.UnsupportedConstraint{F,S}()) - end - end index_map, vis_src, constraints_not_added = _copy_variables_with_set(dest, src) # Copy variable attributes @@ -424,13 +414,21 @@ struct _CopyVariablesWithSetCache end function _build_copy_variables_with_set_cache( + dest::MOI.ModelLike, src::MOI.ModelLike, cache::_CopyVariablesWithSetCache, ::Type{S}, ) where {S<:MOI.AbstractScalarSet} F = MOI.VariableIndex + supports = + MOI.supports_add_constrained_variable(dest, S) || + MOI.supports_constraint(dest, F, S) indices = MOI.ConstraintIndex{F,S}[] for ci in MOI.get(src, MOI.ListOfConstraintIndices{F,S}()) + # Check this inside the loop so there exists at least one constraint + if !supports + throw(MOI.UnsupportedConstraint{F,S}()) + end 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 @@ -479,13 +477,21 @@ function _is_variable_cone( end function _build_copy_variables_with_set_cache( + dest::MOI.ModelLike, src::MOI.ModelLike, cache::_CopyVariablesWithSetCache, ::Type{S}, ) where {S<:MOI.AbstractVectorSet} F = MOI.VectorOfVariables + supports = + MOI.supports_add_constrained_variables(dest, S) || + MOI.supports_constraint(dest, F, S) indices = MOI.ConstraintIndex{F,S}[] for ci in MOI.get(src, MOI.ListOfConstraintIndices{F,S}()) + # Check this inside the loop so there exists at least one constraint + if !supports + throw(MOI.UnsupportedConstraint{F,S}()) + end f = MOI.get(src, MOI.ConstraintFunction(), ci) if _is_variable_cone(cache, f) for fi in f.variables @@ -544,7 +550,7 @@ function _copy_variables_with_set(dest, 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) + _build_copy_variables_with_set_cache(dest, src, cache, S) end column(x::MOI.VariableIndex) = cache.variable_to_column[x] start_column(x) = column(first(x[1])) @@ -715,9 +721,14 @@ function _try_constrain_variables_on_creation( index_map::IndexMap, ::Type{S}, ) where {S<:MOI.AbstractVectorSet} + supports = MOI.supports_add_constrained_variables(dest, S) not_added = MOI.ConstraintIndex{MOI.VectorOfVariables,S}[] for ci_src in MOI.get(src, MOI.ListOfConstraintIndices{MOI.VectorOfVariables,S}()) + # Check this inside the loop so there exists at least one constraint + if !supports + throw(MOI.UnsupportedConstraint{MOI.VectorOfVariables,S}()) + end f_src = MOI.get(src, MOI.ConstraintFunction(), ci_src) if !allunique(f_src.variables) # Can't add it because there are duplicate variables @@ -743,9 +754,14 @@ function _try_constrain_variables_on_creation( index_map::IndexMap, ::Type{S}, ) where {S<:MOI.AbstractScalarSet} + supports = MOI.supports_add_constrained_variable(dest, S) not_added = MOI.ConstraintIndex{MOI.VariableIndex,S}[] for ci_src in MOI.get(src, MOI.ListOfConstraintIndices{MOI.VariableIndex,S}()) + # Check this inside the loop so there exists at least one constraint + if !supports + throw(MOI.UnsupportedConstraint{MOI.VariableIndex,S}()) + end 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 diff --git a/test/Utilities/copy.jl b/test/Utilities/copy.jl index 7d14f1b399..bdecac30c3 100644 --- a/test/Utilities/copy.jl +++ b/test/Utilities/copy.jl @@ -176,12 +176,8 @@ MOI.empty!(model::ConstrainedVariablesModel) = empty!(model.added_constrained) MOI.supports_incremental_interface(::ConstrainedVariablesModel) = true -function MOI.copy_to( - dest::ConstrainedVariablesModel, - src::MOI.ModelLike; - kwargs..., -) - return MOIU.default_copy_to(dest, src; kwargs...) +function MOI.copy_to(dest::ConstrainedVariablesModel, src::MOI.ModelLike) + return MOIU.default_copy_to(dest, src) end function MOI.add_variables(model::ConstrainedVariablesModel, n) @@ -192,6 +188,13 @@ function MOI.add_variables(model::ConstrainedVariablesModel, n) return MOI.VariableIndex.(m .+ (1:n)) end +function MOI.supports_add_constrained_variables( + ::ConstrainedVariablesModel, + ::Type{<:MOI.AbstractVectorSet}, +) + return true +end + function MOI.add_constrained_variables( model::ConstrainedVariablesModel, set::MOI.AbstractVectorSet, @@ -204,6 +207,14 @@ function MOI.add_constrained_variables( return MOI.VariableIndex.(m .+ (1:MOI.dimension(set))), ci end +function MOI.supports_constraint( + ::ConstrainedVariablesModel, + ::Type{MOI.VectorOfVariables}, + ::Type{<:MOI.AbstractVectorSet}, +) + return true +end + function MOI.add_constraint( ::ConstrainedVariablesModel, func::MOI.VectorOfVariables, @@ -261,6 +272,14 @@ function MOI.add_variable(model::AbstractConstrainedVariablesModel) return MOI.add_variable(model.inner) end +function MOI.supports_constraint( + ::AbstractConstrainedVariablesModel, + ::Type{<:MOI.AbstractFunction}, + ::Type{<:MOI.AbstractSet}, +) + return true +end + function MOI.add_constraint( model::AbstractConstrainedVariablesModel, f::MOI.AbstractFunction, @@ -273,10 +292,9 @@ end function MOI.copy_to( dest::AbstractConstrainedVariablesModel, - src::MOI.ModelLike; - kwargs..., + src::MOI.ModelLike, ) - return MOIU.default_copy_to(dest, src; kwargs...) + return MOIU.default_copy_to(dest, src) end MOI.supports_incremental_interface(::AbstractConstrainedVariablesModel) = true @@ -321,7 +339,7 @@ function MOI.supports_constraint( ::Type{MOI.VectorOfVariables}, ::Type{MOI.Nonnegatives}, ) - return false + return true end function MOI.supports_add_constrained_variables( @@ -1003,32 +1021,32 @@ function test_copy_to_sorted_sets() return end -function test_add_constraint_not_allowed_scalar_variable() +function test_default_copy_to_unsupported_scalar_variable() F, S = MOI.VariableIndex, MOI.GreaterThan{Float64} model = MOI.Utilities.Model{Float64}() x, _ = MOI.add_constrained_variable(model, MOI.GreaterThan(1.0)) dest = MOI.FileFormats.CBF.Model() - @test_throws(MOI.AddConstraintNotAllowed{F,S}, MOI.copy_to(dest, model)) + @test_throws(MOI.UnsupportedConstraint{F,S}, MOI.copy_to(dest, model)) return end -function test_add_constraint_not_allowed_vector_variables() +function test_default_copy_to_unsupported_vector_variables() F, S = MOI.VectorOfVariables, MOI.AllDifferent model = MOI.Utilities.Model{Float64}() x, _ = MOI.add_constrained_variables(model, MOI.AllDifferent(3)) dest = MOI.FileFormats.CBF.Model() - @test_throws(MOI.AddConstraintNotAllowed{F,S}, MOI.copy_to(dest, model)) + @test_throws(MOI.UnsupportedConstraint{F,S}, MOI.copy_to(dest, model)) return end -function test_add_constraint_not_allowed_scalar_function() +function test_default_copy_to_unsupported_scalar_function() F, S = MOI.ScalarNonlinearFunction, MOI.EqualTo{Float64} model = MOI.Utilities.Model{Float64}() x = MOI.add_variable(model) f = MOI.ScalarNonlinearFunction(:log, Any[x]) MOI.add_constraint(model, f, MOI.EqualTo(0.0)) dest = MOI.FileFormats.CBF.Model() - @test_throws(MOI.AddConstraintNotAllowed{F,S}, MOI.copy_to(dest, model)) + @test_throws(MOI.UnsupportedConstraint{F,S}, MOI.copy_to(dest, model)) return end