Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Add support for {Hermitian,Symmetric} in Zeros constraint #3281
Add support for {Hermitian,Symmetric} in Zeros constraint #3281
Changes from 2 commits
eb171ef
c6d3707
2332a57
c5f21d8
f5ae40d
839459b
c7bbaba
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We recently added (#3273) support for
vector == vector
. But it is less clear what to do formatrix == matrix
.It gets lowered to
X - A in Zeros()
, butX - A
is a matrix, andZeros()
is a vector-valued set, which triggers this error.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
orHermitian
matrices outside the context of SDP.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 inSymmetric
orHermitian
) for equality between matrices of such special structure"There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perfect, thanks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated the docs and the error message.