-
-
Notifications
You must be signed in to change notification settings - Fork 398
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
Fix *(::AbstractJuMPScalar{<:Real}, ::Hermitian) to return Hermitian #3695
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3695 +/- ##
==========================================
- Coverage 98.36% 98.35% -0.02%
==========================================
Files 43 43
Lines 5699 5707 +8
==========================================
+ Hits 5606 5613 +7
- Misses 93 94 +1 ☔ View full report in Codecov by Sentry. |
Could you also do the Symmetric case? It's actually much easier because you don't need to test whether the scalar is real. |
Co-authored-by: Mateus Araújo <maltusan@gmail.com>
This one we can fix in MA: jump-dev/MutableArithmetics.jl#268 |
I've just realized that with this definition of multiplication you are generating hidden using JuMP
prob = Model()
@variable(prob,x)
m = x*LinearAlgebra.Hermitian(randn(2,2))
m+m |
Perhaps we should close this then. The currently behavior is correct. The new PR would introduce the risk of incorrect behavior due to bugs in Base. If JuMP generates a It's really hard to uniformly support all of LinearAlgebra, SparseArrays, MutableArithmetics, and JuMP. |
Or you could just return instead LinearAlgebra.Hermitian(Matrix(LinearAlgebra.Hermitian(B, c)),c) This gets rids of the undefs, so no bugs are possible, you keep the performance gain, and don't require the users to always rewrap everything in Hermitian. |
Thanks a lot for solving all of the problems I pointed out! I'm always amazed by how quick you and @blegat are. |
No problem. A lot of the problems are because you might be the only person actively using JuMP for complex Hermitian stuff... 😆 |
Well, one might say only, but I'd rather say first. 😉 There are also these guys, but they invented their own reformulation of the complex Hermitian PSD cone, so they are only using JuMP for the real one. |
You might recognize an author |
Well, I know some of them personally, but there is one that I'm sure you do. 😆 |
Closes #3694