-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Fixes 2 issues with the Cholesky update/downdate tests #17199
Conversation
1. It was possible to get random matrices which would not be positive definite when down-dated (e.g. if I ran the block with `srand(1)`. I have no idea how this didn't become an issue more often. 2. Apparently `v*v'` isn't Hermitian when using the system BLAS on Power.
F = cholfact(AcA, uplo) | ||
@test LinAlg.lowrankupdate(F, v)[uplo] ≈ cholfact(AcA + v*v')[uplo] |
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.
Maybe cholfact(Hermitian(AcA + v*v'))
instead of cholfact(BcB)
.
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.
julia> cholfact(Hermitian(AcA + v*v'))
ERROR: ArgumentError: Cannot construct Hermitian from matrix with nonreal diagonals
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.
Arrrr... I'd forgotten that we still do that. That check should go away.
Ah, thanks, this is an issue on AArch64 (with OpenBLAS) too. |
FMAs seem to be the issue here. It makes complex multiplication non-commutative. |
Yikes. That's not good at all. Is there an issue for this? |
srand(1)
. I have no idea how this didn't become an issue more often.v*v'
isn't Hermitian when using the system BLAS on Power.cc: @andreasnoack @vtjnash