Fixing changebonds, some inconsistency remaining by LHerviou · Pull Request #415 · QuantumKitHub/MPSKit.jl

LHerviou · 2026-04-17T09:08:32Z

Following discussion with @lkdvos

Here is a small fix for some minor bugs I noticed in changebonds: for RandExpand or SvdCut, the returned environments were not updated after the update of the mps. Schematically, we had
newmps, oldenv = changebonds(oldmps, H, alg = ..., oldenv)
which made any groundstate algorithm failed.
Now the environments are updated such that
newmps, newenv = changebonds(oldmps, H, alg = ..., oldenv)
without touching oldmps or oldenv

Before accepting the update: there are some inconsistencies in the behavior of changebonds.
In particular, for OptimalExpand, env are directly modified in place, which makes oldenv not work with the old mps. Is it intended behavior for memory optimization? If so, I can reproduce a similar behavior for others. It should in any case be advertised.

I modified also slightly the converter between InfiniteMPOHamiltonian and InfiniteMPO which was only used by OptimalExpand. There is some unsafe looking environment update in it, though.

The test pass for FiniteMPS, though I did not investigate it as much. In particular, the environment update in OptimalExpand is hidden in the computation of AC2.

Finally, I added a constructor of the type FiniteMPS(Type, Space, Space) as the syntax works for infiniteMPS and not Finite, which was annoying me.

For reproduction: here is a script that fails before the PR, and pass afterwards:

## Infinite chain, InfiniteMPOHamiltonian
L = 2
mps = InfiniteMPS(ComplexF64, repeat([ComplexSpace(2)], L), repeat([ComplexSpace(20)], L) )
H = transverse_field_ising(ComplexF64, Trivial, InfiniteChain(L))
env = environments(mps, H)

for alg in [OptimalExpand, RandExpand, SvdCut, VUMPSSvdCut]
  env = environments(mps, H)
  newmps, newenv = changebonds(mps, H, alg(trscheme = truncrank(10)), env)    
  try
    newmps, newenv, = find_groundstate(newmps, H, VUMPS(; maxiter=1, verbosity=1, tol= 1e-6), newenv)
  catch e
    error("Fail in Infinite $alg")
    #println(e)
  end
end

## Infinite chain, InfiniteMPO
L = 2
mps = InfiniteMPS(ComplexF64, repeat([ComplexSpace(2)], L), repeat([ComplexSpace(20)], L) )
H = InfiniteMPO(transverse_field_ising(ComplexF64, Trivial, InfiniteChain(L)))
env = environments(mps, H)

for alg in [OptimalExpand, RandExpand, SvdCut, VUMPSSvdCut]
  env = environments(mps, H)
  newmps, newenv = changebonds(mps, H, alg(trscheme = truncrank(10)), env)    
  try
    newmps, newenv, = find_groundstate(newmps, H, VUMPS(; maxiter=1, verbosity=1, tol= 1e-6), newenv)
  catch e
    error("Fail in Infinite $alg")
    #println(e)
  end
end


##Finite Chain
L = 10
mps = FiniteMPS(ComplexF64, repeat([ComplexSpace(2)], L), repeat([ComplexSpace(20)], L-1) )
H = transverse_field_ising(ComplexF64, Trivial, FiniteChain(L))
env = environments(mps, H)


for alg in [OptimalExpand, RandExpand, SvdCut]
  env = environments(mps, H)
  newmps, newenv = changebonds(mps, H, alg(trscheme = truncrank(10)), env)    
  try
    newmps, newenv, = find_groundstate(newmps, H, DMRG(; maxiter=1, verbosity=1, tol= 1e-6), newenv)
  catch e
    error("Fail in Finite $alg")
    #println(e)
  end
end```

github-actions · 2026-04-17T09:15:07Z

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

lkdvos · 2026-04-17T20:00:07Z

As a general comment, I feel like we are not super consistent about the role of environments throughout the entire package to be honest. In general they are somewhat treated as an in-place thing, so I think I would be okay with all functions counting as "destroying the input/taking ownership of that", which would then indeed mean that the environments are no longer compatible with the old state, unless you did an explicit copy. However, this is probably something I should better document 😁

LHerviou · 2026-04-20T08:21:29Z

If you want, I can go through the different changebonds functions to systematize it.
Just let me know whether you want to:

always modify the environments in place
never modify the environments

At least for infiniteMPS, we can make stuff consistent.
I will also update the doc accordingly

codecov · 2026-04-30T08:47:05Z

Codecov Report

❌ Patch coverage is 0% with 23 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
src/algorithms/changebonds/randexpand.jl	0.00%	7 Missing ⚠️
src/algorithms/changebonds/svdcut.jl	0.00%	7 Missing ⚠️
src/algorithms/changebonds/optimalexpand.jl	0.00%	5 Missing ⚠️
src/algorithms/changebonds/vumpssvd.jl	0.00%	2 Missing ⚠️
src/states/finitemps.jl	0.00%	2 Missing ⚠️

Files with missing lines	Coverage Δ
src/algorithms/changebonds/changebonds.jl	`0.00% <ø> (-100.00%)`	⬇️
src/algorithms/changebonds/vumpssvd.jl	`0.00% <0.00%> (-100.00%)`	⬇️
src/states/finitemps.jl	`73.48% <0.00%> (-21.24%)`	⬇️
src/algorithms/changebonds/optimalexpand.jl	`0.00% <0.00%> (-100.00%)`	⬇️
src/algorithms/changebonds/randexpand.jl	`0.00% <0.00%> (-100.00%)`	⬇️
src/algorithms/changebonds/svdcut.jl	`40.54% <0.00%> (-53.67%)`	⬇️

... and 74 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

Left some minor comments, but I definitely like that this is more principled with the inplace vs non-inplace functions.

lkdvos · 2026-04-30T12:06:01Z

 [Zauner-Stauber et al. Phys. Rev. B 97 (2018)](@cite zauner-stauber2018), by selecting the
 dominant contributions of a two-site updated MPS tensor, orthogonal to the original ψ.

+changedbonds! is only defined for FiniteMPS, and modify its environment.


Suggested change

changedbonds! is only defined for FiniteMPS, and modify its environment.

!!! note

`changebonds!` is only defined for `FiniteMPS`, and modifies its environment.

lkdvos · 2026-04-30T12:07:00Z

 distributed weights for each state in the two-site space and truncating that.

+The environments are not used here.
+changebonds! modify both the provided state and environments.


Suggested change

changebonds! modify both the provided state and environments.

`changebonds!` modifies both the provided state and environments.

lkdvos · 2026-04-30T12:07:35Z


    newψ = _expand(ψ, AL′, AR′)
-    recalculate!(envs, newψ, H)
+    envs = environments(newψ, H)


I'm not entirely following this change. Is it not possible to overwrite the old environments?

Ah, I think I see now what you intended: is it that the non-bang (!) functions shouldn't modify their inputs?
I can see that being a good choice, I think it is mostly that the envs are always kind of treated as a special thing that is modified everywhere.
I'm happy either way, I think you have a better feeling of what is convenient/efficient.

lkdvos · 2026-04-30T12:07:41Z


    newψ = _expand(ψ, AL′, AR′)
-    recalculate!(envs, newψ, H)
+    envs = environments(newψ, H) #  recalculate!(envs, newψ, H)


Same comment here

lkdvos · 2026-04-30T12:10:05Z


 An algorithm that uses a two-site update step to change the bond dimension of a state.

+changedbonds! is not defined.


Suggested change

changedbonds! is not defined.

!!! note

`changebonds!` is not defined.

Fixing changebonds, some inconsistency remaining

989c1d6

LHerviou added 2 commits April 17, 2026 11:27

Fixing formatting

e1f8e39

Fixing an order of calls (no local bug)

afdc245

lkdvos reviewed Apr 17, 2026

View reviewed changes

Comment thread src/algorithms/changebonds/optimalexpand.jl

Comment thread src/algorithms/changebonds/optimalexpand.jl Outdated

LHerviou added 2 commits April 20, 2026 10:30

Lukas proposed change, more explicit calls/type convert

9a29b8c

Fixing some inconsistencies, and at least adding details in docstrings

44d514c

lkdvos reviewed Apr 30, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixing changebonds, some inconsistency remaining#415

Fixing changebonds, some inconsistency remaining#415
LHerviou wants to merge 5 commits intoQuantumKitHub:mainfrom
LHerviou:fix_changebonds

LHerviou commented Apr 17, 2026

Uh oh!

github-actions Bot commented Apr 17, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

lkdvos commented Apr 17, 2026

Uh oh!

LHerviou commented Apr 20, 2026 •

edited

Loading

Uh oh!

codecov Bot commented Apr 30, 2026 •

edited

Loading

Uh oh!

lkdvos left a comment

Uh oh!

lkdvos Apr 30, 2026

Uh oh!

lkdvos Apr 30, 2026

Uh oh!

lkdvos Apr 30, 2026

Uh oh!

lkdvos Apr 30, 2026

Uh oh!

lkdvos Apr 30, 2026

Uh oh!

lkdvos Apr 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	changedbonds! is only defined for FiniteMPS, and modify its environment.
	!!! note
	`changebonds!` is only defined for `FiniteMPS`, and modifies its environment.

	changebonds! modify both the provided state and environments.
	`changebonds!` modifies both the provided state and environments.


		An algorithm that uses a two-site update step to change the bond dimension of a state.

		changedbonds! is not defined.

	changedbonds! is not defined.
	!!! note
	`changebonds!` is not defined.

Conversation

LHerviou commented Apr 17, 2026

Uh oh!

github-actions Bot commented Apr 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lkdvos commented Apr 17, 2026

Uh oh!

LHerviou commented Apr 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov Bot commented Apr 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

lkdvos left a comment

Choose a reason for hiding this comment

Uh oh!

lkdvos Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

lkdvos Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

lkdvos Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

lkdvos Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

lkdvos Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

lkdvos Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

github-actions Bot commented Apr 17, 2026 •

edited

Loading

LHerviou commented Apr 20, 2026 •

edited

Loading

codecov Bot commented Apr 30, 2026 •

edited

Loading