Some observations concerning correlation_length

[borisdevos]

This is a two-parter. This really stems from me not thinking hard enough beforehand in my specific case, but it might be more generally applicable.

1. The AKLT model's correlation length is known to be 1/ln(3). If you call correlation_length without passing a sector (because in general you don't know which one to look at, here obviously not the case with the D = 2 solution), it just errors because in the unit sector it filters out the only transfer matrix eigenvalue, namely the leading one. The error is cryptic, and I'm not sure if we want this to actually immediately error. Maybe a preventive NaN?

The stacktrace:

Details

julia> correlation_length(gs) # gs is the AKLT state
ERROR: BoundsError: attempt to access 0-element Vector{Float64} at index [1]
Stacktrace:
  [1] throw_boundserror(A::Vector{Float64}, I::Tuple{Int64})
    @ Base .\essentials.jl:15
  [2] getindex
    @ .\essentials.jl:919 [inlined]
  [3] first
    @ .\abstractarray.jl:452 [inlined]
  [4] marek_gap(spectrum::SubArray{ComplexF64, 1, Vector{ComplexF64}, Tuple{UnitRange{Int64}}, true}; tol_angle::Float64)
    @ MPSKit c:\Boris\Unief\PhD\Code\Roughening transition\MPSKit.jl\src\algorithms\transfer_spectrum.jl:144
  [5] (::MPSKit.var"#575#576"{Float64})(c::SU2Irrep)
    @ MPSKit .\none:-1
  [6] iterate
    @ .\generator.jl:48 [inlined]
  [7] collect(itr::Base.Generator{Vector{SU2Irrep}, MPSKit.var"#575#576"{Float64}})
    @ Base .\array.jl:790
  [8] TensorKit.SortedVectorDict(kv::Base.Generator{Vector{SU2Irrep}, MPSKit.var"#575#576"{Float64}})
    @ TensorKit C:\Users\bodvos\.julia\packages\TensorKit\SLHMM\src\auxiliary\dicts.jl:76
  [9] marek_gap(above::InfiniteMPS{…}; sector::Nothing, tol_angle::Float64, howmany::Int64, kwargs::@Kwargs{})
    @ MPSKit c:\Boris\Unief\PhD\Code\Roughening transition\MPSKit.jl\src\algorithms\transfer_spectrum.jl:130
 [10] correlation_length(above::InfiniteMPS{TensorMap{…}, TensorMap{…}}; sector::Nothing, kwargs::@Kwargs{})
    @ MPSKit c:\Boris\Unief\PhD\Code\Roughening transition\MPSKit.jl\src\algorithms\transfer_spectrum.jl:171
 [11] correlation_length(above::InfiniteMPS{TensorMap{…}, TensorMap{…}})
    @ MPSKit c:\Boris\Unief\PhD\Code\Roughening transition\MPSKit.jl\src\algorithms\transfer_spectrum.jl:170
 [12] top-level scope
    @ REPL[53]:1
Some type information was truncated. Use `show(err)` to see complete types.

2. I actually got to this state with a two-site unit cell, and in the correct spin-1 sector the correlation length is half what it's supposed to be. Of course, in hindsight it's obvious because of how transfer matrices are multiplied within a unit cell for the transfer matrix, so eigenvalues belong to a unit cell. At least for me it was still unexpected to have the correlation length be returned in unit cells, not sites. This should at least be documented?

[lkdvos]

Let me first link some related things here, so this is easier to find in the future:

• [discussion] correlation length interface PEPSKit.jl#216
• refactor: transfer_spectrum and correlation_length default to sector-centered implementation #452

Long story short, I absolutely agree that this is not ideal and we should change this. (For the sake of getting a release out, I will not include this in v0.14 though).

---

I think the easiest strategy here is to follow what was already done for transfer_spectrum and entanglement_spectrum, which is to by default return a SectorVector or SectorDict, which simply returns all correlation lengths unless you manually specify a sector, in which case it returns only one.

This would somewhat bypass your question about the error, in some cases. The main point then is that whenever the degeneracy is exactly 1, we don't actually know whether this is the case because all other transfer eigenvalues are 0 or because we are missing the next one. While I agree that NaN could be a way around this, I'd be happy to just throw a better error, since we really can't return anything reasonable. This is of course only when you ask for a single correlation length in a given sector, when you ask for all of them it would be annoying if one of the values makes everything error?
I guess NaN could be a solution there, but I would like to print some information about this, perhaps through a warning, because obtaining NaN is about as illuminating as the stacktrace you posted, if not worse because it is not immediately clear where it came from.

Considering the convention for the unit cell normalization, I have no preferences here although I want everything to be consistent, which in particular means the quasiparticle momenta and correlation lengths are per unit cell, but I also think our expectation values are per unit cell. The main point here is that indeed this naturally pops out of how it is computed, and there are cases where this is a more natural measure, for example if the unit cell really is just various orbitals on a given atom, or ... (Ultimately, the unitcell determining the unit does not seem all that bad to me 😉).
Feel free to open a PR with a better documentation for this though, I tend to always forget myself which one it is so that would be very convenient.