Avoid copy in twist for tensors with bosonic braiding - #305
lkdvos merged 9 commits into
Conversation
Sorry, something went wrong.
Codecov Report❌ Patch coverage is 77.77778% with 6 lines in your changes missing coverage. Please review.
|
Sorry, something went wrong.
|
This feels like a pretty big breaking change and I'm a little scared of this introducing silent and subtle bugs. In principle, for the other "indexmanipulations" we have a copy = false keyword to indicate if data sharing with the input is allowed, which is probably the easiest solution. Unfortunately, if we then want to make this non-breaking we would have to set that to true by default, which is not in line with the other default values. The fastest way forward for tousle specific case is probably to just special case it in the reverse rules, which is in line with the forward rules special case. However, I think it would be nicest in the long run to just additionally include the keyword as a breaking change, and then possibly simplify the use in the contractions, with the next 0.16 release. I don't know what you think @Jutho ? |
Sorry, something went wrong.
|
We are anyway making breaking changes, I assume this one is quite innocent, so if it is bundled in a breaking release and listed as such, I do not see why we could not introduce the copy keyword with false als default value. |
Sorry, something went wrong.
|
Does anyone understand how this change causes the autodiff tests to fail? |
Sorry, something went wrong.
|
I am actually guessing it's an RNG thing, it looks like these tests are still a bit brittle :( |
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>
When profiling some AD-based optimizations, I noticed the twists that occur in the rrules of the tensor operations show up quite prominently sometimes. This was caused by the fact that by default twist copies its input before calling twist!, even for tensors with bosonic braiding for which twist! does nothing in the end.
Here I just added the check for trivial twists from twist! to twist to avoid a copy for trivial twists. I don't know if anything really relies on the fact that twist makes a copy though. If this is the case, maybe there's a different way to handle trivial twists in the tensor operations pullbacks themselves?