Add support for Tuple functions with epigraph matrix sets #3427

odow · 2023-06-27T01:31:28Z

@blegat suggested this in #3424. Previously suggested by @matbesancon in jump-dev/MathOptInterface.jl#1624.

Preview: https://jump.dev/JuMP.jl/previews/PR3427/tutorials/conic/tips_and_tricks/#RootDetCone

TODO:

A nicer error message when a tuple passed instead of a vector

codecov · 2023-06-27T01:45:24Z

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.01% 🎉

Comparison is base (038be2e) 97.99% compared to head (2634e11) 98.01%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3427      +/-   ##
==========================================
+ Coverage   97.99%   98.01%   +0.01%     
==========================================
  Files          36       36              
  Lines        5043     5087      +44     
==========================================
+ Hits         4942     4986      +44     
  Misses        101      101

Files Changed	Coverage Δ
src/print.jl	`99.52% <100.00%> (+<0.01%)`	⬆️
src/sd.jl	`100.00% <100.00%> (ø)`
src/sets.jl	`100.00% <100.00%> (ø)`
src/shapes.jl	`100.00% <100.00%> (ø)`

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

odow · 2023-06-27T05:26:03Z

This certainly makes a lot of sense. It make the log/root det cones a lot nicer to use:

https://jump.dev/JuMP.jl/previews/PR3427/tutorials/conic/tips_and_tricks/#RootDetCone

But I worry about special casing the syntax to only a few cones.

You could wonder why the rest of the epigraph cones don't support the same syntax, or why cones with fixed input dimension like Exponential require a vector instead of a tuple.

We could add support one at a time, but I don't know if there's a generic way to support them all.

matbesancon · 2023-06-27T07:43:06Z

We could add support one at a time, but I don't know if there's a generic way to support them all.

I would say this is fairly natural for all epigraph-type of cones

src/sd.jl

test/test_constraint.jl

odow · 2023-06-28T01:59:40Z

test/test_constraint.jl

+    end
+    for set in
+        (MOI.SecondOrderCone(4), SecondOrderCone(), MOI.GeometricMeanCone(4))
+        c = @constraint(model, (t, x) in set)


This is now a generic implementation that works for arbitrary cones. If it has a downside, it's that it's too permissive. Effectively, the tuple vectorized by concatenating the elements, and doesn't concern itself with whether the structure of the tuple makes sense for the set. So these three constraints are all valid:

@constraint(model, (t, x) in SecondOrderCone()) @constraint(model, (t, x[1], x[2], x[3]) in SecondOrderCone()) @constraint(model, (t, x[1], x[2:3]) in SecondOrderCone())

We don't have a way of saying that SecondOrderCone is an epigraph with Scalar,Vector, and that RotatedSecondOrderCone is a perspective epigraph with Scalar,Scalar,Vector. We just know the full dimension.

docs/src/tutorials/conic/tips_and_tricks.jl

blegat · 2023-06-29T06:36:10Z

This looks really nice, now what about @variable(model, (t, X[1:3,1:3]) in MOI.RootDetConeTriangle(3)), this will address your comment in jump-dev/MathOptInterface.jl#2198 (comment)

odow · 2023-06-29T09:37:20Z

This is a fairly big syntax change, so @mlubin better take a look.

odow · 2023-06-29T09:38:58Z

@variable(model, (t, X[1:3,1:3]) in MOI.RootDetConeTriangle(3))

This seems. Hard. It requires a massive change to how we currently do parsing:

julia> using JuMP

julia> model = Model()
A JuMP Model
Feasibility problem with:
Variables: 0
Model mode: AUTOMATIC
CachingOptimizer state: NO_OPTIMIZER
Solver name: No optimizer attached.

julia> @variable(model, (t, x[1:2]) in SecondOrderCone())
ERROR: LoadError: Expression (t, x[1:2]) cannot be used as a name.
Stacktrace:
 [1] error(s::String)
   @ Base ./error.jl:33
 [2] _get_name(c::Expr)
   @ JuMP.Containers ~/.julia/packages/JuMP/Gra9Y/src/Containers/macro.jl:14
 [3] var"@variable"(__source__::LineNumberNode, __module__::Module, args::Vararg{Any, N} where N)
   @ JuMP ~/.julia/packages/JuMP/Gra9Y/src/macros.jl:2619
in expression starting at REPL[96]:1

mlubin · 2023-06-30T01:48:03Z

docs/src/tutorials/conic/ellipse_approx.jl

@@ -109,7 +109,7 @@ m, n = size(S)
    [i in 1:m],
    S[i, :]' * Z * S[i, :] - 2 * S[i, :]' * z + s <= 1,
 )
-@constraint(model, [t; vec(Z)] in MOI.RootDetConeSquare(n))


We use the syntax in examples, but what about documenting it in the general case, along with its limitations?

Effectively, the tuple vectorized by concatenating the elements, and doesn't concern itself with whether the structure of the tuple makes sense for the set.

I guess my concern was more: is this a good idea and should we move forward with it. It adds two ways to write each constraint (vector or tuple) which might be confusing for users.

mlubin · 2023-06-30T01:58:29Z

I wonder if tuples are too magical and we should do something more explicit, e.g.,

@constraint(model, concat(t, x) in SecondOrderCone())
@constraint(model, concat(t, x[1], x[2], x[3]) in SecondOrderCone())
@variable(model, concat_define(t, X[1:3,1:3]) in MOI.RootDetConeTriangle(3))

