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

Expand exact_diagonalization and FiniteMPS docstrings #261

Merged
merged 5 commits into from
Mar 3, 2025

Conversation

leburgel
Copy link
Member

@leburgel leburgel commented Mar 2, 2025

Some docstring additions on how to obtain charged eigenstates using exact_diagonalization. I also added a line on charged states in the FiniteMPS docstring, since this is very related and there have been complaints about this not being clear in the past.

See also #260.

@leburgel
Copy link
Member Author

leburgel commented Mar 2, 2025

I was looking at this, and I don't really know how this could work when len != length(opp). It seems opp is never restricted in length, so then envs = environments(state, opp) should somehow work for a state and environment which have a different length? Seemed very strange, so thought I should at least bring it up here.

Copy link

codecov bot commented Mar 2, 2025

Codecov Report

Attention: Patch coverage is 0% with 6 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/algorithms/ED.jl 0.00% 6 Missing ⚠️
Files with missing lines Coverage Δ
src/states/finitemps.jl 77.97% <ø> (ø)
src/algorithms/ED.jl 0.00% <0.00%> (ø)

@lkdvos
Copy link
Member

lkdvos commented Mar 2, 2025

Thanks for the clarifications.

As a question, should we consider actually swapping the sector for the dual? Since the current charge is added to the left, it has an outgoing arrow, which is why it is somehow the opposite of what most people expect, and I don't like that it does not align with the results of calling eigvals on the dense hamiltonian

@leburgel
Copy link
Member Author

leburgel commented Mar 2, 2025

As a question, should we consider actually swapping the sector for the dual?

I would kind of like this, since this means that for example targeting a state containing N bosons can be done by passing U1Irrep(+N) instead of U1Irrep(-N) now. However, I don't think it's a good idea to change this since all other charged states or tensors also put a Vect[typeof(sector)](sector => 1) in the codomain of the corresponding tensor map (see e.g. excitations, transfer_spectrum, ...). So while it's a bit confusing, it really is a package-wide convention that is at least being followed consistently internally.

I think that if we instead

  • Clearly document that specifying a sector adds this as an outgoing charge, i.e. in the codomain.
  • Provide an example of how to work with charged finite states.

Things should be clear to users and still consistent internally.

@lkdvos
Copy link
Member

lkdvos commented Mar 2, 2025

We could also just change this package-wide, and make a breaking release?

@leburgel
Copy link
Member Author

leburgel commented Mar 2, 2025

If we change it everywhere that's also fine, I would definitely find this more intuitive.

@Jutho
Copy link
Member

Jutho commented Mar 2, 2025

I agree that this would be much more intuitive and in line with how I would think this would have been implemented 😄 .

Copy link
Member

@lkdvos lkdvos left a comment

Choose a reason for hiding this comment

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

Given the above and an internal poll, let's merge this set of changes, and settle on changing the convention in the next breaking release. This way, I can still include this in a released version before that.

Let's keep the other issue open as a reminder.

Copy link
Member

@lkdvos lkdvos left a comment

Choose a reason for hiding this comment

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

Ok when tests pass.

@ZongYongyue
Copy link

Sorry to bother you, but I’d like to raise a related question here. Since the MPSKit documentation doesn’t explicitly describe how to create a charged state A=c^+∣0>, my previous solution was to add a trivial leg to the 3-leg tensor c^+ and place it at the desired site in the FiniteMPO. On the side of the trivial leg, I filled the sites with the identity operator I, and on the charged leg side, I filled the sites with the parity or braiding operator Z. This resulted in a FiniteMPO structure resembling −I−I−I−C−Z−Z−Z−:

function chargedMPO(operator::AbstractTensorMap, site::Integer, nsites::Integer)
    pspace = domain(operator)[1]
    if (length(domain(operator)) == 2)&&(length(codomain(operator)) == 1)
        Z, vspace = fZ(operator), domain(operator)[2]
        I = isomorphism(storagetype(operator), oneunit(vspace)*pspace, pspace*oneunit(vspace))
        mpo = FiniteMPO([i < site ? I : i == site ? add_single_util_leg(operator) : Z for i in 1:nsites])
......

function fZ(operator::AbstractTensorMap)
    length(domain(operator))==2 ? vspace=domain(operator)[2] : length(codomain(operator))==2 ? vspace=codomain(operator)[1] : throw(ArgumentError("invalid creation or annihilation operator"))
    pspace = domain(operator)[1]
    iso₁ = isomorphism(storagetype(operator), vspace, vspace)
    iso₂ = isomorphism(storagetype(operator), pspace, pspace)
    @planar Z[-1 -2; -3 -4] := iso₁[-1; 1] * iso₂[-2; 2] * τ[1 2; 3 4] * iso₂[3; -3] * iso₁[4; -4]
    return Z
end

While this approach solved the problem (and matches my exact diagonalization in multiple test cases), it feels somewhat inelegant. What would be the most appropriate way to handle this in MPSKit?

@lkdvos
Copy link
Member

lkdvos commented Mar 3, 2025

That's definitely the way to do it if you really need a specific charged state.
If all you care about is any charged state with the correct sector, you could also initialize the the MPS with a charged left or right leg, as described in the docstring additions in this PR.

Note that it's a bit annoying to try and automatically handle these things, for the particular reason that you can arbitrarily choose the left or the right side to push this auxiliary charge into, but you really need to be consistent. I would gladly have a better interface for this, but I just don't have the time to think it through properly.
If you would have an interface in mind that you would like, do let me know.
Typically, it does not take me too much time to implement something, but coming up with the interface is what makes everything time-consuming :).

(also, if you want to continue this discussion, it's a bit more convenient to open a separate issue/discussion for that, since this is not necessarily related to this PR and should not block merging it)

@lkdvos lkdvos merged commit be1e2a1 into master Mar 3, 2025
27 of 28 checks passed
@lkdvos lkdvos deleted the lb/exact_diagonalization_docstring branch March 3, 2025 16:20
@ZongYongyue
Copy link

ZongYongyue commented Mar 3, 2025 via email

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.

4 participants