Ensure PSD operators are being optimized over proper space #600
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #600 +/- ##
======================================
Coverage 98.1% 98.1%
======================================
Files 162 162
Lines 3108 3111 +3
Branches 760 759 -1
======================================
+ Hits 3050 3053 +3
Misses 37 37
Partials 21 21 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
LGTM except for 1 minor comment where the if else conditional is dropped.
We'll wait for @vprusso's feedback before merging this.
| if is_real: | ||
| objective = cvxpy.Maximize(cvxpy.real(win)) | ||
| else: | ||
| objective = cvxpy.Maximize(win) | ||
| objective = cvxpy.Maximize(cvxpy.real(win)) |
There was a problem hiding this comment.
Is this change related to this comment?
There was a problem hiding this comment.
Yes - if both branches use cvxpy.real there there isn't really a need to have a conditional, right?
There was a problem hiding this comment.
Thanks for the clarification. When I made my previous comment, I was trying to understand why Vincent had to use the if else block. But I get it now.
This shouldn't be a blocker for merging the PR.
There was a problem hiding this comment.
LGTM! Thanks for the help, @purva-thakre !
I'll go ahead and assign you to the issue for tracking, and MWR, @golanor !
|
Congrats on the bounty @golanor ! |
Description
Fixes #431 - Replacing "PSD" with the Hermitian type, so we optimize over complex variables instead of real ones.
The "Symmetric Extension Hierarchy" tests currently do not pass.
Changes
PSDin CVXPY tohermitian=Trueand ensure the variables are positive.Checklist
rufffor errors related to code style and formatting.pytest.Sphinxbuild can be checked locally for any failures related to your PRlinkcheckto check for broken links in the documentationdoctestto verify the examples in the function docstrings work as expected.