Skip to content

DMRG3S: strictly single-site DMRG subspace expansion (Hubig et al. 2015)#460

Open
VinceNeede wants to merge 19 commits into
QuantumKitHub:mainfrom
VinceNeede:vb-dmrg3s
Open

DMRG3S: strictly single-site DMRG subspace expansion (Hubig et al. 2015)#460
VinceNeede wants to merge 19 commits into
QuantumKitHub:mainfrom
VinceNeede:vb-dmrg3s

Conversation

@VinceNeede

@VinceNeede VinceNeede commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Summary

Implements DMRG3S — strictly single-site DMRG with subspace expansion — from Hubig, McCulloch, Schollwöck & Wolf, Phys. Rev. B 91, 155115 (2015). Unlike CBE (alg_expand), which enriches the bond before the local eigensolve, DMRG3S enriches it after, using a perturbation built from the just-optimized tensor, the local Hamiltonian, and the environment. This lets single-site DMRG escape local minima caused by missing quantum-number sectors that CBE-style pre-expansion can't reach.

This PR also targets TDVP, pending design agreement

DMRG only for now. TDVP support belongs in this same PR, but I'd like design agreement on the interface below first, since it directly shapes how it plugs into TDVP too. Docs are also on hold for the same reason — no point documenting an interface that might still change.

What's here

  • Refactored find_groundstate!'s left-to-right and right-to-left sweeps from two separate, near-duplicate loops into a single loop over direction in (:right, :left), dispatching on Val. No behavioral change; this just made room for the new expansion step without doubling its code.
  • alg_post_expand field on DMRG, dispatching a new post_expand! step after the eigensolve, alongside the existing pre-eigensolve alg_expand. Default (NoExpand()) reproduces current behavior exactly.
  • DMRG3S, with a composable NoiseSchedule for the mixing factor (ExponentialDecay, Warmup, for combining them).
  • alg_expand and alg_post_expand are independent and can be used together or separately.
  • A warning fires if either expansion mechanism is active without a truncating alg_gauge, since the bond would otherwise grow unboundedly each sweep.
  • Tests: a bond-growth smoke test alongside the existing CBE tests, and a reproduction of the paper's Sec. VII A example (plain single-site DMRG gets stuck at E ≈ -6.35, DMRG3S recovers E ≈ -8.68).

Open design questions for maintainers

  1. alg_post_expand sentinel: I used a dedicated NoExpand() type rather than nothing (which alg_expand uses). This lets NoExpand carry its own post_expand! method for dispatch, but it's an inconsistency with alg_expand's convention worth flagging.
  2. Expand+gauge+neighbor-pad are fused into one post_expand! call rather than kept separate, because splitting them left the MPS briefly inconsistent between steps (caught via a real bug during development — happy to share details). This does mean post_expand! needs alg_gauge threaded in as a parameter, coupling it to the gauge step more tightly than alg_expand is.

TODO before merge

  • Docs
  • TDVP support (Decided to add it in another PR)
  • Decide on Q1–Q2 above

@github-actions

github-actions Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Your PR no longer requires formatting changes. Thank you for your contribution!

@codecov

codecov Bot commented Jul 14, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 85.36585% with 12 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/algorithms/post_expand/dmrg3s.jl 88.13% 7 Missing ⚠️
src/algorithms/groundstate/dmrg.jl 75.00% 4 Missing ⚠️
src/utility/utility.jl 0.00% 1 Missing ⚠️
Files with missing lines Coverage Δ
src/MPSKit.jl 100.00% <ø> (ø)
src/algorithms/approximate/fvomps.jl 91.30% <100.00%> (+91.30%) ⬆️
src/algorithms/post_expand/post_expand.jl 100.00% <100.00%> (ø)
src/utility/utility.jl 58.68% <0.00%> (+27.91%) ⬆️
src/algorithms/groundstate/dmrg.jl 92.63% <75.00%> (+92.63%) ⬆️
src/algorithms/post_expand/dmrg3s.jl 88.13% <88.13%> (ø)

... and 79 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@lkdvos

lkdvos commented Jul 15, 2026