concat_define is weird, but we're running up against the issue of @variable being weird syntax. We never resolved #692.

odow · 2023-06-30T02:10:51Z

I think the tuple syntax in @constraint is the most natural.

I'd be against concat. You can already use vcat

@constraint(model, vcat(t, x) in SecondOrderCone())
@constraint(model, vcat(t, x[1], x[2], x[3]) in SecondOrderCone())

I guess the only benefit would be that we could define concat(::Variable, ::Symmetric{Matrix}) which would return the triangle. But we could also define a upper_triangular which would achieve the same thing:

@constraint(model, vcat(t, upper_triangular(X)) in SecondOrderCone())

The @variable syntax is going to break regardless. We could parse the tuples at macro-expansion time. It's just a bit of work.

There'd also be some nasty edge-cases that aren't or the form (::Variable, ::Vector{Variable}). We probably want the first one only of these:

@variable(model, (t, w[1:2], x[1:2]) in MOI.RelativeEntropyCone(5))
@variable(model, (t, w, x) in MOI.RelativeEntropyCone(5))
@variable(model, (t, x) in MOI.RelativeEntropyCone(5))
@variable(model, (t, x[1:5]) in MOI.RelativeEntropyCone(5))

odow · 2023-08-15T22:54:39Z

I'd still like to push forward with this. I think it makes some of the conic examples a lot nicer.

odow · 2023-08-24T18:32:53Z

docs/src/tutorials/conic/experiment_design.jl

-    [t, 1, (E[i, j] for i in 1:q for j in 1:i)...] in MOI.LogDetConeTriangle(q)
-)
+E = LinearAlgebra.Symmetric(V * LinearAlgebra.diagm(0 => np ./ n) * V')
+@constraint(dOpt, (t, 1.0, E) in MOI.LogDetConeTriangle(q))


What is the error thrown if E is not symmetric

odow · 2023-08-24T18:39:47Z

docs/src/tutorials/conic/experiment_design.jl

-E = V * LinearAlgebra.diagm(0 => np ./ n) * V'
-@constraint(
-    dOpt,
-    [t, 1, (E[i, j] for i in 1:q for j in 1:i)...] in MOI.LogDetConeTriangle(q)


Maybe this PR is really just asking for a nicer way to vec a symmetric matrix

odow · 2023-08-25T03:18:08Z

Closing in favor of #3456

blegat reviewed Jun 27, 2023

View reviewed changes

src/sd.jl Show resolved Hide resolved

odow commented Jun 27, 2023

View reviewed changes

test/test_constraint.jl Outdated Show resolved Hide resolved

odow commented Jun 28, 2023

View reviewed changes

docs/src/tutorials/conic/tips_and_tricks.jl Show resolved Hide resolved

blegat approved these changes Jun 29, 2023

View reviewed changes

mlubin reviewed Jun 30, 2023

View reviewed changes

odow and others added 10 commits August 16, 2023 10:55

Add support for Tuple functions with epigraph matrix sets

8dabb03

Fix formatting

b96d633

Fix docs

01e8799

Add support for generic tuples

c2e1b7e

Update

33cb9c1

Fix ambiguity

6a51d01

Updates

f8cb7f2

Update

2ee4130

Update test/test_constraint.jl

ad140f3

Update tips_and_tricks.jl

2634e11

odow force-pushed the od/tuple-shape branch from eed6957 to 2634e11 Compare August 15, 2023 22:55

odow commented Aug 24, 2023

View reviewed changes

odow mentioned this pull request Aug 25, 2023

Add upper_triangle and docs for {Log,Root}DetCone{Triangle,Square} #3456

Merged

odow closed this Aug 25, 2023

odow deleted the od/tuple-shape branch August 25, 2023 03:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for Tuple functions with epigraph matrix sets #3427

Add support for Tuple functions with epigraph matrix sets #3427

odow commented Jun 27, 2023 •

edited

Loading

codecov bot commented Jun 27, 2023 •

edited

Loading

odow commented Jun 27, 2023

matbesancon commented Jun 27, 2023

odow Jun 28, 2023

blegat commented Jun 29, 2023

odow commented Jun 29, 2023

odow commented Jun 29, 2023

mlubin Jun 30, 2023

odow Jun 30, 2023

mlubin commented Jun 30, 2023

odow commented Jun 30, 2023

odow commented Aug 15, 2023

odow Aug 24, 2023

odow Aug 24, 2023

odow commented Aug 25, 2023

Add support for Tuple functions with epigraph matrix sets #3427

Add support for Tuple functions with epigraph matrix sets #3427

Conversation

odow commented Jun 27, 2023 • edited Loading

codecov bot commented Jun 27, 2023 • edited Loading

Codecov Report

odow commented Jun 27, 2023

matbesancon commented Jun 27, 2023

odow Jun 28, 2023

Choose a reason for hiding this comment

blegat commented Jun 29, 2023

odow commented Jun 29, 2023

odow commented Jun 29, 2023

mlubin Jun 30, 2023

Choose a reason for hiding this comment

odow Jun 30, 2023

Choose a reason for hiding this comment

mlubin commented Jun 30, 2023

odow commented Jun 30, 2023

odow commented Aug 15, 2023

odow Aug 24, 2023

Choose a reason for hiding this comment

odow Aug 24, 2023

Choose a reason for hiding this comment

odow commented Aug 25, 2023

odow commented Jun 27, 2023 •

edited

Loading

codecov bot commented Jun 27, 2023 •

edited

Loading