-
-
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
RFC:Add generic Cholesky decomposition and make Cholesky parametric on matrix type #7236
Conversation
👍 |
Nice! |
I think it is ready for review now so also time for some advertising. The Cholesky is generic both in the sense that you can use e.g.
and in the sense that you can factorise block matrices
because it isn't assumed that the elements commute under multiplication and therefore it also works for quaternions
|
The block matrix stuff is really cool. Perhaps it can be leveraged for a multi threaded implementation. |
I would like to try something like that. |
I heard multithreading, so I have to link #6741 ;-) |
|
||
function getindex{T,S,UpLo}(C::Cholesky{T,S,UpLo}, d::Symbol) | ||
d == :U && return Triangular(triu!(UpLo == d ? C.UL : C.UL'),:U) | ||
d == :L && return Triangular(tril!(UpLo == d ? C.UL : C.UL'),:L) |
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.
It occurs to me that if UpLo == d
, we don't need to triu!
/tril!
.
@tknopp We will need some ways to manage dependencies within threads. In the distributed case, we have |
…trix type plus some fixes. Define tril! and triu! for AbstractMatrix and use elements instead of type to construct zeros. Also use elements for constructing zeros in getindex(Triangular).
Rebased. I think this one is ready for merge, so I'll do so soon if no objections are made. |
👍 Also looking forward to the |
Add generic Cholesky decomposition and make Cholesky parametric on matrix type
This just broke quite a few packages, not sure if that was intentional? |
(also congratulations for having the first PR to break a nontrivial number of packages 🍰, I was wondering what would be the first one) |
And private code, too! But that's fine. |
Should probably add a NEWS.md entry if the breakage was intentional. |
It is possible, but needs to be done manually |
There are still some methods that need to be adjusted for this to work completely, but I think most of the functionality should be there.