Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

vexity(::IndexAtom) is inexact #603

Closed
blegat opened this issue Apr 23, 2024 · 3 comments · Fixed by #625
Closed

vexity(::IndexAtom) is inexact #603

blegat opened this issue Apr 23, 2024 · 3 comments · Fixed by #625

Comments

@blegat
Copy link
Member

blegat commented Apr 23, 2024

The vexity of the IndexAtom is incorrect:

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

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
Copy link
Collaborator

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
Copy link
Member

odow commented Apr 23, 2024

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

@odow odow changed the title vexity of getindex vexity of getindex is inexact Apr 23, 2024
@odow odow changed the title vexity of getindex is inexact vexity(::IndexAtom) is inexact Apr 23, 2024
@blegat
Copy link
Member Author

blegat commented Apr 26, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants