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

Add method to isposdef! for Hermitian and Symmetric #21561

Merged
merged 1 commit into from
Apr 27, 2017

Conversation

fredrikekre
Copy link
Member

@fredrikekre fredrikekre commented Apr 26, 2017

Fix JuliaLang/LinearAlgebra.jl#422

Perhaps too late for 0.6, but here it is

@KristofferC
Copy link
Member

KristofferC commented Apr 26, 2017

Instead of A.data shouldn't it be something like triu(A.data) (or tril depending on the uplo char). So "garbage" on the unused side gets ignored?

Edit: Nvm, I think this is not an issue.

@andreasnoack
Copy link
Member

andreasnoack commented Apr 26, 2017

@KristofferC You probably concluded that already but xpotrf ignores the other triangle (and imaginary parts in the diagonal).

@kshyatt kshyatt added the linear algebra Linear algebra label Apr 26, 2017
@@ -129,6 +129,12 @@ let n=10
@test det(a + a.') ≈ det(Symmetric(a + a.', :U))
@test det(a + a.') ≈ det(Symmetric(a + a.', :L))

# isposdef
if eltya != BigFloat # isposdef not defined for Matrix{BigFloat}
@test isposdef(Symmetric(asym)) == isposdef(full(Symmetric(asym)))
Copy link
Member

Choose a reason for hiding this comment

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

Might be good to directly test the methods for the mutating version, isposdef!, that you've added here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Guess we should test both.

@fredrikekre
Copy link
Member Author

Forgot to exclude Int for the in place testing, now it should work.

@@ -264,6 +264,8 @@ det(A::Symmetric) = det(bkfact(A))
inv{T<:BlasFloat,S<:StridedMatrix}(A::Hermitian{T,S}) = Hermitian{T,S}(inv(bkfact(A)), A.uplo)
inv{T<:BlasFloat,S<:StridedMatrix}(A::Symmetric{T,S}) = Symmetric{T,S}(inv(bkfact(A)), A.uplo)

isposdef!(A::HermOrSym{<:BlasFloat, <:StridedMatrix}) = ishermitian(A) && LAPACK.potrf!(A.uplo, A.data)[2] == 0
Copy link
Member

Choose a reason for hiding this comment

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

Extraneous space prior to <:StridedMatrix?

@JeffBezanson JeffBezanson merged commit fdfec7b into JuliaLang:master Apr 27, 2017
@fredrikekre fredrikekre deleted the fe/isposdef branch April 27, 2017 16:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
linear algebra Linear algebra
Projects
None yet
Development

Successfully merging this pull request may close these issues.

isposdef not defined for symmetric matrices
7 participants