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

Define chol for UniformScaling's #22633

Merged
merged 2 commits into from
Jul 18, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions base/linalg/uniformscaling.jl
Original file line number Diff line number Diff line change
Expand Up @@ -313,3 +313,27 @@ function hvcat(rows::Tuple{Vararg{Int}}, A::Union{AbstractVecOrMat,UniformScalin
end
return hvcat(rows, promote_to_arrays(n,1, promote_to_array_type(A), A...)...)
end


## Cholesky
function _chol!(J::UniformScaling, uplo)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this mutating?

Copy link
Contributor Author

@mschauer mschauer Jun 30, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is following verbatim the chol(::Number) implementation.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is a trick to allow a recursive definition of chol. See

Akk, info = _chol!(A[k,k], UpperTriangular)
. The outer structures will typically be mutable but not the inner. I think it is fine because ! is not saying "will mutate", just "be aware, might mutate".

c, info = _chol!(J.λ, uplo)
UniformScaling(c), info
end

chol!(J::UniformScaling, uplo) = ((J, info) = _chol!(J, uplo); @assertposdef J info)
Copy link
Member

@fredrikekre fredrikekre Nov 9, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the motivation for this method? Should fall in the same category as the now deprecated chol(::Number) IIUC, see #24498

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The function chol! is a chainable modifying array function and should do the right thing to immutable arrays. If !-versions of chainable array functions are defined for immutable arrays, then f!(g!(h!(copy(A())))) is a good way to write efficient and generic code which works for immutables and mutables (here ! allows f, g, h to use the copy of A as workspace where appropriate). In the same fashion, copy pushes through immutables and is defined for UniformScalings.


"""
chol(J::UniformScaling) -> C

Compute the square root of a non-negative UniformScaling `J`.

# Examples
```jldoctest
julia> chol(16I)
UniformScaling{Float64}
4.0*I
```
"""
chol(J::UniformScaling, args...) = ((C, info) = _chol!(J, nothing); @assertposdef C info)

8 changes: 8 additions & 0 deletions test/linalg/uniformscaling.jl
Original file line number Diff line number Diff line change
Expand Up @@ -173,3 +173,11 @@ end
hvcat((2,1,2),B,2eye(3,3),eye(6,6),3eye(3,3),4eye(3,3))
end
end

@testset "chol" begin
for T in (Float64, Complex64, BigFloat, Int)
λ = T(4)
@test chol(λ*I) ≈ √λ*I
@test_throws LinAlg.PosDefException chol(-λ*I)
end
end