Increase test coverage for CTMRG contractions#358
Conversation
|
Aside from the conceptual question above about what we want the environment application to mean exactly in the presence of fermionic symmetries, there's three more things to be addressed here I think:
|
There was a problem hiding this comment.
Looking at this a little bit, the only thing that seems slightly off to me is to have to store both the rotated as well as the unrotated PEPS tensors. I understand why you want to add the rotated version, (since we want to optimize the repeated application of these operators, right?), but could we get away with not having to store the unrotated version then? Trying to look into this and it seems they are only needed in the contract projectors since that still goes through the enlarged corner construction, can we get rid of that one?
The main reason why I'm saying this is that if we have to construct the enlarged corners here anyways, it is actually way more efficient to then compute the action through applying those, since then there are no more permutations involved in the contraction and it becomes simple matmul.
No worries if the reply is that for now this is the simplest solution and we can improve from there, I mostly just wanted to see if this was on purpose.
There was a problem hiding this comment.
For HalfInfiniteEnvironment and FullInfiniteEnvironment, I decided to add in the rotated tensors because I didn't want to write different contractions for every direction and I didn't want to do the permute at every application. The reason that the un-rotated versions are still there, is that for EnlargedCorner there are actually different contractions for every direction, so these constructors take un-rotated tensors. I had some routines where I could reuse implementations involving EnlargedCorner when dealing with e.g. HalfInfiniteEnv by splitting it into parts, at which point I would have had to de-rotate the tensors again. This also felt silly, so then I just decided to keep them both.
We could make a decision to either always rotate or never rotate, but I just didn't feel like implementing all of the rotated contractions for things that are currently unused.
There was a problem hiding this comment.
I'm slightly confused by this, would it not just mean that in the EnlargedCorner you just have to always use the north direction since you rotated the tensors already? (I did not look at this in a lot of detail, so do correct me if I'm wrong).
As in, I would expect that if you end up rotating the peps sandwich, now the HalfInfiniteEnv is effectively always the north one, so it should somehow be possible to always use the north contraction schemes, since the peps sandwhich is the main thing that is not rotation invariant, structure-wise?
There was a problem hiding this comment.
Just considering the enlarged corners, I think you're right. However, combining this with any other object that needs to be indexed (like isometries of projectors) would then be a bit of a nightmare, since once you put in the rotated tensor and shift the direction the enlarged corner itself has no memory of its original orientation. Maybe this can also be handled, but I would rather not instantiate EnlargedCorners with rotated PEPS tensors for which their direction doesn't match the actual direction in the network. I'm afraid this will inevitably give space mismatches that will be very hard to debug at some point.
If you prefer to hold off on merging this until all sparse environments (EnlargedCorner, HalfInfiniteEnvironment and FullInfiniteEnvironment) are handled in the same way, either only storing rotated tensors or only storing unrotated tensors, then I'm fine with that. But probably that won't be sorted out until we start using the fully sparse decompositions, at which point we'll probably naturally make a choice.
2 participants
Adds coverage for all of the sparse CTMRG contractions that were previously untested, by directly comparing their results to the dense equivalents. Quite a few of these were broken or even missing, so I fixed things up where necessary and added the missing parts so that we can actually do everything we would need using sparse environments.
There's one part that's a bit unclear to me, in the application of a (half or full infinite) environment to a vector, which I think we plan to use for fully matrix-free sparse SVDs in the future. There I'm not sure if for the "right" action we need the contraction
tensorcontract(env, x)or the multiplicationenv * xinstead, and the same for the "left" action.I chose contraction for now since that was the easiest and there is currently no use case to check this on. But if we need multiplication we'll have to add some additional twists.If we want the use ofsvdsolveto correspond to ansvd_truncon the dense environment, then application should correspond to proper tensor composition, soenv * x. I updated the implementation and test accordingly.