[WIP] Add tensor factorizations through MatrixAlgebraKit #23
Conversation
| using Test: @test, @testset, @inferred | ||
| using TestExtras: @constinferred | ||
| using TensorAlgebra: contract, svd, tsvd, TensorAlgebra | ||
| using TensorAlgebra.MatrixAlgebraKit: truncrank |
There was a problem hiding this comment.
Just as a style preference, I prefer using MatrixAlgebraKit: truncrank. using TensorAlgebra.MatrixAlgebraKit implicitly assumes the dependency is part of the API of the package, which I think isn't a good habit to get into and also just gets confusing.
There was a problem hiding this comment.
To be honest, I added this as a placeholder while we settle on where the keyword arguments are supposed to be handled, since we should either import that into TensorAlgebra, or provide a different interface. I just wanted to run some tests in the meantime, but for sure will adapt this :)
|
Looks like a great start! At first glance, it definitely looks nicer than if we were to call out to LinearAlgebra.jl directly. Does this fix #21? |
src/factorizations.jl
Outdated
| as a linear map from the domain to the codomain indices. These can be specified either via | ||
| their labels, or directly through a `biperm`. | ||
| """ | ||
| function tsvd(A::AbstractArray, labels_A, labels_codomain, labels_domain; kwargs...) |
There was a problem hiding this comment.
Just a suggestion, but what if truncated SVD is implemented as an algorithm backend of a more general svd function (i.e. we have svd(::Algorithm"truncated", a::AbstractArray, ...) and svd(::Algorithm"untruncated", a::AbstractArray, ...))?
There was a problem hiding this comment.
I'm definitely also fine with that, behind the scenes this is already what is going on in MatrixAlgebraKit as well, see the definition here. It's just a matter of deciding on where you want to start intercepting the user-input and what you would like the keywords for that to be, and I'm happy to map it to whatever MatrixAlgebraKit has
There was a problem hiding this comment.
Yeah, that is definitely a bit subtle, since ITensor has its own syntax for truncation keyword arguments.
Maybe a strategy we can take for now is starting out by following whatever MatrixAlgebraKit.jl does (say TensorAlgebra.svd just forwards kwargs to MatrixAlgebraKit.svd), and then at the ITensor level we can map our truncation arguments like cutoff and maxdim to the TensorAlgebra.jl/MatrixAlgebraKit.jl names and conventions, and additionally choose TruncatedAlgorithm as the default SVD algorithm.
Not yet, since we still need to add the SparseArrays extension in MatrixAlgebraKit, but it does move the responsibility lower down the chain, which I think is fair in this case. |
No problem, sounds good, partially I just wanted to understand the plan for that kind of issue. |
2 participants
This PR adds a variety of factorizations by building on the work in MatrixAlgebraKit.jl.
To do