Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rewrite MPS types to use C instead of CR (and CL) and add ψ.center functionality. #216

Merged
merged 45 commits into from
Dec 19, 2024

Conversation

VictorVanthilt
Copy link
Contributor

@VictorVanthilt VictorVanthilt commented Dec 19, 2024

This PR changes the name of MPS bondtensors. The bond tensor for all MPS types is now called C and is equivalent to what was previously called CR.

A new property "center" of FiniteMPS is introduced. It can be accessed through ψ.center.

It will return a HalfInteger i:

  • isinteger(i)i is a whole number and indicates the location of the first AC tensor present in ψ.ACs
  • ishalfodd(i)i is a half-odd-integer (e.g. 3/2), meaning that there are no AC tensors, and indicating between which sites the bond tensor lives.
    For example: i = 3/2 (= 1.5) means that there lives a C between the AL on site 1 and the AR on site 2

The goal is to use this new center property to avoid unnecessary gaugefixing in methods like norm(::FiniteMPS) where before, the center gauge was brought to the first site, after which the norm was calculated. After this PR the norm is calculated using the (bond) tensor at ψ.center, without gaugefixing.

An important note is that ψ.C[center(ψ)] does not work, ψ.C should be indexed using integers and ψ.C[i] returns the bond tensor between sites i and i+1

@Gertian
Copy link
Collaborator

Gertian commented Dec 19, 2024

In my opinion renaming CR to C might also be confusing since it's then unclear if C[site] corresponds to the C matrix left or right of that site.
(Admittedly the CR is also confusing but this was the original motivation for the R)

In hindsight it's probably most logical to allow indixation of the Cs with half integers. This way we get things like : state.C[1+1//2] which should be clear to anyone reading/writing the code.

@VictorVanthilt
Copy link
Contributor Author

VictorVanthilt commented Dec 19, 2024

Nothing that good documentation (that is planned) can't fix. Its also in the docstring of FiniteMPS.
Storing CLs and only having the CR property, like it was before with FiniteMPS, was even more confusing imo.

@Gertian
Copy link
Collaborator

Gertian commented Dec 19, 2024

It surely was confusing. Essentially I looked into the code to see what it returned whenever relevant...
Anyways, good docs are indeed sufficient.

For the front end user half indexing might be very handy and user friendly though. If you and @lkdvos are open to this I can make a PR. Imo both can just coexist :)

@VictorVanthilt
Copy link
Contributor Author

VictorVanthilt commented Dec 19, 2024

@Gertian indexing with HalfIntegers doesn't seem practical nor aesthetically pleasing to @lkdvos and me so we will decide against this. I updated the FiniteMPS docstring to explain the implementation of the center property. It also contains the convention of where C[i] lives (to the right of site i).

@VictorVanthilt VictorVanthilt enabled auto-merge (squash) December 19, 2024 19:32
@lkdvos lkdvos disabled auto-merge December 19, 2024 21:02
src/states/orthoview.jl Outdated Show resolved Hide resolved
src/states/finitemps.jl Show resolved Hide resolved
@lkdvos lkdvos enabled auto-merge (squash) December 19, 2024 22:20
@lkdvos lkdvos disabled auto-merge December 19, 2024 22:21
@lkdvos lkdvos merged commit 71b709b into QuantumKitHub:master Dec 19, 2024
28 checks passed
@lkdvos lkdvos deleted the rewrite-MPS branch December 19, 2024 22:24
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