[Discussion] Bring back `MPSKit.PeriodicArray`?

[leburgel]

Working with non-trivial cells has been, and still is I think, quite tricky in terms of correctly handling coordinates and directions. Looking at the current state of the code, all of the mod1, _next and _prev calls that are required to make this work aren't exactly helping with the transparency of the unit cell logic.

I remember that initially PEPSKit.jl was using MPSKit.PeriodicArrays to represent unit cells of infinite networks, but this approach was abandoned in the process of figuring out some very early issues with AD and Zygote.jl in particular. However, at this point never having to manually call mod1 feels like a very nice prospect to me. Should we have a go at bringing back the unit cell representation in terms of MPSKit.PeriodicArray? I think at this point we should be better equipped to figure out what issues this was causing, if there would be any issues with reinstating this now in the first place.

[lkdvos]

I would absolutely love that, although my hope was that the work of https://github.com/QuantumKitHub/PEPSKit.jl/tree/tiledarray would automatically include that.
The reality however is that I got that to a more or less working state, and then hit the AD wall and got a bit demotivated to actually continue to work on it.

One intermediate step though could be to already generalize the structs we are using.
I went a bit back and forth, trying to decide whether or not to hardcode the array types, or to make them parametric.
For example, I ended up hardcoding the tiled array for convenience:

PEPSKit.jl/src/states/infinitepeps.jl

Lines 10 to 12 in 30fc6fe

	struct InfinitePEPS{T<:PEPSTensor}
	A::InfiniteTiledArray{T,2}
	function InfinitePEPS(A::InfiniteTiledArray{T,2}) where {T<:PEPSTensor}

But something like

struct InfinitePEPS{T <: PEPSTensor, VA <: AbstractMatrix{T}}
    A::VA
end

Could be more future-proof as well.

This does raise the question, do you think it could make more sense to rethink some of the type parameters a bit? I've been wondering if something like this might not be more convenient:

struct InfinitePEPS{T <: Number, S <: ElementarySpace, VA <: AbstractMatrix{<:PEPSTensor{T, S}}}
    A::VA
end

Or in the other direction:

struct InfinitePEPS{VA <: AbstractMatrix{<:PEPSTensor}}
    A::VA
end

[pbrehmer]

I would also really like that but I would be careful with intermediate/half-baked solutions for this because rewriting all the contractions will probably be an annoying and error-prone effort. Personally, I would really love to have tiled arrays for this :-)

Regarding the InfinfitePEPS type parameters, I don't really have strong feelings but in the case of

struct InfinitePEPS{T <: Number, S <: ElementarySpace, VA <: AbstractMatrix{<:PEPSTensor{T, S}}}
    A::VA
end

I'm wondering whether this is generic enough. Couldn't there be some hypothetical case where we would like to have different space types S across the unit cell?

[lkdvos]

Not really, as this is not how tensorkit works (it will violently shout at you every step of the way if you try 😉 )
The correct way of achieving this is actually by embedding both space types in a direct product spacetype (deligne product if you wish to be precise), in which case the spacetype would again be uniform.

[lkdvos]

Also linking this here: https://github.com/Vexatos/CircularArrays.jl