Copy link
Copy Markdown
Member

Thanks for this, I had a brief look and I think it already looks quite well.
I'll try and make some time for a proper review either tomorrow or the day after, but mostly I'm wondering if it might make sense to wait for #455, as that has a similar kind of refactor of the DMRG machinery to avoid duplication.

Considering the expansion algorithm itself - without having looked in a lot of detail so definitely feel free to tell me I'm wrong - I think it could be nice to avoid having both an alg_gauge and an alg_postexpand, and simply merge the two together. As always, naming things is really the hardest part, but it feels a bit clumsy to have the if-guard in the generic local step, and to have to pass 2 algorithms to the perturb! body.
Do you think it might be possible to simply put the alg_gauge inside of the DMRG3S, and then keep only one of these around that we dispatch on? I'm not sure how blasphemous it would be to call these both alg_gauge...
We might also try and discuss more carefully about the invalid state though, the FiniteMPS struct and the interaction with the environments is quite subtle, and it might still be that it is still fixable to separate the two cleanly as well.

For the TDVP implementation, I definitely agree to flush this one out first, and honestly it's probably easier to tackle and review that in a separate PR.

Considering your other question, I'm definitely happy to add the NoExpand, and since we are in the middle of a breaking cycle, I would be happy to change the alg_expand to use that too.

@lkdvos
lkdvos marked this pull request as ready for review July 15, 2026 00:24
@lkdvos
lkdvos marked this pull request as draft July 15, 2026 00:24
@VinceNeede
VinceNeede force-pushed the vb-dmrg3s branch 2 times, most recently from 776835c to f2c34cc Compare July 20, 2026 13:50
@VinceNeede

Copy link
Copy Markdown
Contributor Author

Hi @lkdvos,

Pushed a sketch of merging alg_gauge/alg_post_expand along the lines you suggested. The dispatch side is in place: NoExpand/DMRG3S both wrap an inner gauge algorithm, gauge!/gauge2! dispatch on whichever one alg_gauge is, and local_update! ends up with a single unconditional gauge!(...) call — no if-guard, no second algorithm argument. The noise schedule advances once per outer iteration via _update_alg_gauge, threaded through find_groundstate!.

I haven't wired up DMRG's constructor yet, so this isn't runnable end-to-end through the public API — that's next, and here's the direction I'm planning to take it.

Since DMRG's trscheme/alg_svd/alg_orth also determine the inner gauge, a user could in principle construct DMRG3S with its own gauge already set (DMRG3S(noise, schedule, some_gauge)) and pass a different trscheme to DMRG, giving two conflicting sources for the same thing. My plan is to make trscheme/alg_svd/alg_orth keyword arguments of NoExpand/DMRG3S themselves, building the inner gauge there — rather than have them accessible from DMRG's constructor directly. That way each gauge object is self-contained, and the conflict can't arise in the first place. Let me know if you'd rather keep trscheme etc. as DMRG-level kwargs instead (with some other rule for resolving the conflict), otherwise I'll go ahead and wire it up this way.

@lkdvos lkdvos left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for rebasing onto that other work!

Considering the DMRG constructor, I think I agree that we should exclude being able to set conflicting schemes, but unless I'm missing something that would already not be possible - the gauging algorithm is either a (truncated) orth algorithm or DMRG3S, and in the latter case there is only a single (truncated) orth algorithm in the DMRG3S.
If at all possible, I would like to design the DMRG constructor to be able to take the trscheme still, and have that routed through automatically, or error if both a trscheme and DMRG3S are provided. My experience is that while these "algorithm structs" are very convenient as a developer, it is somewhat harder as a newcomer to the package to keep track of everything, and a simple keyword-argument approach, which possibly does not expose all settings but handles the common cases, helps alleviate some of these issues.

Comment thread src/algorithms/groundstate/dmrg.jl Outdated
Comment thread src/algorithms/post_expand/dmrg3s.jl Outdated
Comment thread src/algorithms/post_expand/dmrg3s.jl Outdated
@VinceNeede

VinceNeede commented Jul 21, 2026

Copy link
Copy Markdown
Contributor Author

