Conversation
Codecov Report❌ Patch coverage is
... and 5 files with indirect coverage changes 🚀 New features to boost your workflow:
|
lkdvos
left a comment
There was a problem hiding this comment.
I don't think the _tr_repr is really needed, I would expect that you should be able to follow the same printing implementation as is used for the group irreps.
The main point there is that there are a number of components that make this work:
- In the implementation of
show, we are printing the fields directly. - There exist
convertmethods that take the fields and convert it back into a sector, see e.g. thednirrepone. - The default constructor of a type will automatically call
converton the provided arguments
Let me know if this helps, otherwise I'll have a look to more cleanly show what I mean.
As an aside, note how in the irrep show method we are checking the typeinfo context to see whether or not the type actually still has to be printed, which is also how you can show a sector without actually printing the type, by passing the IOContext(io, :typeinfo => ...) instead.
src/timereversed.jl
Outdated
| function fusiontensor( | ||
| a::TimeReversed{I}, b::TimeReversed{I}, c::TimeReversed{I} | ||
| ) where {I <: Sector} | ||
| return fusiontensor(a.a, b.a, c.a) # conj here? all fusiontensors available are real |
There was a problem hiding this comment.
I would guess yes, but additionally there might be some weird interplay with multiplicities too. Basically, since you can compute the F and R from the fusiontensor, I think you can work your way back from there to figure out what this has to be?
There was a problem hiding this comment.
I've convinced myself that they should be the same for the time-reversed sector. Basically, time-reversal will only invert the braiding, but should have no influence on associativity. This makes sense that the "F-symbol-from-fusion tensor" test requires untouched fusion tensors, since orientation doesn't matter. For the "R-symbol-from-fusion tensor" test the same fusion tensors still appear on both sides, but just with swapped legs. Importantly, nothing changes for the vertex itself, so the fusion tensor should remain untouched.
I don't know if this reasoning is sound, though. Also, I'm not sure what you mean by weird interplay with multiplicities.
There was a problem hiding this comment.
Wouldn't you expect that everything gets conjugated? I'll think about it; I think this can definitely be found in some of the category books. Also, in reviewing/modifying the fusion tree PR and adding some tests, I am now using (still to be committed):
uniquefusionsectorlist = (
Z2Irrep, Z3Irrep, Z4Irrep, Z3Irrep ⊠ Z4Irrep,
U1Irrep, FermionParity, FermionParity ⊠ FermionParity, FermionNumber,
Z3Element{1}, ZNElement{5, 2},
)
simplefusionsectorlist = (
CU1Irrep, SU2Irrep, FibonacciAnyon, IsingAnyon,
FermionParity ⊠ U1Irrep ⊠ SU2Irrep, FermionParity ⊠ SU2Irrep ⊠ SU2Irrep,
Z3Element{1} ⊠ FibonacciAnyon ⊠ FibonacciAnyon,
)
genericfusionsectorlist = (
A4Irrep, A4Irrep ⊠ FermionParity, A4Irrep ⊠ SU2Irrep, A4Irrep ⊠ Z3Element{2}, A4Irrep ⊠ A4Irrep,
)
multifusionsectorlist = (
IsingBimodule, IsingBimodule ⊠ SU2Irrep, IsingBimodule ⊠ IsingBimodule, IsingBimodule ⊠ Z3Element{1}, IsingBimodule ⊠ FibonacciAnyon ⊠ A4Irrep,
)
sectorlist = (
uniquefusionsectorlist...,
simplefusionsectorlist...,
genericfusionsectorlist...,
multifusionsectorlist...,
)Taking the DeligneProduct with Z3Element{1} makes Fsymbols complex, which is definitely something that I want to test more rigorously against.
There was a problem hiding this comment.
I think this is confusing because what time-reversal does here is not the same as what some sources might call the opposite or reverse category. Really this is just selecting the other braided fusion category which satisfies the same pentagon equations, but is the "other" solution to the hexagon equation, namely what used to be underbraiding is now defined to be overbraided. Thinking about this, I actually don't know if just taking the adjoint of the R-symbol is enough, but the first two arguments might need to be switched.
There was a problem hiding this comment.
In that case, I would maybe argue to leave out the fusiontensor definition.
The point is that if you want this to be consistent with the Rsymbol, you have to have that dot(fusiontensor(a, b, c), fusiontensor(b, a, c)) = dim(c) * Rsymbol(a, b, c) survives.
This would be fixed by conjugating the fusiontensors, but then this would mess up the Fsymbol consistency, so my intuition here seems to point at the fact that this only really is possible for real representations, or real braidingscalartype.
Thinking about this a bit further, don't we have that TimeReversed of any group irrep is trivial anyways, since BraidingStyle(I) <: Bosonic implies symmetric braiding, and therefore there is a gauge where the Rsymbol is real (R^2 = 1)?
So basically, all of the cases where fusiontensor makes sense coincide with the cases where TimeReversed is trivial?
There was a problem hiding this comment.
I understand the arguments, but it's a bit annoying because the actual main point of this PR (besides the pretty printing) was to actually still convert time-reversed graded tensors to arrays. And even though it's trivial for cases where the fusion tensor can even be defined, this should just be possible, no?
You might wonder why I would use TimeReversed in cases where it's trivial. It simply boils down to being mathematically correct in dealing with the symmetry of the bra state, and is also somewhat helpful in tracking the origin of the symmetries when considering both those of the bra and ket. But this isn't a hill I'm willing to die on, and I'm fine with just defining this method locally 🤣
There was a problem hiding this comment.
Some possible alternative strategies:
timereversed(x::AbstractIrrep) = x, which could also just be extended forisrealsector types in general- Define
fusiontensor(::TimeReversed...)but error out if the sectortype is not real.
I honestly don't particularly like the second option, since this really is just code complexity for the sake of being pedantic. I feel like you have to choose between being pedantic and generic, in which case converting to an array is not defined, or being pragmatic and simply using the code instead of the math, but that is of course just my opinion 😉
There was a problem hiding this comment.
I don't like the first option, because then the time-reversed irrep no longer shows as actually time-reversed. Also, I don't see why the error in the second is necessary; we currently do not support fusion tensors beyond group irreps anyway, where there is indeed no issue.
There was a problem hiding this comment.
The error is necessary because this definition is just wrong in a general case, and just relying on the fact that we currently don't have a fusiontensor definition that would violate this, it is something that might very easily lead to unexpected issues down the line. (e.g. if you or someone else for whatever reason decide to define fusiontensor as a triple line thing in 12 months, it would be pretty hard for them to then remember that there is a silent assumption in the fusiontensor definition of TimeReversed, which might go unnoticed for a couple more months, and leads to incredibly hard to track bugs. It's definitely better to spend the time to explicitly check these assumptions).
Also, just reading the code as is, this seems to suggest that the general definition of fusiontensor for this case is given by the exact same fusiontensor, which is not true. From an educational point of view that means that reading this code is also misleading, and I would argue that if you do not like irreps not printing as time-reversed in the case where time reversion is an identity operation, I equally dislike printing code that is only correct when time reversion is an identity operation without explicit errors 😉.
There was a problem hiding this comment.
I can follow your last argument, and am willing to sacrifice the time-reversed printing where it simply means nothing :)
|
Regarding the last commit and revert, |
It was a little annoying to work with the
TimeReversedsector, so I added some constructors to make life easier.I also needed fusion tensors of time-reversed sectors. I'm not sure if these need to be conjugated or not, but every single fusion tensor we have available across all packages with sectors define them with real representations.
I attempted to make printing prettier with this sector as well, since before you'd see e.g.
TimeReversed{A4Irrep}(Irrep[A₄](0)). What I actually wanted was something likeTimeReversed{Irrep[A₄]}(0), but this seems to mess up in the parsing test of a sector. Also, if something like this would work, I'm guessing there needs to be a fallback to just print it fully, since this_tr_reprfunction might not be defined.Some comments should be addressed and removed before merging.