jump-dev / Convex.jl

A Julia package for disciplined convex programming
https://jump.dev/Convex.jl/stable/
Other
559 stars 119 forks source link

vexity(::IndexAtom) is inexact #603

Closed blegat closed 2 months ago

blegat commented 3 months ago

The vexity of the IndexAtom is incorrect:

julia> x = Variable()
Variable
size: (1, 1)
sign: real
vexity: affine
id: 176…085

julia> v = vcat(x, square(x))
reshape (convex; real)
└─ * (convex; real)
   ├─ 2×2 Matrix{Bool}
   └─ reshape (convex; real)
      └─ hcat (convex; real)
         ├─ …
         └─ …

julia> vexity(v)
Convex.ConvexVexity()

julia> vexity(v[1])
Convex.ConvexVexity()

julia> v = vcat(-square(x), square(x))
reshape (Convex.NotDcp; real)
└─ * (Convex.NotDcp; real)
   ├─ 2×2 Matrix{Bool}
   └─ reshape (Convex.NotDcp; real)
      └─ hcat (Convex.NotDcp; real)
         ├─ …
         └─ …

julia> vexity(v)
Convex.NotDcp()

julia> vexity(v[1])
Convex.NotDcp()
ericphanson commented 3 months ago

The current definition doesn’t depend on the index, it just defers to the object being indexed. I think it hasn’t come up before since usually all elements in the vector have the same vexity.

odow commented 3 months ago

This isn't incorrect so much as inexact: could be improved, but it won't give wrong answers.

blegat commented 2 months ago

We discussed this in the JuMP-dev call, the conclusion was that we should try adding a vcat atom and specialize getindex for the vcat atom to try and see if the indices fit in one of the argument in the vcat in which case we remove the vcat atom and propagage getindex