address comments

Author: joaquimg

src/bridges.jl

@@ -115,75 +115,85 @@ _square_offset(::MOI.AbstractSymmetricMatrixSetSquare) = 0
 _square_offset(::MOI.RootDetConeSquare) = 1
 _square_offset(::MOI.LogDetConeSquare) = 2
 
-function _square_to_triangle_indices(
-    bridge::MOI.Bridges.Constraint.SquareBridge,
-)
-    s = bridge.square_set
-    dim = MOI.side_dimension(s)
-    offset = _square_offset(s)
-    upper_triangle_indices = collect(1:offset)
-    sizehint!(upper_triangle_indices, offset + div(dim * (dim + 1), 2))
-    k = offset
-    for j in 1:dim
-        for i in 1:j
-            k += 1
-            push!(upper_triangle_indices, k)
-        end
-        k += dim - j
-    end
-    return upper_triangle_indices
-end
-
-"""
-    _triangle_to_square_scalars(tri_scalars, s)
-
-Expand triangle-vectorized scalars to square column-major form, mirroring
-off-diagonal entries. `s` is the square set (e.g. `PositiveSemidefiniteConeSquare`).
-"""
-function _triangle_to_square_scalars(tri_scalars, s)
-    dim = MOI.side_dimension(s)
-    offset = _square_offset(s)
-    square_dim = offset + dim * dim
-    square = Vector{eltype(tri_scalars)}(undef, square_dim)
-    for i in 1:offset
-        square[i] = tri_scalars[i]
-    end
-    tri_k = offset
-    for j in 1:dim
-        for i in 1:j
-            tri_k += 1
-            ij = offset + i + (j - 1) * dim
-            square[ij] = tri_scalars[tri_k]
-            if i != j
-                ji = offset + j + (i - 1) * dim
-                square[ji] = tri_scalars[tri_k]
-            end
-        end
-    end
-    return square
-end
-
+# Similar to `MOI.set` for `MOI.ConstraintPrimalStart` on `SquareBridge` in
+# MathOptInterface/src/Bridges/Constraint/bridges/SquareBridge.jl
 function MOI.set(
     model::MOI.ModelLike,
     attr::DiffOpt.ForwardConstraintFunction,
     bridge::MOI.Bridges.Constraint.SquareBridge{T},
     func::MOI.VectorAffineFunction{T},
 ) where {T}
-    indices = _square_to_triangle_indices(bridge)
-    tri_func = MOI.Utilities.eachscalar(func)[indices]
-    return MOI.set(model, attr, bridge.triangle, tri_func)
+    dim = MOI.side_dimension(bridge.square_set)
+    offset = _square_offset(bridge.square_set)
+    scalars = MOI.Utilities.eachscalar(func)
+    tri_scalars =
+        Vector{eltype(scalars)}(undef, offset + div(dim * (dim + 1), 2))
+    for i in 1:offset
+        tri_scalars[i] = scalars[i]
+    end
+    k = offset
+    for j in 1:dim, i in 1:j
+        k += 1
+        tri_scalars[k] = scalars[offset+j+(i-1)*dim]
+    end
+    MOI.set(
+        model,
+        attr,
+        bridge.triangle,
+        MOI.Utilities.operate(vcat, T, tri_scalars...),
+    )
+    for ((i, j), ci) in bridge.sym
+        f_ij = scalars[offset+i+(j-1)*dim]
+        f_ji = scalars[offset+j+(i-1)*dim]
+        MOI.set(model, attr, ci, MOI.Utilities.operate(-, T, f_ij, f_ji))
+    end
+    return
 end
 
+# Adjoint of `MOI.set` for `ForwardConstraintFunction` on `SquareBridge` above.
+# The forward map extracts upper triangle and sym diffs; this is its transpose.
+# Similar structure to `MOI.get` for `MOI.ConstraintPrimal` on `SquareBridge` in
+# MathOptInterface/src/Bridges/Constraint/bridges/SquareBridge.jl
 function MOI.get(
     model::MOI.ModelLike,
     attr::DiffOpt.ReverseConstraintFunction,
     bridge::MOI.Bridges.Constraint.SquareBridge{T},
 ) where {T}
