Exponential#465
Conversation
lkdvos
left a comment
There was a problem hiding this comment.
We should probably discuss together how to best approach this, because I guess the question is if we want to implement the MatrixAlgebraKit functionality and interface directly on the tensors first, like we did for the factorizations.
I didn't even remember we have exp!, which is as far as I can tell defined in TensorKit itself, which means we could in principle indeed decide to use that for exp!((tau, t)), although my initial feeling seems to gravitate more towards just overloading the MatrixAlgebraKit versions.
That would then also at least expose the algorithm selection and in-place versions a little better?
|
I can definitely get behind just overloading the |
| end | ||
|
|
||
| # Default algorithm for exponential with Tuple | ||
| MAK.exponential!((τ, t)::Tuple{E, T}) where {E <: Number, T <: DiagonalTensorMap} = MAK.exponential!((τ, t), DefaultAlgorithm()) |
There was a problem hiding this comment.
This is a line that I'm a bit sceptical about. I don't know why this has to be included, since I think it should be covered by the functions above, but if I don't include this, the algorithm that is selected for DiagonalTensorMaps is the default (LinearAlgebra.exp!) instead of the desired DiagonalAlgorithm.
|
The diagonal case needed some extra care (see also the comment above). So if there's any suggestions on tests, that would be greatly appreciated. That would also illuminate whether there are any additional type restrictions needed in MatrixAlgebraKit (preferably before tagging the new version). |
| MAK.exponential!(t::Tuple{E, T}, ::DefaultAlgorithm) where {E <: Number, T <: Diagonal} = MAK.exponential!(t, DiagonalAlgorithm()) | ||
| MAK.exponential!(t::Tuple{E, T1}, out::T2, ::DefaultAlgorithm) where {E <: Number, T1 <: Diagonal, T2 <: Diagonal} = MAK.exponential!(t, out, DiagonalAlgorithm()) |
There was a problem hiding this comment.
These definitions should probably go into MatrixAlgebraKit, see also e.g. https://github.com/QuantumKitHub/MatrixAlgebraKit.jl/blob/11096c1667ca0c08cb15eaa7e8388a55a0abe24a/src/implementations/lq.jl#L89-L98
There was a problem hiding this comment.
I pushed this in QuantumKitHub/MatrixAlgebraKit.jl#253.
| MAK.exponential!(t::Tuple{E, T}, ::DefaultAlgorithm) where {E <: Number, T <: Diagonal} = MAK.exponential!(t, DiagonalAlgorithm()) | ||
| MAK.exponential!(t::Tuple{E, T1}, out::T2, ::DefaultAlgorithm) where {E <: Number, T1 <: Diagonal, T2 <: Diagonal} = MAK.exponential!(t, out, DiagonalAlgorithm()) | ||
|
|
||
| function MAK.default_algorithm(::typeof(exponential!), ::Type{Tuple{E, T}}; kwargs...) where {E <: Number, T} |
There was a problem hiding this comment.
| function MAK.default_algorithm(::typeof(exponential!), ::Type{Tuple{E, T}}; kwargs...) where {E <: Number, T} | |
| function MAK.default_algorithm(::typeof(exponential!), ::Type{Tuple{E, T}}; kwargs...) where {E <: Number, T <: AbstractTensorMap} |
Although I think it might be better to replace this in MAK: https://github.com/QuantumKitHub/MatrixAlgebraKit.jl/blob/11096c1667ca0c08cb15eaa7e8388a55a0abe24a/src/interface/exponential.jl#L37-L39
should just forward to the non-tuple case, which will then select the default.
There was a problem hiding this comment.
What is then your suggested change in MAK? Because there we don't specify anything (in both exponential!(t) as exponential!((tau,t)) ). Do you want to restrict it to A <: AbstractMatrix there?
There was a problem hiding this comment.
function default_algorithm(::typeof(exponential!), ::Type{A}; kwargs...) where {A}
return default_exponential_algorithm(A; kwargs...)
end
function default_algorithm(::typeof(exponential!), ::Tuple{A, B}; kwargs...) where {A, B}
return default_algorithm(exponential!, B; kwargs...)
end
function default_algorithm(::typeof(exponential!), ::Type{Tuple{A, B}}; kwargs...) where {A, B}
return default_algorithm(exponential!, B; kwargs...)
endSomething like this?
| error("Exponential of a tensor only exist when domain == codomain.") | ||
| for (c, b) in blocks(t) | ||
| copy!(b, LinearAlgebra.exp!(b)) | ||
| MatrixAlgebraKit.exponential!(b, b) |
There was a problem hiding this comment.
I think here we should probably replace the entire function, not just the blockwise one
There was a problem hiding this comment.
This is now outdated, since we deprecate exp! and just use exponential! instead.
| return t | ||
| end | ||
|
|
||
| function exp!((τ, t)::Tuple{Number, TensorMap}) |
There was a problem hiding this comment.
I wouldn't really introduce this new functionality into exp!, and just call exponential! directly if the scaled version is required.
Co-authored-by: Lukas Devos <ldevos98@gmail.com>
This changes the current way of calculating the exponential (using
LinearAlgebra) to the one recently implemented inMatrixAlgebraKit(see QuantumKitHub/MatrixAlgebraKit.jl#94). This will not yet work, since there is no new version of MatrixAlgebraKit tagged yet. I just wanted to start the discussion here already.I added both
exp!(t)andexp!(tau, t). I added the special case where t is real and tau is complex, whenexp!cannot overwrite the original tensor and instead a new one is allocated. Suggestions on how to best do this are definitely appreciated.I still need to add some tests. We can definitely add some tests on whether the output tensor has the expected
scalartype, and whetherexp!(copy(a)) == exp!((1.0, copy(a)), but suggestions on more nontrivial tests are also welcome.