Implement sparse SVD with function handles #72
Conversation
Codecov ReportAttention: Patch coverage is
|
|
I think if you change the initial guess to be a slight deviation from the first singular vector, you'll have better performance. I think you need a small component orthogonal to that vector to start generating new vectors in the krylov subspaces. |
…rmalize_corner contraction
|
So in principle this should be a relatively complete implementation of "function handle CTMRG", however it's still untested and there are some things to discuss/fix:
Maybe we could discuss, review and fix these things next week or so @lkdvos. |
There was a problem hiding this comment.
I left some specific remarks in the code, but maybe let me summarize:
- We should probably be a little careful with trying to push too many things into the type domain. The runtime benefit is not obvious, as it would just compile away a couple if checks, and the compiletime will increase, as well as the code complexity.
- The main source of misery here seems to be that we should rethink a good interface for
svdsolvewith bothTensorMaps andFunctions. I will attempt to come up with something better (probably tomorrow, otherwise coming week)
As a more general remark, I do really like the sparse environment structs, and would probably structure the code to always construct them first, and then, depending on the algorithm, either convert to a dense environment TensorMap or not. This would probably (hopefully) organise a bit of the code a bit more, such that the responsability of dealing with these objects is centralized to these structs.
| TensorKit.InnerProductStyle(::HalfInfiniteEnv) = EuclideanProduct() | ||
| TensorKit.sectortype(::HalfInfiniteEnv) = Trivial | ||
| TensorKit.storagetype(env::HalfInfiniteEnv) = storagetype(env.ket_1) | ||
| TensorKit.spacetype(env::HalfInfiniteEnv) = spacetype(env.ket_1) | ||
|
|
||
| function TensorKit.domain(env::HalfInfiniteEnv) | ||
| return domain(env.E_4) * domain(env.ket_2)[3] * domain(env.bra_2)[3]' | ||
| end | ||
| function TensorKit.codomain(env::HalfInfiniteEnv) | ||
| return codomain(env.E_1)[1] * domain(env.ket_1)[3]' * domain(env.bra_1)[3] | ||
| end | ||
| function TensorKit.space(env::HalfInfiniteEnv) | ||
| return codomain(env) ← domain(env) | ||
| end | ||
| function TensorKit.blocks(env::HalfInfiniteEnv) | ||
| return TensorKit.SingletonDict(Trivial() => env) | ||
| end | ||
| function TensorKit.blocksectors(::HalfInfiniteEnv) | ||
| return TensorKit.OneOrNoneIterator{Trivial}(true, Trivial()) | ||
| end | ||
| function TensorKit.block(env::HalfInfiniteEnv, c::Sector) | ||
| return env | ||
| end | ||
| function TensorKit.tsvd!(f::HalfInfiniteEnv; trunc=NoTruncation(), p::Real=2, alg=IterSVD()) | ||
| return _tsvd!(f, alg, trunc, p) | ||
| end | ||
| function TensorKit.MatrixAlgebra.svd!(env::HalfInfiniteEnv, args...) | ||
| return TensorKit.MatrixAlgebra.svd!(env(), args...) # Construct environment densely as fallback | ||
| end |
There was a problem hiding this comment.
This somehow confuses me, I would not have guessed that if we dispatch to KrylovKit, we would need this TensorKit interop...
src/utility/svd.jl
Outdated
There was a problem hiding this comment.
I looked at these changes in a bit more detail, and I think we will probably have to commit to a better svd interface before we can make this work, as this definitely seems a bit unwieldy.
I think the main issue comes from the fact that we ended up merging the "compute pullback of svd block" step with the "merge pullbacks back into TensorMap form" step. I have some time on the flight tomorrow, I'll try and see if I cannot make this work a bit cleaner.
The idea would be something like: if we can have a stack-like function for TensorMap's, which is compatible with AD, it should be possible to use the KrylovKit rrule for the first step described above, while letting the AD engine combine the pullbacks automatically. This should then avoid this code-duplication, and make it more robust for function handles as well.
There was a problem hiding this comment.
Yes, I definitely agree. The way the interface is designed right now makes things tricky but there needs to be some compatibility layer that translates between the svdsolve calls and the block TensorMap structure that also needs to be generic enough to somehow work for function handles. I'd really welcome a better solution than what we have right now.
| @@ -0,0 +1,265 @@ | |||
| const SparseCTMRG = CTMRG{M,true} where {M} # TODO: Is this really a good way to dispatch on the sparse CTMRG methods? | |||
There was a problem hiding this comment.
I would probably put this as a regular field. I am starting to feel less and less happy with all our type parameters, as it makes things a lot less transparent...
There was a problem hiding this comment.
As a more general remark, I do really like the sparse environment structs, and would probably structure the code to always construct them first, and then, depending on the algorithm, either convert to a dense environment TensorMap or not. This would probably (hopefully) organise a bit of the code a bit more, such that the responsability of dealing with these objects is centralized to these structs.
When I was writing these struct I was also thinking that it would be more useful to center the CTMRG function around them. That would definitely organize the code a bit more naturally. I'll get to it next week!
|
Had to add some extra Edit: Is this related to the rrule of the |
|
Wow, only now I figured out that the problem with the iterative SVD and the Weirdly enough, the TensorKit |
|
This should be finished up to the function handle SVD aspects - but those we'll anyway implement later on. Once the KrylovKit/TensorKit SVD interface is implemented, the function handles should also be very easy to add. |
In this PR we will use function handles to perform the iterative SVD of the half-infinite environment in CTMRG. To that end, a struct will be implemented that stores all necessary tensors for the environment contraction and on which
tsvd!can dispatch.Before doing that, a long-standing issue will be resolved where
IterSVDwould lead to singular vectors which, when supplied toFixedSVD, would not lead to element-wise converged environments and thereby making it not possible to use the iterative SVD during PEPS optimization.