Conversation
Codecov ReportAttention: Patch coverage is
🚀 New features to boost your workflow:
|
|
Update, it seems like the intermediate permutations actually make a huge difference, I found a different order (technically due to @ogauthe) that is another factor 2 faster. I'll look into how this can be implemented, but also how this could be automated in the future. # PR state now
BenchmarkTools.Trial: 1 sample with 1 evaluation per sample.
Single result which took 6.661 s (0.12% GC) to evaluate,
with a memory estimate of 6.50 GiB, over 88989 allocations.
# PR state with updated intermediate permutations
BenchmarkTools.Trial: 2 samples with 1 evaluation per sample.
Range (min … max): 2.990 s … 3.086 s ┊ GC (min … max): 1.56% … 4.75%
Time (median): 3.038 s ┊ GC (median): 3.18%
Time (mean ± σ): 3.038 s ± 68.183 ms ┊ GC (mean ± σ): 3.18% ± 2.25%
█ █
█▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁█ ▁
2.99 s Histogram: frequency by time 3.09 s <
Memory estimate: 6.49 GiB, allocs estimate: 21237. |
pbrehmer
left a comment
There was a problem hiding this comment.
Thanks a lot for taking care of this! I really wasn't aware of the big difference the intermediate permutations make. Let me know when I should review again or if I can help anywhere.
|
This seems to only matter for non-abelian tensors however, so there is still some interesting interplay going on here. I'll keep you posted |
|
I'm glad you found out the explanation. I expect this effect to matter for all tensors, but indeed it will be more important for non-abelian. On my benchmark, frostspin was slightly faster in the Trivial case. With SU(2), reaching the asymptotic behavior where contraction dominates requires very high bond dimensions. |
|
I have now updated all the PEPS corner contractions appropriately. |
pbrehmer
left a comment
There was a problem hiding this comment.
Really impressive improvement! Thanks also for including the benchmark study - that might become useful in the future in case we want to set up a proper benchmark suite for PEPSKit.
* Store `dir` in `EnlargedCorner` * Manually fix enlarged corner contractions * Update contractions --- Co-authored-by: Olivier Gauthe <olivier.gauthe.2011+github@polytechnique.org>
* Store `dir` in `EnlargedCorner` * Manually fix enlarged corner contractions * Update contractions --- Co-authored-by: Olivier Gauthe <olivier.gauthe.2011+github@polytechnique.org>
* Store `dir` in `EnlargedCorner` * Manually fix enlarged corner contractions * Update contractions --- Co-authored-by: Olivier Gauthe <olivier.gauthe.2011+github@polytechnique.org>
Fixes #213 .
This manually fixes the contraction order for the enlarged corners. While it would be great to avoid having to manually check the optimal orders every time, at least for now it seems reasonable to manually fix some of them.
In particular, the
@autooptcurrently has absolutely no way of taking into account that some contraction orders that have equal cost might have different subleading costs due to the permutations, which I'm not entirely sure how to fix.Additionally this fixes something that has been bothering me for a while: the enlarged corners now actually keep track of which one they are, so the
TensorMap(Q::EnlargedCorner)no longer needs to get an additional argument.Performance-wise, I ended up trying a bunch of orders and ended up with the results that contracting first the edges, then the bra and then the ket ends up on top consistently. With the spaces for SU(2) as linked in the parent issue I end up with:
I did verify that these results are consistent for the other sizes as well.