Skip to content

Fallbacks for product sectors involving fusion tensors#80

Open
borisdevos wants to merge 2 commits intomainfrom
bd/morefallbacks
Open

Fallbacks for product sectors involving fusion tensors#80
borisdevos wants to merge 2 commits intomainfrom
bd/morefallbacks

Conversation

@borisdevos
Copy link
Member

@borisdevos borisdevos commented Mar 16, 2026

This is necessary when ..._from_fusiontensor has its own method within downstream packages to circumvent view. I encountered this specifically when testing SU3Irrep ⊠ SU3Irrep, but it's probably relevant within CategoryData as well.

@codecov
Copy link

codecov bot commented Mar 16, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

Files with missing lines Coverage Δ
src/TensorKitSectors.jl 16.66% <ø> (ø)
src/auxiliary.jl 94.31% <100.00%> (+1.89%) ⬆️
src/product.jl 93.45% <100.00%> (-1.33%) ⬇️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Member

@lkdvos lkdvos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, makes sense to me!

Minor suggestion: I think it might be possible to avoid some code repetition by factoring out some of the shared logic, although it is not entirely trivial because of the sizes right now.
Perhaps replacing kron with kron!, implemented through tensorproduct!, you can simply allocate the destination and go from there?
If I'm not making sense, let me know, I might find some time to give a better example of what I mean.

@borisdevos
Copy link
Member Author

I can get to the sizes through the data themselves. They'll be either numbers, matrices or 4-dimensional arrays. So I can definitely refactor the matrix vs number or 4d array vs number cases easily, but I'm guessing that's not what you have in mind?
Your second comment indeed doesn't make sense to me currently 😅

@Jutho
Copy link
Member

Jutho commented Mar 16, 2026

Would it be useful if Fsymbol_from_fusiontensor and friends always return an Array of the correct size? And then the fallback that computes Fsymbol via Fsymbol_from_fusiontensor can convert from Array to scalar in the MultiplicityFreeFusion cases?

@lkdvos
Copy link
Member

lkdvos commented Mar 16, 2026

What I meant is that it would be nice if something like this would work:

_kron(A::Number, B::Number) = A * B
_kron(A::AbstractArray, B::Number) = A * B
_kron(A::Number, B::AbstractArray) = A * B
_kron(A::AbstractArray, B::AbstractArray) = ...

But somehow I'm assuming this will lead to issues, given that you explicitly turn the numbers into matrices first?

@borisdevos
Copy link
Member Author

borisdevos commented Mar 17, 2026

Would it be useful if Fsymbol_from_fusiontensor and friends always return an Array of the correct size? And then the fallback that computes Fsymbol via Fsymbol_from_fusiontensor can convert from Array to scalar in the MultiplicityFreeFusion cases?

This already happens, no?

What I meant is that it would be nice if something like this would work:

_kron(A::Number, B::Number) = A * B
_kron(A::AbstractArray, B::Number) = A * B
_kron(A::Number, B::AbstractArray) = A * B
_kron(A::AbstractArray, B::AbstractArray) = ...

But somehow I'm assuming this will lead to issues, given that you explicitly turn the numbers into matrices first?

This does work, with the last method being the current _kron implementation, such that I can just return e.g. _kron(F1, F2) before the if-else blocks. The only issue is that the array x number or number x array cases will have the wrong sizes. The number being 0 will not insert in which vertex the zero(s) appeared, so it ends up being a filled array with entry/entries zero. This actually only got picked up in

@test size(Bsymbol(a, b, c)) == (0, 0)
, meaning I need to work on the shapes PR again 🫠

@Jutho
Copy link
Member

Jutho commented Mar 17, 2026

This already happens, no?

No:

julia> TensorKitSectors.Rsymbol_from_fusiontensor(U1Irrep(0), U1Irrep(0), U1Irrep(0))
1

julia> TensorKitSectors.Rsymbol_from_fusiontensor(U1Irrep(0), U1Irrep(0), U1Irrep(1))
0

What I mean is that these would respectively return

julia> TensorKitSectors.Rsymbol_from_fusiontensor(U1Irrep(0), U1Irrep(0), U1Irrep(0))
1×1 Matrix{Int}:
 1
julia> TensorKitSectors.Rsymbol_from_fusiontensor(U1Irrep(0), U1Irrep(0), U1Irrep(1))
0×0 Matrix{Int}

@Jutho
Copy link
Member

Jutho commented Mar 17, 2026

So the output any of the _from_fusiontensor would never be a Number, but always be an Array of the correct size.

@Jutho
Copy link
Member

Jutho commented Mar 17, 2026

However, that might make it harder to use these functions for what they were intended, namely comparing the exactly programmed result with the generically determined result from the fusion tensors.

I thought there were also fallbacks, that did

Rsymbol(a, b, c) = Rsymbol_from_fusiontensor(a, b, c)

and these could then do the conversion from Array to Number when appropriate. But I guess that was not the main point of the _from_fusiontensor functions.

@borisdevos
Copy link
Member Author

You're right, I would also like to reserve the _from_fusiontensor functions to only test vs the manually implemented data. This means that the refactoring needs to be done another way. I made an attempt at one, which makes use of properties I add tests to in #64. Let me know what you guys think.

@lkdvos
Copy link
Member

lkdvos commented Mar 19, 2026

I'm not sure about keeping these function for only testing purposes. They should also be available to SUNRepresentations.jl, or other finite group implementations where these formulas can be unavailable or cumbersome to come by?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants