Generalising functions to support GenericUnit
#291
Conversation
…concerning sectors + some more unitspace
…itspace` and `removeunitspace`
|
Your PR no longer requires formatting changes. Thank you for your contribution! |
Codecov Report❌ Patch coverage is
... and 35 files with indirect coverage changes 🚀 New features to boost your workflow:
|
Co-authored-by: Jutho <[email protected]>
lkdvos
left a comment
lkdvos
left a comment
There was a problem hiding this comment.
I think I would be happy to merge this as-is. Thanks for the great effort @borisdevos, another large PR in the books!
Let's wait for @Jutho's approval and get this merged.
| while isempty(⊗(out1...)) | ||
| out1 = random_fusion(I, Val(N)) | ||
| out1 = Base.setindex(out1, in2, i) | ||
| end |
There was a problem hiding this comment.
This is another while loop that could potentially become long-lived (maybe not, I really don't have any intuition about multi-fusion categories). I am definitely fine leaving it as is for the time being, as clearly the tests work.
But maybe it could be helpful to add a small # TODO comment here as a reminder to think about whether there is a better strategy in the future. Maybe we can implement some helpful tools to generate random fusion trees with given constraints, like a fixed sector at position i, or a fixed coupled sector, or ...
There was a problem hiding this comment.
Since random_fusion now manually cuts off after 20 loops, the tests shouldn't get stuck here too long at any point, especially with the relatively small Ns we consider in the tests. But a TODO is added for potential future improvements :)
There was a problem hiding this comment.
I don't see how the cutoff in random_fusion would prevent this while loop from running an infinite number of times? But that's not important right now; it is good to have the TODO present.
There was a problem hiding this comment.
No you're right, I confused myself.
|
I am still confused by the behavior of the tests on nightly; something I have been ignoring for the last few PRs. Both the "other" group and the "autodiff" group print the following error from loading Zygote: ERROR: LoadError: ParseError:
# Error @ /home/runner/.julia/packages/Zygote/55SqB/src/lib/array.jl:609:19
# to create a Diagonal
tr(x), function (Δ::Number)
# └─────────┘ ── Invalid signature in function definitionNonetheless, the "autodiff" tests run and succeed, whereas the "other" tests fail because of not being able to precompile Zygote. Do you have any idea @lkdvos or @kshyatt ? |
|
Looking at the quoted source code here: https://github.com/FluxML/Zygote.jl/blob/35acbf3ef2b3ab7fbc5ba41cac04eab26ffe1759/src/lib/array.jl#L609 I wonder if nightly changed parsing rules such that this syntax is no longer valid, but it seems like the entire Zygote ecosystem is somewhat dead since even 1.12 doesn't really work with it. The autodiff seems to still fail for the CompatHelper runs, although that looks to me like its just a badly conditioned matrix due to RNG in the |
|
It seems TensorKitSectors doesn't export using TensorKitSectors: HasBraiding |
|
Well spotted that |
|
|
* initial basic design SectorVector * some additional functionality * relax `foreachblock` signature * replace `SectorDict` with `SectorVector` for eig/svdvals * export `svd_vals` * clean up SectorVector design * small fix * add finitedifferences support * update changelog * some simplifications and extensions * some further fixes * some more fixes * update dates --------- Co-authored-by: Jutho Haegeman <[email protected]>
This PR redoes parts of #263, but now with
UnitStyleto more cleanly deal with multifusion categories.