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

Error in matrix^power if eigenvalues are complex #340

Closed
alyst opened this issue Jun 14, 2016 · 11 comments
Closed

Error in matrix^power if eigenvalues are complex #340

alyst opened this issue Jun 14, 2016 · 11 comments
Labels
bug Something isn't working

Comments

@alyst
Copy link

alyst commented Jun 14, 2016

The M^p code for the generic case (p::Number) assumes that eig() returns real eigenvalues that could be compared with 0. That gives an error if eigenvalues are complex. E.g. (julia 0.4.5):

> cplx_eigvals_mtx = [3.0 -2.0; 4.0 -1.0];

> eigvals(cplx_eigvals_mtx)
2-element Array{Complex{Float64},1}:
 1.0+2.0im
 1.0-2.0im

> cplx_eigvals_mtx^1.5
ERROR: MethodError: `isless` has no method matching isless(::Complex{Float64}, ::Int64)
Closest candidates are:
  isless(::AbstractFloat, ::Real)
  isless(::Real, ::Real)
  isless(::Char, ::Integer)
 in bitcache_lt at broadcast.jl:406
 in .< at broadcast.jl:422
 in ^ at linalg/dense.jl:180
@andreasnoack
Copy link
Member

If we revert this, I think we'd get rid of the error.

The matrix power function could use some love. I don't think it is advisable to compute the eigen decomposition in the nonsymmetric case. We should do Algorithm 1 in http://citeseerx.ist.psu.edu/viewdoc/download?doi=10.1.1.452.6407&rep=rep1&type=pdf which I think we are already doing for some of the other matrix function. That would also fix this issue.

@alyst
Copy link
Author

alyst commented Jun 14, 2016

@andreasnoack Yes, if we replace the line in question (dense.jl:180) by

eltype(v) <: Complex || (any(v.<0) && (v = complex(v)))

the error should go away.

But using "Algorithm 1" would be much better. IIUC, M^p should not contain complex entries if M.>0. It happens currently because of the numerical errors in eig().

@alyst
Copy link
Author

alyst commented Jun 14, 2016

@andreasnoack
Copy link
Member

@alyst It would be great if you could prepare a PR.

@alyst
Copy link
Author

alyst commented Jun 15, 2016

@andreasnoack There's already JuliaLang/julia#12584, which implements state-of-the-art Schur–Padé algorithm (http://citeseerx.ist.psu.edu/viewdoc/download?doi=10.1.1.297.2021&rep=rep1&type=pdf). I hope the author is around and that PR could be merged soon.

@andreasnoack
Copy link
Member

True. I'd forgotten that PR. Hopefully, @mfasi can be persuaded to finish it. That would be a big improvement over what we have now.

@GunnarFarneback
Copy link

I don't think it is advisable to compute the eigen decomposition in the nonsymmetric case.

I agree.

julia> A=[1 0;1 1]
2×2 Array{Int64,2}:
 1  0
 1  1

julia> A^7
2×2 Array{Int64,2}:
 1  0
 7  1

julia> A^7.00001
2×2 Array{Float64,2}:
 1.0  0.0
 0.0  1.0

julia> expm(7.00001 * logm(A))
2×2 Array{Float64,2}:
 1.0      0.0
 7.00001  1.0

@mfasi
Copy link

mfasi commented Jun 16, 2016

@alyst @andreasnoack I am still around and willing to work on the PR. I seem to recollect that the patch was ready to be merged, when I stopped working on it. I will rebase the branch and see what happens.

@alyst
Copy link
Author

alyst commented Jun 16, 2016

@mfasi Thanks!

@andreasnoack
Copy link
Member

@mfasi That would be really great.

alyst referenced this issue in alyst/Clustering.jl Sep 17, 2016
@JeffBezanson JeffBezanson added the bug Something isn't working label Mar 23, 2017
@stevengj
Copy link
Member

stevengj commented May 1, 2017

Fixed by JuliaLang/julia#21184.

@stevengj stevengj closed this as completed May 1, 2017
@KristofferC KristofferC transferred this issue from JuliaLang/julia Nov 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

7 participants