added missing matrix exponentiation methods #29782
Conversation
base/mathconstants.jl
Outdated
| @@ -89,6 +89,7 @@ catalan | |||
| # loop over types to prevent ambiguities for ^(::Number, x) | |||
| for T in (AbstractIrrational, Rational, Integer, Number, Complex) | |||
| Base.:^(::Irrational{:ℯ}, x::T) = exp(x) | |||
| Base.:^(::Irrational{:ℯ}, x::AbstractMatrix{T}) = exp(x) | |||
There was a problem hiding this comment.
The loop says it's for ambiguities. Shouldn't we instead have a general
Base.:^(::Irrational{:ℯ}, x) = exp(x) fallback?
There was a problem hiding this comment.
I don't know, wouldn't that still allow for the ambiguities of ^(::Number, x)? I would have thought it can create an ambiguity if we have the fallback, even if we have the specific methods.
There was a problem hiding this comment.
In that case, maybe just add AbstractMatrix to the thing that T loops over?
There was a problem hiding this comment.
This should be defined in LinearAlgebra.
There was a problem hiding this comment.
I didn't want to add AbstractMatrix as T to the loop because I thought it would be safer (in regard to method ambiguities) to ensure that only matrices with these element types have exponentials defined for them. Having said that, I just realized that this should have read AbstractMatrix{<:T} rather than what I have here.
Let me know whether you think this should indeed be fore general AbstractMatrix and I'll move it to LinearAlgebra.
There was a problem hiding this comment.
Since exp(::AbstractMatrix) is defined in LinearAlgebra this should too.
|
Ok, wasn't quite sure where to put this. Most of the methods are in By the way, I don't see a The tests seem a bit awkward where I put them, but I didn't know where else they would go. |
| @@ -340,6 +340,9 @@ rdiv!(A, B) | |||
| copy_oftype(A::AbstractArray{T}, ::Type{T}) where {T} = copy(A) | |||
| copy_oftype(A::AbstractArray{T,N}, ::Type{S}) where {T,N,S} = convert(AbstractArray{S,N}, A) | |||
|
|
|||
| # matrix exponentials for ℯ | |||
| Base.:^(::Irrational{:ℯ}, A::AbstractMatrix) = exp(A) | |||
There was a problem hiding this comment.
I would move this just below
julia/stdlib/LinearAlgebra/src/dense.jl
Line 490 in 84024a1
There was a problem hiding this comment.
are you sure about that? the reason I didn't put it in there is because it is only for dense, it could work for sparse array types as well.
There was a problem hiding this comment.
But it doesn't, this is the generic exp so I would put it here.
stdlib/LinearAlgebra/test/dense.jl
Outdated
| @@ -401,6 +401,10 @@ end | |||
| [4.000000000000000 -1.414213562373094 -1.414213562373095 | |||
| -1.414213562373095 4.999999999999996 -0.000000000000000 | |||
| 0 -0.000000000000002 3.000000000000000]) | |||
|
|
|||
| # alias using ℯ, should be exact | |||
| @test ℯ^(fill(2, (4,4))) == exp(fill(2, (4,4))) | |||
There was a problem hiding this comment.
I would also move these test out the loop over the elty, maybe to a separate sub-testset.
|
I've had something similar on my list and was actually planning to open a PR for this just now. 😄 I think it would be really nice for consistency to also have |
|
Yeah, good point, I don't know how I forgot about the general method (i.e. for Ok, the tests I wrote yesterday were awful, I've cleaned them up a bit so that they test more eltypes and they are in their own little section. Are you really sure you want these methods in |
|
Not sure what error travis had on MacOS. I don't know if it's just bad luck, but I feel like every time I make a PR I wind up with bogus failures on MacOS. |
|
Is any action needed here? I'm guessing this is probably not a real error on MacOS (as this has happened multiple times before and linux passes), can we re-trigger somehow? |
|
Seems to be passing now? |
|
Ok, I don't know what the deal was with that thing. The MacOS seems to have had bogus failures every time I've ever made a PR. |
stdlib/LinearAlgebra/src/dense.jl
Outdated
| @@ -489,6 +489,10 @@ julia> exp(A) | |||
| exp(A::StridedMatrix{<:BlasFloat}) = exp!(copy(A)) | |||
| exp(A::StridedMatrix{<:Union{Integer,Complex{<:Integer}}}) = exp!(float.(A)) | |||
|
|
|||
| Base.:^(b::Number, A::AbstractMatrix) = exp(log(b)*A) | |||
There was a problem hiding this comment.
This could actually call exp!(log(b)*A) since the multiplication returns a new array.
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>
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>
Fixes #29729.
I added some tests for existing methods
exp.expis missing methods forRationalandIrrational; I put in commented-out tests for those. I'm not sure if inserting speculative tests like this is an acceptable practice, I can take out those comments if not.