-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
sqrtm is not type-stable #4006
Comments
This is also the case with sqrt. This is what dynamic languages are meant for. The types also look fine. We can keep it the way it is IMO. |
That's not how
|
|
Oops, |
Ok. This behavior seems undesirable to me, but I suppose this is traditional enough that I should just learn to accept it. I would have thought we'd want behavior like |
I agree that this does not seem great from the type-stability perspective. |
One option would be to return a real matrix when the input is a matrix of |
The underlying issue is that special symmetries of the matrix (Hermitian, real symmetric etc) allow for special-cased algorithms that are significantly faster than the generic algorithms. Functions like If a user wants to explicitly cast the return into a |
It might be nice to mention the type instability issue in the documentation for |
Yeah, we should document this. This behavior isn't specific to sqrtm however; rather it is also a feature of other functions, most notably linear solve and eig. Reopening as documentation issue. |
We should really be documenting the return type of every function. |
My earlier analysis of the
A matrix that is neither Hermitian nor symmetric will in general have a square root with complex elements. A matrix that is Hermitian or symmetric will also have a square root with complex elements unless it is positive definite, in which case we can specialize to the case of square root with real elements. Since you can determine whether a Hermitian/symmetric matrix is positive definite essentially for free after computing its eigenvalues, and recomposing a real matrix as a product of real factors is cheaper than in the complex case, I still think that the current polyalgorithm is optimally performant. The current implementation of the individual components, however, suggests some beneficial refactoring. For example:
|
- Reimplement expm and sqrtm (ref #4006)
In general yes, but e.g. the special case of real matrices with no eigenvalues on the non-positive real axis (positive and conjugate pairs are fine) have real square roots. I wrote about some of the peculiarities of the matrix square root in a mailing list thread about sqrtm when it was implemented:
I did write a Julia implementation of Higham's real square root method (starting from the real Schur decomposition instead of the complex Schur decomposition) but it's really not worth the code complexity. |
@GunnarFarneback thanks for the back history. I'll have to think for a bit about how easy it would be to check for the special case you mentioned in the triangular sqrtm routine. The mailing list discussion belies a much more complicated issue, namely that of defining a principal square root. It bears repeating that matrices don't have unique square roots. I think most people would expect matrix-valued functions to produce the "canonical" version that is implemented, which is to factorize, square root, and rotate back to the original basis. But perhaps this is worth mentioning in the docs. |
…y reflectors. Add Float16 and BigFloat to tests and test promotion. Cleaup promotion in LinAlg. Avoid promotion in mutating ! functions. Make Symmetric, Hermitian and QRs immutable. Make thin a keyword argument in svdfact. Remove cond argument in sqrtm.
Maybe we should change
sqrtm
so that it always returns a complex array?The text was updated successfully, but these errors were encountered: