Fixing changebonds, some inconsistency remaining

[LHerviou]

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]

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

[lkdvos]

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]

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]

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.46% <0.00%> (-20.88%)	⬇️
src/algorithms/changebonds/optimalexpand.jl	0.00% <0.00%> (-100.00%)	⬇️
src/algorithms/changebonds/randexpand.jl	0.00% <0.00%> (-97.57%)	⬇️
src/algorithms/changebonds/svdcut.jl	41.09% <0.00%> (-53.11%)	⬇️

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