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.

Codecov Report

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

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

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.

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

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

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

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?

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)