Skip to content
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

Unnecessary scalar indexing in logdet(C::Cholesky) #46124

Open
sharanry opened this issue Jul 21, 2022 · 3 comments
Open

Unnecessary scalar indexing in logdet(C::Cholesky) #46124

sharanry opened this issue Jul 21, 2022 · 3 comments
Labels

Comments

@sharanry
Copy link

Related: JuliaGaussianProcesses/AbstractGPs.jl#329
https://github.com/JuliaLang/julia/blob/master/stdlib/LinearAlgebra/src/cholesky.jl#L672

@sharanry
Copy link
Author

sharanry commented Jul 21, 2022

Can't we do sum(log, real.(diag(C.factors))) ?

@dkarrasch
Copy link
Member

Have you tried it with allowscalar(false) (or whatever the correct syntax is)? I don't think diag(C.factors) does anything else than scalar indexing and collecting the resulting vector. That would be worse in general, because it adds an allocation.

@dkarrasch dkarrasch added the domain:linear algebra Linear algebra label Jul 22, 2022
@stevengj
Copy link
Member

stevengj commented Jul 26, 2022

Seems like a minimal change is:

 @inbounds for i in diagind(C.factors)
    dd += log(real(C.factors[i]))
end

Probably that doesn't help with OP's case, however?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants