[LinearAlgebra] Add isdiag/isposdef for Diagonal and UniformScaling by dkarrasch · Pull Request #29638 · JuliaLang/julia

dkarrasch · 2018-10-14T16:22:22Z

I found a tiny tiny gap, a noop test for diagonality of Diagonal matrices. It seems that all other matrix structure tests have specialized Diagonal-methods: istriu, istril, issymmetric, and isposdef.

Edit: Based on @mcognetta's suggestion, this now also includes isdiag and isposdef for UniformScalings (title changed accordingly).

mcognetta · 2018-10-14T17:05:32Z

There should probably be the same for UniformScaling, which currently errors:

julia> isdiag(UniformScaling(2))
ERROR: MethodError: no method matching isdiag(::UniformScaling{Int64})
Closest candidates are:
  isdiag(::AbstractArray{T,2} where T) at /home/mc/github/julia-stable/usr/share/julia/stdlib/v1.0/LinearAlgebra/src/generic.jl:1127
  isdiag(::Number) at /home/mc/github/julia-stable/usr/share/julia/stdlib/v1.0/LinearAlgebra/src/generic.jl:1128
Stacktrace:
 [1] top-level scope at none:0

julia> versioninfo()
Julia Version 1.0.0
Commit 5d4eaca0c9 (2018-08-08 20:58 UTC)

Edit: Also isposdef for UniformScaling. I can put these in another PR if that would be better.

dkarrasch · 2018-10-15T07:07:43Z

Thanks @mcognetta for your suggestions. I think I found the right spot to put those methods and went ahead to include them. I hope you don't mind.

mcognetta · 2018-10-15T07:08:53Z

stdlib/LinearAlgebra/src/uniformscaling.jl

 istril(::UniformScaling) = true
 issymmetric(::UniformScaling) = true
 ishermitian(J::UniformScaling) = isreal(J.λ)
+isposdef(J::UniformScaling) = J.λ > zero(J.λ)


This fails when J.λ is complex with imaginary part 0 (since there is no comparison operator).

You can simply do isposdef(J::UniformScaling) = isposdef(J.λ).

mcognetta · 2018-10-15T07:14:18Z

@dkarrasch No problem. I had two other tests that might be useful as well (for the case I mentioned in the comment).

    @test isposdef(UniformScaling(complex(1.0, 0.0)))
    @test !isposdef(UniformScaling(complex(1.0, 1.0)))

fredrikekre · 2018-10-15T07:18:14Z

stdlib/LinearAlgebra/test/uniformscaling.jl

    @test ishermitian(I)
    @test !ishermitian(UniformScaling(complex(1.0,1.0)))
+    @test isposdef(I)
+    @test !isposdef(-I)


Maybe test these for complex-valued UniformScalings as well?

Yes, I think that's what @mcognetta's suggestion above does, I believe. This is now included.

mcognetta · 2018-10-15T07:18:32Z

stdlib/LinearAlgebra/src/diagonal.jl

 real(D::Diagonal) = Diagonal(real(D.diag))
 imag(D::Diagonal) = Diagonal(imag(D.diag))

+isdiag(D::Diagonal) = true


Sorry, one more thing. This fails when we have a block diagonal matrix in which not all blocks are diagonal.

MWE:

julia> D=Diagonal([[1 0; 0 1], [1 0; 1 1]]) 2×2 Diagonal{Array{Int64,2},Array{Array{Int64,2},1}}: [1 0; 0 1] ⋅ ⋅ [1 0; 1 1] julia> isdiag(D) true

I believe it should be
isdiag(D::Diagonal) = all(isdiag, D.diag)
isdiag(D::Diagonal{<:Number}) = true

But I am not so sure on what it means to be a matrix of matrices so maybe @fredrikekre can correct me.

I see, I only had the Number case in mind. I'll fix this.

... and add a test.

dkarrasch · 2018-10-15T07:18:37Z

Cool, I'll include them. Thanks!

mcognetta · 2018-10-15T07:45:56Z

While you are at it, you might as well correct isposdef for diagonals. Right now it checks all(x -> x>0, D.diag) which fails for the same cases as before (complex and block diagonals). A better method is

isposdef(D::Diagonal) = all(isposdef, D.diag)

dkarrasch · 2018-10-15T08:01:54Z

That wasn't me. :-))) Indeed, isposdef(Diagonal([[1 0; 0 1], [1 0; 0 1]])) was failing for lack of isless.

I included it and will add tests.

StefanKarpinski · 2018-10-16T04:45:35Z

This is looking good. Is it ready to go from your perspective, @dkarrasch?

dkarrasch · 2018-10-16T05:36:06Z

Thanks. I think so, yes.

changes between Julia 1.0 and 1.1, including: - Custom .css-style for compat admonitions. - Information about compat annotations to CONTRIBUTING.md. - NEWS.md entry for PRs #30090, #30035, #30022, #29978, #29969, #29858, #29845, #29754, #29638, #29636, #29615, #29600, #29506, #29469, #29316, #29259, #29178, #29153, #29033, #28902, #28761, #28745, #28708, #28696, #29997, #28790, #29092, #29108, #29782 - Compat annotation for PRs #30090, #30013, #29978, #29890, #29858, #29827, #29754, #29679, #29636, #29623, #29600, #29440, #29316, #29259, #29178, #29157, #29153, #29033, #28902, #28878, #28761, #28708, #28156, #29733, #29670, #29997, #28790, #29092, #29108, #29782, #25278 - Documentation for broadcasting CartesianIndices (#30230). - Documentation for Base.julia_cmd(). - Documentation for colon constructor of CartesianIndices (#29440). - Documentation for ^(::Matrix, ::Number) and ^(::Number, ::Matrix). - Run NEWS-update.jl. Co-authored-by: Morten Piibeleht <morten.piibeleht@gmail.com> Co-authored-by: Fredrik Ekre <ekrefredrik@gmail.com>

