tensorly / tensorly

TensorLy: Tensor Learning in Python.
http://tensorly.org
Other
1.51k stars 281 forks source link

PARAFAC2: Move normalization within CP, avoid unnecessary sign normalization in OP #523

Closed aarmey closed 9 months ago

aarmey commented 9 months ago
  1. The SVD interface flips the sign of the results by default, so that the results are more consistent. This is a useful and reasonable feature, but Orthogonal Procrustes only uses U @ Vh, so this has no effect. On indexing-expensive backends this is about 20% of the PARAFAC2 time, so it does provide an appreciable performance benefit.
  2. The CP implementations used within PARAFAC2 already implement normalization, so it seems beneficial to use that implementation, rather than performing it within PARAFAC2.

These are some simpler adjustments before I open pull requests for a line search routine and faster reconstruction error calculation, with more substantial performance improvements.

codecov[bot] commented 9 months ago

Codecov Report

Merging #523 (b1819fd) into main (6c7ecfb) will decrease coverage by 0.16%. Report is 26 commits behind head on main. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #523      +/-   ##
==========================================
- Coverage   86.94%   86.78%   -0.16%     
==========================================
  Files         120      118       -2     
  Lines        7535     7508      -27     
==========================================
- Hits         6551     6516      -35     
- Misses        984      992       +8     
Files Changed Coverage Δ
tensorly/decomposition/_parafac2.py 92.56% <100.00%> (-0.47%) :arrow_down:

... and 14 files with indirect coverage changes

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more