-
Notifications
You must be signed in to change notification settings - Fork 89
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 an rrule for the Cholesky decomposition #44
Conversation
d = view(A, j, j) | ||
B = view(A, j+1:n, 1:j-1) | ||
c = view(A, j+1:n, j) | ||
end |
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.
This mighjt be one of the rare times i suggest wrapping the block in @views
Also didn't we just do out bounds check so we can now @inbounds
it.
(again rare time i am going to suggest doing that to a whole l block)
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.
The only problem with @views
is that AFAICT you aren't guaranteed to actually get a view, since it uses Base.maybeview
rather than replacing getindex
with view
.
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.
Isn't that ok though?
Or does this outright need a view
because it is going to mutate it?
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.
Not sure, I just wanted to make sure this was the same as Nabla's implementation.
This is a direct port of the code from Nabla.
This is a (mostly) direct port of the code from Nabla.