Add support for {Hermitian,Symmetric} in Zeros constraint #3281
Conversation
| "vector-valued set. Did you mean to use the broadcasting syntax " * | ||
| "`.==` instead?", | ||
| ), | ||
| @constraint(model, X == A), |
There was a problem hiding this comment.
We recently added (#3273) support for vector == vector. But it is less clear what to do for matrix == matrix.
It gets lowered to X - A in Zeros(), but X - A is a matrix, and Zeros() is a vector-valued set, which triggers this error.
There was a problem hiding this comment.
We could make the equality case work.
The inequality, it's better that it's an error since the user most probably forgot PSDCone()
There was a problem hiding this comment.
I think it wouldn't even be possible to support inequalities, as JuMP cannot know whether the user wants PSDCone() or HermitianPSDCone().
In any case, I concur, allowing inequalities leads to ambiguities. Maybe the user has a symmetric matrix, but wants to constrain it to be elementwise nonnegative.
There's also a bug that always happens with YALMIP: the user has a matrix they think is Hermitian but it's actually not because of numerical noise. Then the user writes >= 0 thinking it's constraining it to be PSD, but YALMIP instead interprets that as elementwise non-negativity.
There was a problem hiding this comment.
There's also a bug that always happens with YALMIP: the user has a matrix they think is Hermitian but it's actually not because of numerical noise. Then the user writes >= 0 thinking it's constraining it to be PSD, but YALMIP instead interprets that as elementwise non-negativity.
Indeed, we want to avoid that, an error asking the user to be explicit about it is better than trying to save a few keystrokes
There was a problem hiding this comment.
Let's merge this and see how we go re-feedback. The non-symmetric case X .== Y is pretty easy to type, and there's a nice error message.
The specialized support for symmetric and hermitian is more useful because of the redundant constraints.
Agree on inequalities.
There was a problem hiding this comment.
The case of equalities is not so clear-cut because the user intention is not ambiguous. If JuMP interprets == as .== in the non-symmetric case the result will be a slowdown, not a bug (assuming the user thought the matrix was Hermitian).
I would just change the error message to say "Please declare your matrix as Symmetric or Hermitian if it is the case. Otherwise use .=="
We can't really do that since JuMP is a general purpose solver so having equality between matrices is completely fine and it's weird to talk about Symmetric or Hermitian matrices outside the context of SDP.
There was a problem hiding this comment.
Fair enough, but the error message seems to imply that == is never supported for matrices, which is not the case.
There was a problem hiding this comment.
Good point, JuMP.Zeros is indeed not a vector-valued set (since it works with scalars, Symmetric and Hermitian). "Equality between $(typeof(matrix)). Use .== for elementwise inequality or wrap the matrix in Symmetric or Hermitian) for equality between matrices of such special structure"
There was a problem hiding this comment.
Updated the docs and the error message.
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #3281 +/- ##
=======================================
Coverage 98.11% 98.11%
=======================================
Files 34 34
Lines 4714 4732 +18
=======================================
+ Hits 4625 4643 +18
Misses 89 89
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
|
Let's also document it, e.g., in https://jump.dev/JuMP.jl/stable/manual/constraints/#Symmetry. Or rather in https://jump.dev/JuMP.jl/stable/manual/constraints/#Vectorized-constraints |
|
One should also update the complex tutorial to use the new syntax and mention that it's more efficient. |
x-ref #3257 (comment)