Add CanonicalConstraintFunction attribute

docs/src/apireference.md

@@ -276,6 +276,7 @@ ConstraintPrimal
 ConstraintDual
 ConstraintBasisStatus
 ConstraintFunction
+CanonicalConstraintFunction
 ConstraintSet
 ```
 

src/Bridges/bridge.jl

@@ -61,6 +61,11 @@ function MOI.get(::MOI.ModelLike, attr::MOI.AbstractConstraintAttribute,
     throw(ArgumentError("Bridge of type `$(typeof(bridge))` does not support accessing the attribute `$attr`."))
 end
 
+function MOI.get(model::MOI.ModelLike, ::MOI.CanonicalConstraintFunction,
+                 bridge::AbstractBridge)
+    return MOI.Utilities.canonical(MOI.get(model, MOI.ConstraintFunction(), bridge))
+end
+
 """
     function MOI.set(model::MOI.ModelLike, attr::MOI.AbstractConstraintAttribute,
                      bridge::AbstractBridge, value)

src/FileFormats/MOF/MOF.jl

@@ -24,6 +24,7 @@ struct Nonlinear <: MOI.AbstractScalarFunction
     expr::Expr
 end
 Base.copy(nonlinear::Nonlinear) = Nonlinear(copy(nonlinear.expr))
+MOI.Utilities.canonicalize!(nonlinear::Nonlinear) = nonlinear
 
 MOI.Utilities.@model(InnerModel,
     (MOI.ZeroOne, MOI.Integer),

src/Utilities/functions.jl

@@ -363,6 +363,8 @@ function unsafe_add(t1::VT, t2::VT) where VT <: Union{MOI.VectorAffineTerm,
     return VT(t1.output_index, scalar_term)
 end
 
+is_canonical(::Union{MOI.SingleVariable, MOI.VectorOfVariables}) = true
+
 """
     is_canonical(f::Union{ScalarAffineFunction, VectorAffineFunction})
 
@@ -421,12 +423,16 @@ Returns the function in a canonical form, i.e.
 * The terms appear in increasing order of variable where there the order of the variables is the order of their value.
 * For a `AbstractVectorFunction`, the terms are sorted in ascending order of output index.
 
+The output of `canonical` can be assumed to be a copy of `f`, even for `VectorOfVariables`.
+
 ### Examples
 If `x` (resp. `y`, `z`) is `VariableIndex(1)` (resp. 2, 3).
 The canonical representation of `ScalarAffineFunction([y, x, z, x, z], [2, 1, 3, -2, -3], 5)` is `ScalarAffineFunction([x, y], [-1, 2], 5)`.
 
 """
-canonical(f::Union{SAF, VAF, SQF, VQF}) = canonicalize!(copy(f))
+canonical(f::MOI.AbstractFunction) = canonicalize!(copy(f))
+
+canonicalize!(f::Union{MOI.VectorOfVariables, MOI.SingleVariable}) = f
 
 """
     canonicalize!(f::Union{ScalarAffineFunction, VectorAffineFunction})

src/Utilities/model.jl

@@ -547,7 +547,12 @@ function MOI.add_constraint(model::AbstractModel, f::F, s::S) where {F<:MOI.Abst
         # `@model`'s doc.
         ci = CI{F, S}(model.nextconstraintid += 1)
         # f needs to be copied, see #2
-        push!(model.constrmap, _add_constraint(model, ci, copy(f), copy(s)))
+        # We canonicalize the constraint so that solvers can avoid having to canonicalize
+        # it most of the time (they can check if they need to with `is_canonical`.
+        # Note that the canonicalization is not guaranteed if for instance
+        # `modify` is called and adds a new term.
+        # See https://github.com/jump-dev/MathOptInterface.jl/pull/1118
+        push!(model.constrmap, _add_constraint(model, ci, canonical(f), copy(s)))
         return ci
     else
         throw(MOI.UnsupportedConstraint{F, S}())

src/Utilities/universalfallback.jl

@@ -206,6 +206,15 @@ function _get(uf, attr::MOI.AbstractConstraintAttribute, ci::CI)
     end
     return get(attribute_dict, ci, nothing)
 end
+function _get(uf, attr::MOI.CanonicalConstraintFunction, ci::MOI.ConstraintIndex)
+    return MOI.get_fallback(uf, attr, ci)
+    func = MOI.get(uf, MOI.ConstraintFunction(), ci)
+    if is_canonical(func)
+        return func
+    else
+        return canonical(func)
+    end
+end
 function MOI.get(uf::UniversalFallback,
                  attr::Union{MOI.AbstractOptimizerAttribute,
                              MOI.AbstractModelAttribute})
@@ -216,9 +225,18 @@ function MOI.get(uf::UniversalFallback,
     end
 end
 function MOI.get(uf::UniversalFallback,
-                 attr::Union{MOI.AbstractVariableAttribute,
-                             MOI.AbstractConstraintAttribute}, idx::MOI.Index)
-    if MOI.supports(uf.model, attr, typeof(idx))
+                 attr::MOI.AbstractConstraintAttribute,
+                 idx::MOI.ConstraintIndex{F, S}) where {F, S}
+    if MOI.supports_constraint(uf.model, F, S) &&
+        (!MOI.is_copyable(attr) || MOI.supports(uf.model, attr, typeof(idx)))
+        MOI.get(uf.model, attr, idx)
+    else
+        _get(uf, attr, idx)
+    end
+end
+function MOI.get(uf::UniversalFallback,
+                 attr::MOI.AbstractVariableAttribute, idx::MOI.VariableIndex)
+    if !MOI.is_copyable(attr) || MOI.supports(uf.model, attr, typeof(idx))
         MOI.get(uf.model, attr, idx)
     else
         _get(uf, attr, idx)
@@ -439,7 +457,7 @@ function MOI.add_constraint(uf::UniversalFallback, f::MOI.AbstractFunction, s::M
         constraints = get!(uf.constraints, (F, S)) do
             OrderedDict{CI{F, S}, Tuple{F, S}}()
         end::OrderedDict{CI{F, S}, Tuple{F, S}}
-        ci = _new_constraint_index(uf, f, s)
+        ci = _new_constraint_index(uf, canonical(f), copy(s))
         constraints[ci] = (f, s)
         return ci
     end

src/attributes.jl

@@ -1102,6 +1102,44 @@ struct ConstraintBasisStatus <: AbstractConstraintAttribute
 end
 ConstraintBasisStatus() = ConstraintBasisStatus(1)
 
+"""
+    CanonicalConstraintFunction()
+
+A constraint attribute for a canonical representation of the
+[`AbstractFunction`](@ref) object used to define the constraint.
+Getting this attribute is guaranteed to return a function that is equivalent but
+not necessarily identical to the function provided by the user.
+
+By default, `MOI.get(model, MOI.CanonicalConstraintFunction(), ci)` fallbacks to
+`MOI.Utilities.canonical(MOI.get(model, MOI.ConstraintFunction(), ci))`.
+However, if `model` knows that the constraint function is canonical then it can
+implement a specialized method that directly return the function without calling
+[`Utilities.canonical`](@ref). Therefore, the value returned **cannot** be
+assumed to be a copy of the function stored in `model`.
+Moreover, [`Utilities.Model`](@ref) checks with [`Utilities.is_canonical`](@ref)
+whether the function stored internally is already canonical and if it's the case,
+then it returns the function stored internally instead of a copy.
+"""
+struct CanonicalConstraintFunction <: AbstractConstraintAttribute end
+
+function get_fallback(model::ModelLike, ::CanonicalConstraintFunction, ci::ConstraintIndex)
+    func = get(model, ConstraintFunction(), ci)
+    # In `Utilities.AbstractModel` and `Utilities.UniversalFallback`,
+    # the function is canonicalized in `add_constraint` so it might already
+    # be canonical. In other models, the constraint might have been copied from
+    # from one of these two model so there is in fact a good chance of the
+    # function being canonical in any model type.
+    # As `is_canonical` is quite cheap compared to `canonical` which
+    # requires a copy and sorting the terms, it is worth checking.
+    if Utilities.is_canonical(func)
+        return func
+    else
+        return Utilities.canonical(func)
+    end
+
+    return Utilities.canonical(get(model, ConstraintFunction(), ci))
+end
+
 """
     ConstraintFunction()
 
@@ -1351,13 +1389,13 @@ _result_index_field(attr::DualStatus) = attr.N
 
 
 # Cost of bridging constrained variable in S
-struct VariableBridgingCost{S <: AbstractSet} <: AbstractModelAttribute 
+struct VariableBridgingCost{S <: AbstractSet} <: AbstractModelAttribute
 end
 get_fallback(model::ModelLike, ::VariableBridgingCost{S}) where {S<:AbstractScalarSet} = supports_add_constrained_variable(model, S) ? 0.0 : Inf
 get_fallback(model::ModelLike, ::VariableBridgingCost{S}) where {S<:AbstractVectorSet} = supports_add_constrained_variables(model, S) ? 0.0 : Inf
 
 # Cost of bridging F-in-S constraints
-struct ConstraintBridgingCost{F <: AbstractFunction, S <: AbstractSet} <: AbstractModelAttribute 
+struct ConstraintBridgingCost{F <: AbstractFunction, S <: AbstractSet} <: AbstractModelAttribute
 end
 get_fallback(model::ModelLike, ::ConstraintBridgingCost{F, S}) where {F<:AbstractFunction, S<:AbstractSet} = supports_constraint(model, F, S) ? 0.0 : Inf
 
@@ -1421,8 +1459,9 @@ method should be defined for attributes which are copied indirectly during
 * [`ObjectiveFunctionType`](@ref): this attribute is set indirectly when setting
   the [`ObjectiveFunction`](@ref) attribute.
 * [`NumberOfConstraints`](@ref), [`ListOfConstraintIndices`](@ref),
-  [`ListOfConstraints`](@ref), [`ConstraintFunction`](@ref) and
-  [`ConstraintSet`](@ref): these attributes are set indirectly by
+  [`ListOfConstraints`](@ref), [`CanonicalConstraintFunction`](@ref),
+  [`ConstraintFunction`](@ref) and [`ConstraintSet`](@ref):
+  these attributes are set indirectly by
   [`add_constraint`](@ref) and [`add_constraints`](@ref).
 """
 function is_copyable(attr::AnyAttribute)
@@ -1440,6 +1479,7 @@ function is_copyable(::Union{ListOfOptimizerAttributesSet,
                              ObjectiveFunctionType,
                              ListOfConstraintIndices,
                              ListOfConstraints,
+                             CanonicalConstraintFunction,
                              ConstraintFunction,
                              ConstraintSet,
                              VariableBridgingCost,

test/Bridges/Constraint/rsoc.jl

@@ -63,6 +63,9 @@ end
         MOI.ListOfConstraintIndices{MOI.VectorAffineFunction{Float64},
                                     MOI.SecondOrderCone}()))
 
+    @test !MOI.Utilities.is_canonical(MOI.get(bridged_mock, MOI.ConstraintFunction(), ci))
+    @test MOI.Utilities.is_canonical(MOI.get(bridged_mock, MOI.CanonicalConstraintFunction(), ci))
+
     @testset "$attr" for attr in [MOI.ConstraintPrimalStart(), MOI.ConstraintDualStart()]
         @test MOI.supports(bridged_mock, attr, typeof(ci))
         value = [√2, 1/√2, -1.0, -1.0]

test/Utilities/mockoptimizer.jl

@@ -136,3 +136,14 @@ end
     mock = MOIU.MockOptimizer(MOIU.Model{Float64}())
     MOIT.delete_test(mock)
 end
+
+@testset "CanonicalConstraintFunction" begin
+    mock = MOIU.MockOptimizer(MOIU.Model{Int}())
+    fx, fy = MOI.SingleVariable.(MOI.add_variables(mock, 2))
+    cx = MOI.add_constraint(mock, fx, MOI.LessThan(0))
+    c = MOI.add_constraint(mock, 1fx + fy, MOI.LessThan(1))
+    @test MOIU.is_canonical(MOI.get(mock, MOI.ConstraintFunction(), cx))
+    @test MOIU.is_canonical(MOI.get(mock, MOI.CanonicalConstraintFunction(), cx))
+    @test !MOIU.is_canonical(MOI.get(mock, MOI.ConstraintFunction(), c))
+    @test MOIU.is_canonical(MOI.get(mock, MOI.CanonicalConstraintFunction(), c))
+end

test/Utilities/model.jl

@@ -15,6 +15,7 @@ module TestExternalModel
     using MathOptInterface
     struct NewSet <: MathOptInterface.AbstractScalarSet end
     struct NewFunction <: MathOptInterface.AbstractScalarFunction end
+    MathOptInterface.Utilities.canonicalize!(f::NewFunction) = f
     Base.copy(::NewFunction) = NewFunction()
     Base.copy(::NewSet) = NewSet()
     MathOptInterface.Utilities.@model(ExternalModel,
@@ -174,12 +175,28 @@ end
     @test (MOI.VectorQuadraticFunction{Int},MOI.PositiveSemidefiniteConeTriangle) in loc
     @test (MOI.VectorQuadraticFunction{Int},MOI.PositiveSemidefiniteConeTriangle) in loc
 
-    f3 = MOIU.modify_function(f1, MOI.ScalarConstantChange(9))
-    f3 = MOIU.modify_function(f3, MOI.ScalarCoefficientChange(y, 2))
+    c3 = MOI.add_constraint(model, f1, MOI.Interval(-1, 1))
+
+    change_1 = MOI.ScalarConstantChange(9)
+    f3 = MOIU.modify_function(f1, change_1)
+    change_2 = MOI.ScalarCoefficientChange(y, 2)
+    f3 = MOIU.modify_function(f3, change_2)
 
     @test !(MOI.get(model, MOI.ConstraintFunction(), c1) ≈ f3)
+    @test !(MOI.get(model, MOI.ConstraintFunction(), c3) ≈ f3)
     MOI.set(model, MOI.ConstraintFunction(), c1, f3)
-    @test MOI.get(model, MOI.ConstraintFunction(), c1) ≈ f3
+    MOI.modify(model, c3, change_1)
+    MOI.modify(model, c3, change_2)
+    F1 = MOI.get(model, MOI.CanonicalConstraintFunction(), c1)
+    @test F1 ≈ f3
+    @test F1 !== MOI.get(model, MOI.ConstraintFunction(), c1)
+    @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 MOI.Utilities.is_canonical(F3)
+    @test MOI.get(model, MOI.CanonicalConstraintFunction(), c1) ≈ f3
+    @test MOI.Utilities.is_canonical(MOI.get(model, MOI.CanonicalConstraintFunction(), c1))
 
     f4 = MOI.VectorAffineFunction(MOI.VectorAffineTerm.([1, 1, 2], MOI.ScalarAffineTerm.([2, 4, 3], [x, y, y])), [5, 7])
     c4 = MOI.add_constraint(model, f4, MOI.SecondOrderCone(2))

test/Utilities/universalfallback.jl

@@ -177,6 +177,8 @@ end
 
         MOI.set(uf, MOI.ConstraintFunction(), cx, _affine(y))
         @test MOI.get(uf, MOI.ConstraintFunction(), cx) ≈ _affine(y)
+        @test MOI.get(uf, MOI.CanonicalConstraintFunction(), cx) ≈ _affine(y)
+        @test MOIU.is_canonical(MOI.get(uf, MOI.CanonicalConstraintFunction(), cx))
 
         @test MOI.supports(uf, MOI.ConstraintName(), typeof(cx))
         MOI.set(uf, MOI.ConstraintName(), cx, "LessThan")
@@ -200,6 +202,8 @@ end
 
         MOI.set(uf, MOI.ConstraintFunction(), cx, _affine(y))
         @test MOI.get(uf, MOI.ConstraintFunction(), cx) ≈ _affine(y)
+        @test MOI.get(uf, MOI.CanonicalConstraintFunction(), cx) ≈ _affine(y)
+        @test MOIU.is_canonical(MOI.get(uf, MOI.CanonicalConstraintFunction(), cx))
 
         @test MOI.supports(uf, MOI.ConstraintName(), typeof(cx))
         MOI.set(uf, MOI.ConstraintName(), cx, "EqualTo")