Fix FixedSpaceTruncation for simple update#360
Conversation
|
Just realized that |
sanderdemeyer
left a comment
sanderdemeyer
left a comment
There was a problem hiding this comment.
Apart from the 2 small comments above, this looks good to me.
lkdvos
left a comment
lkdvos
left a comment
There was a problem hiding this comment.
Left some minor comments, otherwise looks good to me!
| V = domain(state[r, c], (d == 1) ? EAST : NORTH) | ||
| isdual(V) && (V = flip(V)) |
There was a problem hiding this comment.
Could we use north_virtualspace and east_virtualspace here?
There was a problem hiding this comment.
I prefer to use domain here since for standard arrow direction they directly return non-dual spaces.
There was a problem hiding this comment.
Since you have the check here anyways, does that matter? It's really a lot easier to read what's going on with the full names
There was a problem hiding this comment.
Returning a dual space in the standard case makes construction of truncspace(V) below a little bit more involved.
There was a problem hiding this comment.
What I mean is that there is already the flip code, so it doesn't really change that much. If you really prefer it this way, that's fine by me, but I would prefer the other way ;)
|
@lkdvos Can't merge because of an expected |
3 participants
This PR mainly fixes
FixedSpaceTruncationfor Simple Update, which was broken after generalizing it for arbitrary MPO gates in #339. This introduces some breaking changes forSiteDependentTruncationas well:truncs[1, :, :]now refers to horizontal (instead of NORTH/vertical) truncations, following the convention ofSUWeight.Rotation is removed, since it is currently no longer used anywhere.For bipartite state tests, I refactored the function
_is_bipartitethat checks whether the input object matches the bipartite lattice structure.Finally, in SU for the ground state, I now also output the energy estimated with SUWeight (equivalently BPEnv). But convergence is still determined from the change in SUWeight (instead of energy).