Addition of NEWS and compat admonitions for important changes between Julia 1.0 and 1.1, including: - Custom .css-style for compat admonitions. - Information about compat annotations to CONTRIBUTING.md. - NEWS.md entry for PRs #30090, #30035, #30022, #29978, #29969, #29858, #29845, #29754, #29638, #29636, #29615, #29600, #29506, #29469, #29316, #29259, #29178, #29153, #29033, #28902, #28761, #28745, #28708, #28696, #29997, #28790, #29092, #29108, #29782 - Compat annotation for PRs #30090, #30013, #29978, #29890, #29858, #29827, #29754, #29679, #29636, #29623, #29600, #29440, #29316, #29259, #29178, #29157, #29153, #29033, #28902, #28878, #28761, #28708, #28156, #29733, #29670, #29997, #28790, #29092, #29108, #29782, #25278 - Documentation for broadcasting CartesianIndices (#30230). - Documentation for Base.julia_cmd(). - Documentation for colon constructor of CartesianIndices (#29440). - Documentation for ^(::Matrix, ::Number) and ^(::Number, ::Matrix). - Run NEWS-update.jl. Co-authored-by: Morten Piibeleht <morten.piibeleht@gmail.com> Co-authored-by: Fredrik Ekre <ekrefredrik@gmail.com>

dkarrasch added 2 commits October 14, 2018 18:14

add isdiag(::Diagonal)

6d1efa9

add test for isdiag(::Diagonal)

d281128

fredrikekre added the linear algebra Linear algebra label Oct 14, 2018

fredrikekre approved these changes Oct 14, 2018

View reviewed changes

dkarrasch added 2 commits October 15, 2018 09:03

add isdiag/isposdef(::UniformScaling)

79315b1

add tests for isdiag/isposdef(::UniformScaling)

c582998

dkarrasch changed the title ~~[LinearAlgebra/diagonal] Add (trivial) isdiag(::Diagonal)~~ [LinearAlgebra] Add (trivial) isdiag(::Diagonal) and isdiag/isposdef(::UniformScaling) Oct 15, 2018

mcognetta reviewed Oct 15, 2018

View reviewed changes

correct isposdef(::UniformScaling)

b53c3a5

fredrikekre reviewed Oct 15, 2018

View reviewed changes

mcognetta reviewed Oct 15, 2018

View reviewed changes

dkarrasch added 3 commits October 15, 2018 09:20

add more tests for isposdef(::UniformScaling)

aeb2b56

fix isdiag(::Diagonal) for non-Number eltypes

b614442

add isdiag tests for non-Number diagonal matrices

ffe7cf3

add isposdef(::Diagonal) for complex/non-number eltype

e68c778

add tests for isposdef(::Diagonal) w/ cmplx/non-number eltype

7dcd568

dkarrasch changed the title ~~[LinearAlgebra] Add (trivial) isdiag(::Diagonal) and isdiag/isposdef(::UniformScaling)~~ [LinearAlgebra] Add isdiag/isposdef for Diagonal and UniformScaling Oct 15, 2018

StefanKarpinski merged commit da5f1cc into JuliaLang:master Oct 16, 2018

fredrikekre added a commit that referenced this pull request Nov 30, 2018

News for #29638 (isdiag and isposdef for Diagonal and UniformScaling).

a82fe78

fredrikekre added a commit that referenced this pull request Nov 30, 2018

News for #29638 (isdiag and isposdef for Diagonal and UniformScaling).

98b79af

fredrikekre added a commit that referenced this pull request Dec 1, 2018

News for #29638 (isdiag and isposdef for Diagonal and UniformScaling).

30734f3

fredrikekre added a commit that referenced this pull request Dec 1, 2018

News for #29638 (isdiag and isposdef for Diagonal and UniformScaling).

c7ba929

fredrikekre added a commit that referenced this pull request Dec 3, 2018

News for #29638 (isdiag and isposdef for Diagonal and UniformScaling).

6f7c934

fredrikekre added a commit that referenced this pull request Dec 4, 2018

News for #29638 (isdiag and isposdef for Diagonal and UniformScaling).

41ed5d7

fredrikekre added a commit that referenced this pull request Dec 4, 2018

News for #29638 (isdiag and isposdef for Diagonal and UniformScaling).

6f4b83b

dkarrasch mentioned this pull request Mar 15, 2019

istril, istriu, diag, etc for block diagonal matrices JuliaLang/LinearAlgebra.jl#610

Open

Uh oh!

Conversation

dkarrasch commented Oct 14, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mcognetta commented Oct 14, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dkarrasch commented Oct 15, 2018

Uh oh!

mcognetta Oct 15, 2018

Choose a reason for hiding this comment

Uh oh!

mcognetta commented Oct 15, 2018

Uh oh!

fredrikekre Oct 15, 2018

Choose a reason for hiding this comment

Uh oh!

dkarrasch Oct 15, 2018

Choose a reason for hiding this comment

Uh oh!

mcognetta Oct 15, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dkarrasch Oct 15, 2018

Choose a reason for hiding this comment

Uh oh!

dkarrasch Oct 15, 2018

Choose a reason for hiding this comment

Uh oh!

dkarrasch commented Oct 15, 2018

Uh oh!

mcognetta commented Oct 15, 2018

Uh oh!

dkarrasch commented Oct 15, 2018

Uh oh!

StefanKarpinski commented Oct 16, 2018

Uh oh!

dkarrasch commented Oct 16, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

dkarrasch commented Oct 14, 2018 •

edited

Loading

mcognetta commented Oct 14, 2018 •

edited

Loading

mcognetta Oct 15, 2018 •

edited

Loading