Make testsuite compatible with multifusion categories

[borisdevos]

With the aid of random_fusion (which was already made in the TensorKit tests), the current testsuite can be straightforwardly extended to be compatible with multifusion categories.

• There was a check where for a non-braided sector, the F-symbols should simply as type the sectorscalartype, but this is incorrect for product sectors consisting of braided and non-braided sectors. This could've already been tested with PlanarTrivial ⊠ ..., so I added such one to the sector list.
• In the value iterator, I removed the test that checked that the first in the sector values must be a unit. I don't see where that's a necessary condition, and it also doesn't hold currently in MultiTensorKit 😄.
• I turned the unitarity of F-symbols check into a function that's exported by the testsuite as well.
• I also changed the smallset function to take random sectors where possible, something I found to be necessary in a different context. I added the size argument, but I'm actually not sure if I'm able to access that if I load this test suite in a different package. I was just thinking of this because a generic multifusion category has a lot of sectors, and testing just 5 at a time which don't necessarily fuse to one another seemed like a recipe for testing absolutely nothing. This is part of why the tests I have currently in Finishing touches to a working MultiTensorKit code MultiTensorKit.jl#2 (as of now) look so custom compared to this test suite.
• I kept the old multifusion test file, removing duplicate tests. The remaining I left as a check that certain behavior of functions is as expected.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

Files with missing lines	Coverage Δ
src/timereversed.jl	91.66% <100.00%> (+0.36%)	⬆️

... and 4 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[lkdvos]

There was a check where for a non-braided sector, the F-symbols should simply as type the sectorscalartype,

I actually think this means there is an error in the implementation of sectorscalartype, since that should really hold. (which does make sense, I should have realized that this is not just the product of the sectorscalartypes of each of the factors)

I turned the unitarity of F-symbols check into a function that's exported by the testsuite as well.

Can you elaborate a bit on this?
While I am not entirely opposed to this, I don't like it too much that this is not consistent between the tests, with some of them being functions and others not.

I also changed the smallset function to take random sectors where possible, something I found to be necessary in a different context.

In principle it is also possible/recommended to overload this function specifically in these cases. Do you think we should just take a higher number by default?
I'm pretty sure the current tests don't actually take that long, so I have no issue with simply increasing the number either

[borisdevos]

I actually think this means there is an error in the implementation of sectorscalartype, since that should really hold. (which does make sense, I should have realized that this is not just the product of the sectorscalartypes of each of the factors)

I fixed this to check the braiding first.

I turned the unitarity of F-symbols check into a function that's exported by the testsuite as well.

Can you elaborate a bit on this?
While I am not entirely opposed to this, I don't like it too much that this is not consistent between the tests, with some of them being functions and others not.

In principle it is also possible/recommended to overload this function specifically in these cases. Do you think we should just take a higher number by default?

Quoting this both because the answer's the same. I want to make sure enough (and currently all) sector values are tested for unitarity and valid F-symbols, meaning I need to either overload the smallset to simply take length(values(...) or just manually loop over the sector values, and then easily call unitarity_test just like how pentagon_equation is called. Just to give an idea, the current A4Object multifusion sector has 174 sectors, and it's important to test all of them. I tried increasing the default size from 5 to 10, but it's rough for anything SU2Irrep related.

Edit: it's flown completely over my head that the pentagon test is just passing. Turns out true is just returned because there's no fusion channels to iterate over. I'm looking into this now

[borisdevos]

So I added some checks related to the colorings of multifusion sectors, but I only put these in the most important tests. In principle I could put these at every test that fuses at least two random objects pulled from smallset, but I refrained from doing so for now.

[lkdvos]

I'm not sure I am still following what you are saying, it seems like this PR is now doing quite a few very different things so it is a bit hard to keep track of what is going on.
Is this correct:

• You want to be able to check unitarity in testsuites of downstream packages, so you refactored that into a function.
• You fixed sectorscalartype for productsectors and added tests for this
• You did some things for time reversed values (because it made something fail, but that is not being tested? or is it?)
• You made some changes to improve the multifusion support of the testsuite?

Just for my sanity, it would be nice to split this all up in separate PRs, but if this is what is going on I did put some more comments here.

[lkdvos]

This still looks a bit off to me to be honest, I think in general this is still type-unstable (reshapedarray vs regular array), and now you have random elements that might have repeats (which is something I missed, apologies for that)

Given all of this, is it not simpler to keep the smallset function as it was before, and then only overload it for A4Object?

[borisdevos]

I can just overload if needed for A4Object. But there was actually an issue at the level of the TKS sectors. The thing I wanted to avoid was take always taking the first size sectors in SectorValues, which messed me up when working with irreps of the Heisenberg group whose first sectors (in my implementation) were all trivial characters, and the latter were non-trivial and actually failing tests. But this wasn't being captured with smallset only taking the trivial sectors. This is why I wanted to sample randomly from all available sectors in values(I).

At least with @test_inferred and @code_warntype the current smallset is type-stable, forcing it to be a vector instead of a potential matrix when values is finite in size.

[borisdevos]

All your bullet points are true.

• I want to export the unitarity test for manual testing in downstream packages. This is also somewhat in line with pentagon_equation being public (even though it's now from a test module instead of TKS itself)
• The sectorscalartype fix was necessary for testing IsingBimodule as it's unbraided, but PlanarTrivial could've captured this already.
• Of the four, only this third one about the time-reversed iteration thing is unrelated to the multifusion support I'm aiming for. This is only needed when I call collect on product sectors, like what's done with the new smallset. If/when I change smallset back to what it was, this is in principle not needed, but I don't see why we wouldn't want this functionality. It's maybe just misplaced in this PR.
• Yes :)

[borisdevos]

Put it temporarily to draft since I need to get the parse-show tests working in MultiTensorKit.

[github-actions]

Your PR no longer requires formatting changes. Thank you for your contribution!