Closed aarmey closed 9 months ago
Merging #524 (554ad99) into main (6c7ecfb) will decrease coverage by
0.15%
. Report is 26 commits behind head on main. The diff coverage is100.00%
.
@@ Coverage Diff @@
## main #524 +/- ##
==========================================
- Coverage 86.94% 86.79% -0.15%
==========================================
Files 120 118 -2
Lines 7535 7515 -20
==========================================
- Hits 6551 6523 -28
- Misses 984 992 +8
Files Changed | Coverage Δ | |
---|---|---|
tensorly/decomposition/_parafac2.py | 93.75% <100.00%> (+0.72%) |
:arrow_up: |
... and 14 files with indirect coverage changes
:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more
Awesome, thanks @aarmey, merging!
In many (most?) applications of PARAFAC2, the slices of X are much larger than the rank along both dimensions. For very large slices, the majority of time is spent on any operations that touch the full X. This necessarily occurs in three places:
With 4, this can be avoided because the inner product operates with the projected tensor. Additionally, the second path of the previous if statement would only be used if the rank is greater than the size along C. In practice, this does not generally happen and, if it did, would be for a small dataset. Consequently, I removed the previous if statement and replaced it with this optional reuse of the projected tensor.
For very large slices of X (e.g., 4,000 x 8,000) this can save ~30% of the runtime because these three later multiplications dominate. It also tends to help more so with a line search routine, because you add an additional OP and reconstruction error calculation at the expense of avoiding another round of CP.