-
Notifications
You must be signed in to change notification settings - Fork 54
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Features/1458 add incremental SVD/PCA #1629
base: main
Are you sure you want to change the base?
Conversation
…e' into features/ESAPCA
Thank you for the PR! |
…w-split=1 needs to be ruled out for the moment due to numerical instabilities of the combination of the respective algorithms.
Thank you for the PR! |
… for iSVD and iPCA; debugging; improved test coverage
Thank you for the PR! |
2 similar comments
Thank you for the PR! |
Thank you for the PR! |
Thank you for the PR! |
1 similar comment
Thank you for the PR! |
Thank you for the PR! |
Thank you for the PR! |
…lmholtz-analytics/heat into features/1458-Add_incremental_SVD/PCA
…d_incremental_SVD/PCA
Thank you for the PR! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, but we have to make sure to merge randomized SVD first, and then solve all the merge conflicts that will appear from this. I'm guessing this was branched from that, but some of the functions have been renamed.
Also, it would be nice to have benchmarks for randomized SVD and incremental SVD.
Thank you for the PR! |
…d_incremental_SVD/PCA
@JuanPedroGHM @ClaudiaComito I have merged the rSVD and resolved the conflicts. Now it should be possible to merge after merging rSVD without conflicts. |
@JuanPedroGHM You're right regarding the benchmarks. I have added this as todo to the corresponding Issue #1615 |
Thank you for the PR! |
Benchmarks results - Sponsored by perun
Grafana Dashboard |
Thank you for the PR! |
Thank you for the PR! |
Thank you for the PR! |
Thank you for the PR! |
Thank you for the PR! |
Due Diligence
Description
Issue/s resolved: #1458
(Should be merged after #1561 as it already contains these changes)
Changes proposed:
Adds incremental SVD (see M. Brand, Fast low-rank modifications of the thin singular value decomposition, Linear Algebra and its Applications 415 (2006)) and corresponding interface for PCA
Type of change
new feature
Does this change modify the behaviour of other functions? If so, which?
no