-
Notifications
You must be signed in to change notification settings - Fork 5
FusionDataStyle type
#49
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
base: main
Are you sure you want to change the base?
Conversation
Codecov Report❌ Patch coverage is
🚀 New features to boost your workflow:
|
lkdvos
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not entirely sure how I feel about this, since I would say there are some conflicting things that influence this decision.
On the one hand, I agree that it would be nice to keep things non-breaking. However, at least in the current design, I'm not sure this is still non-breaking. If we add a call to FusionDataStyle in TensorKit, we are assuming this is implemented for all Sectors, which at least would require a default of NonTrivialFusionData to be registered.
I also just feel like the "cleanest" solution in the long run would just be to have an additional TrivialFusion <: FusionStyle step to the ladder of FusionStyles, however this might also be breaking, since we are using things like FusionStyle(x) isa UniqueFusion to detect anything that doesn't have multiple fusion outputs, which would now fail for TrivialFusion.
If we really want this to be non-breaking, I think we should just fix the foldright implementation to not assume that the Fsymbols are 1, and just specialize that function for <:AbelianIrrep in order to not have any performance regressions there. I don't think the performance is that crucial here anyways, in the sense that even the SimpleFusion implementations might just be fast enough?
I'm slightly hesitant to really add a new style specifically for this, since FusionDataStyle really isn't something that is independent of the FusionStyle, so that feels a bit off to me
|
I'm down to look into fixing I also just want to add that you would specialise to more than just abelian irreps, but also fermions, |
This PR is a proposal to fix QuantumKitHub/TensorKit.jl#245. This allows us to keep
UniqueFusionthe way it is, and is an easier solution to rewritingTensorKit.foldrightitself. Specifically, https://github.com/QuantumKitHub/TensorKit.jl/blob/155aa8997ef8eba1dfa53cd368097ae41aa2a10b/src/fusiontrees/manipulations.jl#L352 would then have an additional&& FusionDataStyle(I) isa TrivialFusionDatacheck. Testing locally withI = Z2Element{1}and this passes the transpose tests.*I didn't think too hard about the name. Other options I can think of are
AssociatorStyleandRecouplingStyle. All these names have their merits and demerits. I also didn't think if this was the ideal approach, but I just wanted to get the ball rolling.*Unrelated to this PR, but as of now
Z2Element{1}passes all TensorKit tests, andZ3Element{1}passes the transpose tests, but fail at braiding tests. Still looking into this, but it seems that the non-self-dual nature and non-trivial F-symbols are messing things up somewhere, even if I blindly add theFusionDataStylecheck whereUniqueFusionis checked. This is a WIP, just wanted to mention it here already.