-
Notifications
You must be signed in to change notification settings - Fork 55
IsingBimodule for testing multifusion category within TensorKit
#263
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
Additional details and impacted files@@ Coverage Diff @@
## master #263 +/- ##
==========================================
+ Coverage 83.39% 83.49% +0.10%
==========================================
Files 44 45 +1
Lines 5757 5823 +66
==========================================
+ Hits 4801 4862 +61
- Misses 956 961 +5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
IsingBimodule for testing multifusion category within TensorKitIsingBimodule for testing multifusion category within TensorKit
This reverts commit 772452f.
…oris-isingbimod
| sector = leftone(first(sectors(S))) | ||
| return spacetype(S)(sector => 1) | ||
| end | ||
| leftoneunit(S::GradedSpace{I}) where {I<:Sector} = oneunit(typeof(S)) |
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.
What was wrong with the previous implementations? This looks like it will always fail for multifusion things
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 wanted left/rightoneunit to also work on empty spaces, because oneunit does so. This will indeed fail for multifusion things, hence the specialisation.
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 see, but what do you think about this then:
function leftoneunit(V::GradedSpace{I}) where {I<:Sector}
s = sectors(V)
u = leftoneunit(isempty(s) ? I : first(s))
return spacetype(V)(u => 1)
endThere 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.
It's not great if we have to specialize everything specifically for Vect[IsingBimodule], since this would mean we have to redo all that work for any multifusion category. In that sense, we would either need a fusion type that specifies this, such that we can specialize properly, or write these methods in a generic way that works for everything
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 sure what you mean by redoing all that work. IsingBimodule should be the only multifusion category tested within TensorKit itself. This infrastructure already exists in MultiTensorKit (albeit in a PR for now), which will support all the other multifusion categories.
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.
What I mean is that if all we are testing is the specializations that are written solely for IsingBimodule, which is used mainly for testing, this doesn't actually help all that much, since it would have to be both reimplemented and retested for other multifusion categories as well
| end | ||
| @timedtestset "Full trace: test self-consistency" begin | ||
| t = rand(ComplexF64, V1 ⊗ V2' ⊗ V2 ⊗ V1') | ||
| t2 = permute(t, ((1, 2), (4, 3))) |
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.
Is there a reason for removing this t2 call?
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 removed all the permutations in tests which weren't explicitely testing for permutations, which I had to do anyway for the multifusion tests. This would allow for testing anyonic sectors if we decide to expand the space list.
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.
That's fair. I think I would like to keep both though, to actually have coverage of as much as possible. Probably it is better to just separate these things out a bit further.
| @planar s2 = t[a b; a b] | ||
| @planar t3[a; b] := t[a c; b c] | ||
| @planar s3 = t3[a; a] |
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.
It's definitely good to test both @planar and @tensor, but we probably want to test both?
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.
This was also with expanding to non-symmetric braidings in mind. I could put a SymmetricBraiding check here to then evaluate @tensor if you want?
| allequal(a.col for a in sectors(S)) || | ||
| throw(ArgumentError("sectors of $S do not have the same rightone")) | ||
|
|
||
| sector = rightone(first(sectors(S))) |
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.
| allequal(a.col for a in sectors(S)) || | |
| throw(ArgumentError("sectors of $S do not have the same rightone")) | |
| sector = rightone(first(sectors(S))) | |
| allequal(rightone, sectors(S)) || throw(throw(ArgumentError("sectors of $S do not have the same rightone")) | |
| return spacetype(S)(rightone(first(list))=>1) |
* change TensorKitSectors compat * add `unitspace` * one -> unit + left/rightone -> left/rightunit + isone -> isunit when concerning sectors + some more unitspace * change conj to dual for sectors * export new functions from TensorKitSectors * introduce `zerospace` to replace `zero` of a space * add `oneunit` for type of space * format * minor changes from #263 * use the const `TK` in tests where appropriate * `otimes` between tensormaps to account for `sectorscalartype` * formatter * update frobeniusschur * reimplement otimes * remove todo comment that breaks docstring * fix test * docs fixes * Reorganize exports in TensorKit.jl * More reorganization of exports * Fix typo in export reorganization --------- Co-authored-by: Lukas Devos <[email protected]> Co-authored-by: Jutho <[email protected]>
This PR makes use of the newly introduced
IsingBimodulemultifusion category in QuantumKitHub/TensorKitSectors.jl#14 to test the changes in #247 within TensorKit itself. This was needed because MultiTensorKit depends on TensorKit itself.What's new:
leftoneunitandrightoneunitare introduced to naturally extendoneunit, but not forType{GradedSpace{...}}because it needs to know the objects within theGradedSpace.artin_braidhad anisonecheck replaced with a multifusion-friendly check (although this might be excessive, since as of now we don't consider braiding within multifusion categories)one(IsingBimodule)IsingBimodulefusion trees, spaces and tensorsI left a bunch of comments everywhere to remind me of certain choices I made in the implementation. It would be nice if these were addressed. I can clean most of them afterwards, but should probably keep some of them, or at least the ones in the tests to help whoever might need to maintain those in the future.
As I started on this before #247 got merged, its commit history is here as well, and I'm not sure what kind of rebase I need to do to get rid of it. I tried something and files seemed to want to vanish, so I aborted that attempt 😅 Those luckily don't appear in the files changed though.
IsingBimodulestill needs to get renamed to that, so currently everything isIsingBimod. I'm assuming we're waiting on QuantumKitHub/TensorKitSectors.jl#19 to merge to release v0.2 though.Edit: I forgot to mention, but currently there's one thing broken which I can't find the error in. It appears in the full trace test. Ignoring the part where I don't know what to do with tracing module legs, it seems that the behavior is different for the two fusion categories inIsingBimodule. In particular, the full trace test passes for the "C" fusion category, but not for "D", the latter tracing to zero via@planar. I checked this quickly in MultiTensorKit with the multifusion category I got there, and it is consistent there.Edit 2: I also forgot that @Jutho and I briefly discussed trying to increase code coverage, most prominently for
GenericFusionbut also here and there for other functions. I can do this in a separate PR.