Define chol for UniformScaling's

[mschauer]

This came also up during the review by @tkelman of https://github.com/mschauer/Bridge.jl and is quite natural to define. There cannot be a CholFact because UniformScaling s are not AbstractArray s

[tkelman]

is this mutating?

[mschauer]

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

[andreasnoack]

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

julia/base/linalg/cholesky.jl

Line 75 in 71d7c16

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".

[tkelman]

Examples plural, no blank line after

[mschauer]

https://github.com/JuliaLang/julia/blob/master/base/linalg/cholesky.jl#L230

[tkelman]

there are pending pr's that are going to be changing all of the existing cases

[mschauer]

ok, then I only change that one instance in base/linalg/uniformscaling.jl

[tkelman]

yeah just change the one you're adding

[mschauer]

Hm, on a related but orthogonal note to this, chol(fill(3.*I, 3,3)) fails with
ERROR: MethodError: no method matching isreal(::UniformScaling{Int64})
and after defining Base.isreal(J::UniformScaling) = isreal(J.λ)) with

ERROR: StackOverflowError:
Stacktrace:
 [1] chol(::Hermitian{UniformScaling{Int64},Array{UniformScaling{Int64},2}}) at ./linalg/cholesky.jl:188 (repeats 80000 times)

[fredrikekre]

Since the implementation is in base/linalg/uniformscaling.jl (rather than base/linalg/cholesky.jl) I suggest that we move the test to test/linalg/uniformscaling.jl (instead of in test/linalg/cholesky.jl as it is currently).

[mschauer]

Travis: Timeout.

[mschauer]

I would not like to move the tests, they integrate nicely into the file. On the other hand, the implementation could well be in linalg/cholesky.jl but the linalg/uniformscaling.jl is later in compilation order so I would not know how to rearrange this.

[fredrikekre]

I think that something like

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

would integrate nicely into test/uniformscaling.jl. It is nice to keep the mirroring between implementation file and test file IMO :)

[mschauer]

Actually, I let myself be convinced here.

[mschauer]

Bump.

[kshyatt]

@fredrikekre are you happy with this as is? @tkelman ?

[fredrikekre]

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

[mschauer]

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.