[WIP] TensorKitSectors #8
Conversation
|
For now I would prefer to keep our own types, and then bias towards code duplication and only call out to TensorKitSectors for nontrivial functionality (let's say, functionality where it is complicated enough to implement that it is worth the effort of converting the types). We can keep this kind of thing in mind once our packages are more developed. |
|
Sure, but how does that apply to either I think the main purpose of what I'm suggesting here is less centered around removing implementations that are here, even though that might be a side bonus. |
|
I was picturing we would keep our own type hierarchy and then have a set of conversions between the types. |
|
I do appreciate the idea of making certain sector types available that we don't have yet, but maybe we can think about that issue later. |
This is the beginning of some more code sharing for the sector definitions. (very much WIP)
I feel lilke there are many things to discuss here and some opinionated changes, so let me try and outline some of my thoughts here.
As we discussed, we can try and keep the types here, but that leads to a very big chunk of duplicated code, and conversions back and forth. To be honest, in that case it does not really seem worth it to reimplement everything by sending it to TensorKitSectors, since it takes more work to convert than it does to implement the minimal set of functions.
One different option might be to instead use:
const AbstractSector = TensorKitSectors.Sector. As outlined in thetensorkitsector.jlfile, it is relatively straightforward to forward the required methods for any<: TensorKitSectors.Sector, and this change would avoid having to rewrap them. Basically, this would simply make allSectortypes available to be used inGradedArrays(and vice versa)If we go on with that change, we might re-evaluate which types to keep, and which ones to simply adopt.
For example, the data representation of
O2differs slightly from theCU1Irrep, even though they represent the same group.Here it makes sense to define
struct O2 <: Sectorto follow the conventions defined here, which would then also make that available toTensorKit.Conversely, the
Z{N}types are virtually equal, so we might decide to useconst Z{N} = ZNIrrep{N}instead.I wouldn't really have any strong opinions about that, since they could still be used interchangably since they have the same abstract supertype.
In the long run, this seems like a very nice convenience feature, since this would allow conversion between the different array types, which might be useful for testing, comparing, and interoperability.
Before I go in and do so, I wanted to see how you all feel about this though.
Concretely and long story short: I would suggest adopting the abstract type from
TensorKitSectorsand building on top of that, such that effectively sectors can be shared between the two, in two directions.