Skip to content

Conversation

@ryanlevy
Copy link
Contributor

This implements two improvements:

  • finite_mps can now have its gauge center be on the first site instead of the last site
  • orthogonalize ensures a real Hermitian transfer matrix eigenvector thanks to schursolve (shout out @lkdvos for the tip)

The first change is a small speed improvement for long finite_mps calls as correlation_matrix will immediately move the orthogonality center to the first site

julia> @btime corrR = correlation_matrix(finite_mps(ψ,1:24; ortho="right"),"Cdagup","Cup"; sites=2:24+1);
  57.864 ms (900184 allocations: 146.93 MiB)

julia> @btime corrL = correlation_matrix(finite_mps(ψ,1:24; ortho="left"),"Cdagup","Cup"; sites=2:24+1);
  47.981 ms (764616 allocations: 117.28 MiB)

The second change includes the additional "gauge fixing" of the output of vumps in the Hubbard example, which should probably always be done so that the state you want to measure has a more consistent C matrix.

λ₁ᴿᴺ, v₁ᴿᴺ = λ⃗₁ᴿᴺ[1], v⃗₁ᴿᴺ[1]

if size(TT, 2) > 1 && TT[2, 1] != 0
@warn("Largest transfer matrix eigenvector is not real?")
Copy link
Member

Choose a reason for hiding this comment

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

Seems a bit funny to make this a question. Can't you check if it is real?

Copy link
Member

Choose a reason for hiding this comment

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

If it isn't a definitive check, it would be better to phrase it as "Largest transfer matrix eigenvector might not be real.".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess it should be "Non-unique largest eigenvector" (or the eigenvector shouldn't be real)

Copy link

Choose a reason for hiding this comment

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

I think what it really means is that it is not unique, this results in complex conjugate eigenvalue pairs (hence checking the TT[2, 1], TT[1:2,1:2] = [real(lambda) imag(lambda); -imag(lambda) real(lambda)] or something like that).

@mtfishman
Copy link
Member

Nice to see it get more general and simpler!

@ryanlevy
Copy link
Contributor Author

@lkdvos it seems that using eager=true causes the vector to be non-hermitian (at least higher than the Arnoldi tolerance)

@lkdvos
Copy link

lkdvos commented Feb 27, 2025

That's interesting... The only difference should be that it is checking every iteration if the tolerance has been reached, instead of waiting until the full krylov subspace is populated. Basically, it's probably not fair to expect that you reach the same tolerance for the eigenvector hermiticity as you would get for the eigenvector itself. If you don't put eager=true, you often just get a higher precision than you asked for, because you are doing additional iterations. However, in our cases the extra iterations are quite expensive, so this strongly affects performance

@mtfishman
Copy link
Member

mtfishman commented Feb 27, 2025

Sounds like we should play around with different tolerances when checking for Hermiticity, I don't think there is anything magical about the current choice besides that it has worked so far for catching issues while not being too strict.

@ryanlevy
Copy link
Contributor Author

I noticed a bug with orthogonalize, you can't call it twice because it adds tags which interfere with the next run

julia> p = orthogonalize.AL, : ; tol=1e-14)
       p2 = orthogonalize(p.AL, :)
ERROR: In `isapprox(::ITensor, ::ITensor)`, the indices of the ITensors do not
          match. The first ITensor has indices:

#....

@mtfishman
Copy link
Member

I noticed a bug with orthogonalize, you can't call it twice because it adds tags which interfere with the next run

julia> p = orthogonalize.AL, : ; tol=1e-14)
       p2 = orthogonalize(p.AL, :)
ERROR: In `isapprox(::ITensor, ::ITensor)`, the indices of the ITensors do not
          match. The first ITensor has indices:

#....

Got it, I don't have time to look into that but feel free to make a PR.

@ryanlevy
Copy link
Contributor Author

@mtfishman is this good to go?

@mtfishman
Copy link
Member

Can you bump the package version?

@ryanlevy
Copy link
Contributor Author

Sure thing!

@mtfishman mtfishman merged commit 65c24b3 into ITensor:main Mar 13, 2025
6 checks passed
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.

3 participants