Conversation
|
Really excited to have this in TensorLy @aarmey, it's a neat method! |
JeanKossaifi
left a comment
There was a problem hiding this comment.
Looks great! Just left some notes in the code.
|
Thanks again @aarmey - I can't seem to be able to relaunch the tests nor check the logs for these runs as they were too long ago |
|
Thanks for pointing this out, @JeanKossaifi. I lost track of this. There are a few more changes for me to make, then I'll hopefully tag you for a review by the end of the year. |
|
Sounds great, thank you! |
- Rename X to matrices_tensor and update internals - Correct tl.copy to use the input tensor - Improve docstring and add References section - Update tests to import from jointdiag module
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #571 +/- ##
==========================================
+ Coverage 88.07% 88.41% +0.33%
==========================================
Files 132 134 +2
Lines 7943 8034 +91
==========================================
+ Hits 6996 7103 +107
+ Misses 947 931 -16 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@JeanKossaifi I think this is now thoroughly cleaned up and working. |
This adds a function for deriving an approximate joint diagonalization for a set of matrices. This function is the basis of solvers that derive PARAFAC factorization by diagonalizing the Tucker decomposition. By using only a few iterations, this can also be used to improve the starting estimate for initializing PARAFAC.