-    tri_func_raw = MOI.get(model, attr, bridge.triangle)
-    tri_func = DiffOpt.standard_form(tri_func_raw)
-    tri_scalars = MOI.Utilities.eachscalar(tri_func)
-    square_scalars = _triangle_to_square_scalars(tri_scalars, bridge.square_set)
-    return MOI.Utilities.operate(vcat, T, square_scalars...)
+    tri_func = DiffOpt.standard_form(MOI.get(model, attr, bridge.triangle))
+    tri = MOI.Utilities.eachscalar(tri_func)
+    dim = MOI.side_dimension(bridge.square_set)
+    offset = _square_offset(bridge.square_set)
+    square = Vector{eltype(tri)}(undef, offset + dim^2)
+    for i in 1:offset
+        square[i] = tri[i]
+    end
+    k = offset
+    sym_index = 1
+    for j in 1:dim, i in 1:j
+        k += 1
+        upper_index = offset + i + (j - 1) * dim
+        lower_index = offset + j + (i - 1) * dim
+        if i == j
+            square[upper_index] = tri[k]
+        elseif sym_index <= length(bridge.sym) &&
+               bridge.sym[sym_index].first == (i, j)
+            π = DiffOpt.standard_form(
+                MOI.get(model, attr, bridge.sym[sym_index].second),
+            )
+            square[upper_index] = MOI.Utilities.operate(
+                +,
+                T,
+                MOI.Utilities.operate(+, T, tri[k], tri[k]),
+                π,
+            )
+            square[lower_index] = MOI.Utilities.operate(-, T, π)
+            sym_index += 1
+        else
+            square[upper_index] = tri[k]
+            square[lower_index] = tri[k]
+        end
+    end
+    return MOI.Utilities.operate(vcat, T, square...)
 end
 
 function _variable_to_index_map(bridge)

src/jump_wrapper.jl

@@ -234,99 +234,41 @@ function set_forward_constraint_function(
     },
     value::Number,
 ) where {M}
-    JuMP.check_belongs_to_model(con_ref, model)
-    return MOI.set(
-        JuMP.backend(model),
-        ForwardConstraintFunction(),
-        JuMP.index(con_ref),
-        JuMP.moi_function(JuMP.AffExpr(value)),
-    )
+    return set_forward_constraint_function(model, con_ref, JuMP.AffExpr(value))
 end
 
+# Similar to `JuMP.set_start_value` for vector `ConstraintRef` in
+# JuMP/src/constraints.jl
 function set_forward_constraint_function(
     model::JuMP.Model,
     con_ref::JuMP.ConstraintRef{
-        M,
+        <:JuMP.AbstractModel,
         <:MOI.ConstraintIndex{<:MOI.AbstractVectorFunction},
     },
     value::AbstractArray{<:JuMP.AbstractJuMPScalar},
-) where {M}
+)
     JuMP.check_belongs_to_model(con_ref, model)
     JuMP.check_belongs_to_model.(value, model)
+    v = JuMP.vectorize(value, con_ref.shape)
     return MOI.set(
         JuMP.backend(model),
         ForwardConstraintFunction(),
         JuMP.index(con_ref),
-        JuMP.moi_function(value),
-    )
-end
-
-function set_forward_constraint_function(
-    model::JuMP.Model,
-    con_ref::JuMP.ConstraintRef{
-        M,
-        <:MOI.ConstraintIndex{<:MOI.AbstractVectorFunction},
-    },
-    value::AbstractArray{<:Number},
-) where {M}
-    JuMP.check_belongs_to_model(con_ref, model)
-    return MOI.set(
-        JuMP.backend(model),
-        ForwardConstraintFunction(),
-        JuMP.index(con_ref),
-        JuMP.moi_function(JuMP.AffExpr.(value)),
+        JuMP.moi_function(v),
     )
 end
 