Update

  • DMRG constructor has been updated to accept alg_gauge. The inner algorithm used in the svd/ortho step is constructed as before, ensuring back compatibility. trscheme is initially defaulted to nothing to prevent supplying the svd/ortho algorithm twice, it is then repristinated to the previous default as trscheme = something(trscheme, notrunc()). The docstring has been updated to explain the behavior and DMRG3S role.
  • fuser type restriction has been removed, and is now used to construct the combiner. The @plansor contraction has been reordered to mimic MPO_AC_Hamiltonian, even though it still can't be used directly for the above mentioned problems.

TODO

  • Move update_alg_gauge inside local_update!, verifying no significative change in performance/allocations is present
  • Understand how MPO_AC_Hamiltonian can be used for the contraction step

@lkdvos

lkdvos commented Jul 22, 2026

Copy link
Copy Markdown
Member

Thanks for the update! I commented on the reuse of the AC_projection, hopefully that makes sense, and I think it is safe to assume that the alg_gauge computation per site is not hurting performance, indeed after these two this looks ready to go for me.
Already thanks for putting in this effort, this is a very nice PR.

@VinceNeede

Copy link
Copy Markdown
Contributor Author

Both two last points have been implemented. I have explicitly update the compatibility of BlockTensorKit since it depends on QuantumKitHub/BlockTensorKit.jl#64. I have marked this ready for review.
Since the formatter is still not executing, I have run it locally:

vince@fedora:~/MPSKit.jl$ git log --oneline -1
660b14e (HEAD -> vb-dmrg3s, origin/vb-dmrg3s) add _get_combiner function and update gauge! to use MPO_AC_Hamiltonian for perturbation calculation
vince@fedora:~/MPSKit.jl$ git runic main
runic did not modify any files

@lkdvos lkdvos left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left some minor things to resolve, will merge afterwards:

  • the gauge! function is slightly inconsistent in its argument order, I think currently both psi as first and third argument exists, could you make that the same order everywhere?
  • Can we still simplify by getting rid of the NoExpand algorithm wrapper?

Comment thread src/algorithms/groundstate/dmrg.jl Outdated
Comment thread src/algorithms/groundstate/dmrg.jl Outdated
Comment thread src/algorithms/groundstate/dmrg.jl Outdated
Comment thread src/algorithms/post_expand/dmrg3s.jl Outdated
Comment thread src/algorithms/post_expand/dmrg3s.jl Outdated
Comment thread src/algorithms/post_expand/post_expand.jl
Comment thread src/algorithms/post_expand/dmrg3s.jl Outdated

@lkdvos lkdvos left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for all the work, will merge as soon as tests pass!

@VinceNeede

Copy link
Copy Markdown
Contributor Author

Thanks for the review! I've implemented the last two points we discussed. I also ran the formatter locally since it's still blocked here, let me know if the CI flags anything I missed, or if you'd like any further changes.

Really want to say thanks for the time and care you put into this review. I learned a lot about the internals of MPSKit going through it with you, and I appreciate you taking a somewhat unconventional PR from someone new to the codebase this seriously.

Comment thread src/algorithms/post_expand/post_expand.jl
Add `FunctionalSchedule`, wrapping an arbitrary `(noise, iter, ϵ) -> noise`
callable as a `NoiseSchedule`. Overload `∘` on `NoiseSchedule` to compose
two schedules into a `FunctionalSchedule`, following the usual function
composition convention (`s2` applied first, `s1` applied to its result).

Add a `threshold` keyword to `ExponentialDecay`, snapping the decayed noise
to exactly zero once it falls below it -- avoids running the (cheap but
non-free) expansion step indefinitely on a vanishingly small amplitude.
This also makes `ExponentialDecay ∘ Warmup` a natural way to combine a
hard iteration cutoff with smooth decay.
@lkdvos

lkdvos commented Jul 24, 2026

Copy link
Copy Markdown
Member

Hopefully now the format tests are fixed as well, will merge as soon as we get the green light from the tests :)

@lkdvos
lkdvos enabled auto-merge (squash) July 24, 2026 20:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants