Generalising functions to support GenericUnit - #291
borisdevos merged 101 commits into
Conversation
Sorry, something went wrong.
…concerning sectors + some more unitspace
…itspace` and `removeunitspace`
|
Your PR no longer requires formatting changes. Thank you for your contribution! |
Sorry, something went wrong.
Codecov Report❌ Patch coverage is 95.31250% with 3 lines in your changes missing coverage. Please review.
... and 35 files with indirect coverage changes 🚀 New features to boost your workflow:
|
Sorry, something went wrong.
Co-authored-by: Jutho <Jutho@users.noreply.github.com>
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.
Sorry, something went wrong.
| 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 ...
Sorry, something went wrong.
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 :)
Sorry, something went wrong.
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.
Sorry, something went wrong.
There was a problem hiding this comment.
No you're right, I confused myself.
Sorry, something went wrong.
|
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 ? |
Sorry, something went wrong.
|
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 rrule(inv, ...) test |
Sorry, something went wrong.
|
It seems TensorKitSectors doesn't export HasBraiding so the tests are failing on this. Maybe you can manually import this here? And we might consider explicitly exporting this in the future. So for now, you will have to add this to the relevant test files using TensorKitSectors: HasBraiding |
Sorry, something went wrong.
|
Well spotted that HasBraiding is exported from TensorKitSectors but needed to be reexported from TensorKit. I think this is now fully ready so it can be merged after the tests have completed. |
Sorry, something went wrong.
|
|
Sorry, something went wrong.
* 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 <jutho.haegeman@ugent.be>
This PR redoes parts of #263, but now with UnitStyle to more cleanly deal with multifusion categories.