+# Similar to `JuMP.set_start_value` for vector `ConstraintRef` in
+# JuMP/src/constraints.jl
 function set_forward_constraint_function(
     model::JuMP.Model,
     con_ref::JuMP.ConstraintRef{
         <:JuMP.AbstractModel,
         <:MOI.ConstraintIndex{<:MOI.AbstractVectorFunction},
-        S,
     },
-    value::AbstractMatrix{<:Number},
-) where {S<:Union{JuMP.SquareMatrixShape,JuMP.SymmetricMatrixShape}}
-    if !LinearAlgebra.issymmetric(value)
-        error(
-            "ForwardConstraintFunction perturbation matrix must be " *
-            "symmetric for PSD cone constraints.",
-        )
-    end
-    JuMP.check_belongs_to_model(con_ref, model)
-    v = JuMP.vectorize(value, con_ref.shape)
-    func = JuMP.moi_function(JuMP.AffExpr.(v))
-    MOI.set(
-        JuMP.backend(model),
-        ForwardConstraintFunction(),
-        JuMP.index(con_ref),
-        func,
-    )
-    return
-end
-
-function set_forward_constraint_function(
-    model::JuMP.Model,
-    con_ref::JuMP.ConstraintRef{<:JuMP.AbstractModel,<:MOI.ConstraintIndex,S},
-    value::AbstractMatrix{<:JuMP.AbstractJuMPScalar},
-) where {S<:Union{JuMP.SquareMatrixShape,JuMP.SymmetricMatrixShape}}
-    if !LinearAlgebra.issymmetric(value)
-        error(
-            "ForwardConstraintFunction perturbation matrix must be " *
-            "symmetric for PSD cone constraints.",
-        )
-    end
-    JuMP.check_belongs_to_model(con_ref, model)
-    JuMP.check_belongs_to_model.(value, model)
-    v = JuMP.vectorize(value, con_ref.shape)
-    func = JuMP.moi_function(v)
-    MOI.set(
-        JuMP.backend(model),
-        ForwardConstraintFunction(),
-        JuMP.index(con_ref),
-        func,
-    )
-    return
+    value::AbstractArray{<:Number},
+)
+    return set_forward_constraint_function(model, con_ref, JuMP.AffExpr.(value))
 end
 
 """

test/bridges.jl

@@ -104,77 +104,6 @@ function test_dU_from_dQ()
     return _test_dU_dQ(U, dU)
 end
 
-function _make_square_bridge(dim)
-    s = MOI.PositiveSemidefiniteConeSquare(dim)
-    return MOI.Bridges.Constraint.SquareBridge{
-        Float64,
-        MOI.VectorAffineFunction{Float64},
-        MOI.ScalarAffineFunction{Float64},
-        MOI.PositiveSemidefiniteConeTriangle,
-        MOI.PositiveSemidefiniteConeSquare,
-    }(
-        s,
-        MOI.ConstraintIndex{
-            MOI.VectorAffineFunction{Float64},
-            MOI.PositiveSemidefiniteConeTriangle,
-        }(
-            1,
-        ),
-        Pair{
-            Tuple{Int,Int},
-            MOI.ConstraintIndex{
-                MOI.ScalarAffineFunction{Float64},
-                MOI.EqualTo{Float64},
-            },
-        }[],
-    )
-end
-
-function test_square_to_triangle_indices()
-    # 2x2: square col-major [a11, a21, a12, a22] → upper tri [a11, a12, a22]
-    @test DiffOpt._square_to_triangle_indices(_make_square_bridge(2)) ==
-          [1, 3, 4]
-    # 3x3: square col-major [a11,a21,a31, a12,a22,a32, a13,a23,a33]
-    #   → upper tri [a11, a12,a22, a13,a23,a33] at indices [1, 4,5, 7,8,9]
-    @test DiffOpt._square_to_triangle_indices(_make_square_bridge(3)) ==
-          [1, 4, 5, 7, 8, 9]
-    # 1x1: trivial
-    @test DiffOpt._square_to_triangle_indices(_make_square_bridge(1)) == [1]
-end
-
-function _make_rootdet_square_bridge(dim)
-    s = MOI.RootDetConeSquare(dim)
-    return MOI.Bridges.Constraint.SquareBridge{
-        Float64,
-        MOI.VectorAffineFunction{Float64},
-        MOI.ScalarAffineFunction{Float64},
-        MOI.RootDetConeTriangle,
-        MOI.RootDetConeSquare,
-    }(
-        s,
-        MOI.ConstraintIndex{
-            MOI.VectorAffineFunction{Float64},
-            MOI.RootDetConeTriangle,
-        }(
-            1,
-        ),
-        Pair{
-            Tuple{Int,Int},
-            MOI.ConstraintIndex{
-                MOI.ScalarAffineFunction{Float64},
-                MOI.EqualTo{Float64},
-            },
-        }[],
-    )
-end
-
-function test_square_to_triangle_indices_with_offset()
-    # RootDetConeSquare(2) has 1 offset entry (the rootdet variable t)
-    # Full square: [t, a11, a21, a12, a22] → triangle: [t, a11, a12, a22]
-    bridge = _make_rootdet_square_bridge(2)
-    @test DiffOpt._square_to_triangle_indices(bridge) == [1, 2, 4, 5]
-end
-
 function test_square_offset()
     @test DiffOpt._square_offset(MOI.PositiveSemidefiniteConeSquare(2)) == 0
     @test DiffOpt._square_offset(MOI.RootDetConeSquare(2)) == 1
@@ -187,27 +116,6 @@ function test_square_offset()
           [1, 2]
 end
 
-function test_triangle_to_square_scalars()
-    # 2x2 PSD: triangle [a11, a12, a22] → square [a11, a12, a12, a22]
-    s = MOI.PositiveSemidefiniteConeSquare(2)
-    @test DiffOpt._triangle_to_square_scalars([1, 2, 3], s) == [1, 2, 2, 3]
-    # 3x3 PSD: triangle [a11, a12, a22, a13, a23, a33]
-    #   → square col-major [a11, a12, a13, a12, a22, a23, a13, a23, a33]
-    s3 = MOI.PositiveSemidefiniteConeSquare(3)
-    @test DiffOpt._triangle_to_square_scalars([1, 2, 3, 4, 5, 6], s3) ==
-          [1, 2, 4, 2, 3, 5, 4, 5, 6]
-    # RootDetConeSquare(2): triangle [t, a11, a12, a22]
-    #   → square [t, a11, a12, a12, a22]
-    sr = MOI.RootDetConeSquare(2)
-    @test DiffOpt._triangle_to_square_scalars([10, 1, 2, 3], sr) ==
-          [10, 1, 2, 2, 3]
-    # LogDetConeSquare(2): triangle [u, t, a11, a12, a22]
-    #   → square [u, t, a11, a12, a12, a22]
-    sl = MOI.LogDetConeSquare(2)
-    @test DiffOpt._triangle_to_square_scalars([10, 20, 1, 2, 3], sl) ==
-          [10, 20, 1, 2, 2, 3]
-end
-
 end
 
 TestBridges.runtests()