Allow bond-dependent truncation scheme in Simple Update

[sanderdemeyer]

This is a small PR that adds an anisotropic version of Simple Update, i.e. where we allow for different bond dimensions on the vertical and horizontal bonds. This amounts to changing the parameter in _su_bondx!. I also added a constructor for when both truncation schemes should be the same. Maybe this is also a good moment to add a default version of Simple Update, which was still a TODO.

[codecov]

Codecov Report

Attention: Patch coverage is 81.25000% with 9 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
src/algorithms/truncation/truncationschemes.jl	68.96%	9 Missing ⚠️

Files with missing lines	Coverage Δ
src/PEPSKit.jl	100.00% <ø> (ø)
src/algorithms/time_evolution/simpleupdate.jl	100.00% <100.00%> (ø)
src/algorithms/time_evolution/simpleupdate3site.jl	100.00% <100.00%> (ø)
src/algorithms/truncation/bond_truncation.jl	97.01% <100.00%> (ø)
src/algorithms/truncation/fullenv_truncation.jl	96.96% <100.00%> (ø)
src/algorithms/truncation/truncationschemes.jl	72.72% <68.96%> (-27.28%)	⬇️

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[lkdvos]

Definitely cool! I had a vague idea somewhere in my mind to attempt this with a new truncationscheme that is fully site- or direction dependent, since this is also something that could be useful for ctmrg. There, we already have some preprocessing of the trscheme too, to allow for fixed space truncation, we might be able to leverage a similar system here?

[sanderdemeyer]

So you would want to generalize the current truncation scheme in CTMRG to be something like a (4,Nr,Nc) array (or whatever shape and type we end up with)? This might indeed be very interesting. In its most general form we could allow even for different truncation scheme subtypes for different sites and directions, although some combinations might run into issues?

It would definitely be worthwhile to look at this. Just for some context, I added this because I want to look at a.o. stripe order in the Hubbard model, where I would want to use a different bond dimension for the direction along the stripe and orthogonal to it. Even though CTMRG can already handle this, a more general approach like this could indeed boost performance.

[lkdvos]

Yeah, that’s more or less the idea I had in mind, to create some kind of wrapper truncation scheme to add the site-dependent information.
There is some precendence here: https://github.com/QuantumKitHub/PEPSKit.jl/blob/c2a32f2f980cbc483dc63cdd66e2ba4a28cb5ec3/src/algorithms/truncation/truncationschemes.jl but obviously it could be expanded

[Yue-Zhengyuan]

For SU (or any time evolution algorithms that can be added later), we can make SimpleUpdate.trschemes an array (wrapped into a struct if you prefer) of the same size as SUWeight. (Well, now I realize I should have named it SUWeights, in plural form...)

For the square lattice, the size is then (2, N_row, N_col), and trschemes[1, r, c] is the truncation scheme for the x-bond [r, c] -- [r, c+1], and [2, r, c] is for the y-bond [r, c] -- [r-1, c].

Several convenient constructors can also be defined, to allow users to fill all entries with the same trscheme (this reduces to the old bond-independent truncation), or fill [1, :, :] with one and [2, :, :] with another (different trschemes for x and y bonds).

Then we need to define rotation/mirroring of this trschemes array, similar to how we defined them for SUWeight.

BTW, if you're willing to share more detail here: why do you want to manually control the truncation in the two directions? Using the same truncerr for all bonds seem to be a more natural approach, unless you're just trying to obtain some initial state with certain desired property, and optimize later with less bias.

[sanderdemeyer]

Currently, we define the Simple Update struct without any reference to (the unit cell of) the PEPS. This would change if we make this truncation scheme to have the same dimensions as the PEPS, which would induce breaking changes. Maybe we can avoid this, but I haven't found a solution yet. I would be okay with introducing breaking changes, but what is your opinion on this?

To give some more details than before: I am thinking about stripe order in the Hubbard model. In this case, it could make sense to have a larger bond dimension along the direction of the charge modulation, and a lower one along the uniform direction.

[lkdvos]

This can definitely be done in a non-breaking way, looking at CTMRG for inspiration. The clue is to define access functions that give the truncation strategy at a given site, which by default return the strategy just like before, but can be overloaded to return a specific one.

For example, you can see here that before each projection step we determine the truncation strategy based on some additional information that might be relevant:

PEPSKit.jl/src/algorithms/ctmrg/sequential.jl

Line 85 in 16956d5

trscheme = truncation_scheme(alg, env.edges[WEST, _prev(r, size(env, 2)), c])

As a side note, reiterating what @Yue-Zhengyuan said: while it is indeed reasonable to specify different dimensions for different sites/directions, a more fair truncation strategy is based on the truncation cutoff/error, which would automatically give you different dimensions.

[lkdvos]

I like the direction of this a lot, looks very versatile without having to alter a lot of code. I left a bunch of minor comments and questions that might be improvements or bring things more in line with the rest of the library in the meantime.

(Also, it might at this point make sense to add a using TensorKit: TruncationScheme somewhere instead of having to qualify that everywhere).

[sanderdemeyer]

Thanks for the comments of you both. I've now taken care of them, like commented above.

In mirror_antidiag, I've now added if-statement on the number of directions to discriminate between SU and CTMRG. I didn't want to add a way that works for both without checking explicitly, since that would be probably wrong when using e.g. a triangular CTMRG algorithm, without giving us any warnings.

As a small side remark: Since the internal function _cluster_truncate! now requires an Vector of truncation schemes, I also had to change a test case that used this function.

[sanderdemeyer]

@Yue-Zhengyuan This is now included. This was a good idea, since there were two small bugs (also for non-square unit cells). I added a test that is very similar to the test where 2-site and 3-site SU are compared. The reason I made it a separate test is because I had to set bipartite=false, and otherwise the case where bipartite=true would never be tested.

[Yue-Zhengyuan]

Do you mind if I combine the tests of SiteDependentTruncation into one file? I prefer this instead of separating them into different files.

[sanderdemeyer]

Will you then duplicate the test on the Heisenberg model, where we'll have one like it was before, and another one in the SiteDependentTruncation test file?

[Yue-Zhengyuan]

Yes. I prefer to feed to AD a state with uniform bond dimension in the old Heisenberg test. Then I create another test file solely dedicated to the test of SiteDependentTruncation, which only needs a few steps of SU.

[sanderdemeyer]

The amount of iterations in the current test of SiteDependentTruncation could indeed be much lower, so for me your proposal is fine. You can push directly to this branch to make your changes.

[Yue-Zhengyuan]

Apart from a few minor changes below, this PR is good to go for me.

[lkdvos]

Made some final clean-up changes I noticed, approved once tests pass.

Changes already applied.

[Yue-Zhengyuan]

I removed a test of SiteDependentTruncation with Hubbard SU, as it is essentially covered by test/timeevol/sitedep_truncation.jl.