Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Features/1458 add incremental SVD/PCA #1629

Merged
merged 38 commits into from
Nov 28, 2024
Merged

Features/1458 add incremental SVD/PCA #1629

merged 38 commits into from
Nov 28, 2024

Conversation

mrfh92
Copy link
Collaborator

@mrfh92 mrfh92 commented Aug 22, 2024 •
edited
Loading

Due Diligence

  • General:
  • Implementation:
    • unit tests: all split configurations tested
    • unit tests: multiple dtypes tested
    • documentation updated where needed

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

@mrfh92 mrfh92 self-assigned this Aug 22, 2024
@mrfh92 mrfh92 added ESAPCA relevant for the ESA-funded project "ESAPCA" linalg labels Aug 22, 2024
@mrfh92 mrfh92 marked this pull request as ready for review August 22, 2024 11:33
@github-actions GitHub Actions
Copy link
Contributor

Thank you for the PR!

Hoppe and others added 3 commits August 22, 2024 13:45
went on with tests
af89e44
@mrfh92
Merge branch 'main' into features/1458-Add_incremental_SVD/PCA
9692603
finalized implementation of iSVD for the moment; case old-split=0, ne…
19154a5 
…w-split=1 needs to be ruled out for the moment due to numerical instabilities of the combination of the respective algorithms.
@github-actions GitHub Actions
Copy link
Contributor

Thank you for the PR!

@github-actions GitHub Actions
Copy link
Contributor

Thank you for the PR!

2 similar comments
@github-actions GitHub Actions
Copy link
Contributor

Thank you for the PR!

@github-actions GitHub Actions
Copy link
Contributor

Thank you for the PR!

@github-actions GitHub Actions
Copy link
Contributor

Thank you for the PR!

1 similar comment
@github-actions GitHub Actions
Copy link
Contributor

Thank you for the PR!

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Sep 2, 2024

Thank you for the PR!

@mrfh92 mrfh92 requested a review from JuanPedroGHM October 8, 2024 09:20
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Oct 8, 2024

Thank you for the PR!

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Oct 9, 2024

Thank you for the PR!

@github-actions GitHub Actions
Copy link
Contributor

Thank you for the PR!

@github-actions GitHub Actions
Copy link
Contributor

Thank you for the PR!

@github-actions GitHub Actions
Copy link
Contributor

Thank you for the PR!

@ClaudiaComito ClaudiaComito added this to the 1.6 milestone Nov 11, 2024
@github-actions GitHub Actions
Copy link
Contributor

Thank you for the PR!

@mtar mtar added PR talk and removed PR talk labels Nov 18, 2024
@ClaudiaComito ClaudiaComito removed their request for review November 18, 2024 09:11
JuanPedroGHM
JuanPedroGHM requested changes Nov 20, 2024
View reviewed changes
Copy link
Member

@JuanPedroGHM JuanPedroGHM left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think iSVD could benefit from a full usage example somewhere, either in the benchmarks or the tests, that we could point people too. The fact that you need to have already done the SVD of a partial matrix, or that partial_fit needs to be used, could generate confusion. A simple example of it in use could help a lot in the future.

heat/core/linalg/svdtools.py Outdated Show resolved Hide resolved
heat/core/linalg/tests/test_svdtools.py Outdated Show resolved Hide resolved
heat/decomposition/tests/test_pca.py Outdated Show resolved Hide resolved
@github-actions GitHub Actions
Copy link
Contributor

Thank you for the PR!

@github-actions GitHub Actions
Copy link
Contributor

Thank you for the PR!

@mrfh92
Copy link
Collaborator Author

mrfh92 commented Nov 25, 2024

I have added a benchmark for IncrementalPCA that also highlights how this function could be used. I also put a reference in the docs of IncrementalPCA.

@github-actions GitHub Actions
Copy link
Contributor

Thank you for the PR!

@mrfh92
Copy link
Collaborator Author

mrfh92 commented Nov 25, 2024

@JuanPedroGHM I have turned off the benchmark_pr as it does not work at the moment, but except of that I guess I have addressed all comments

@mrfh92 mrfh92 requested a review from JuanPedroGHM November 25, 2024 16:04
@mrfh92 mrfh92 merged commit c23428e into main Nov 28, 2024
51 of 52 checks passed
@mrfh92 mrfh92 deleted the features/1458-Add_incremental_SVD/PCA branch November 28, 2024 15:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JuanPedroGHM JuanPedroGHM JuanPedroGHM approved these changes

Assignees

@mrfh92 mrfh92

Labels
ESAPCA relevant for the ESA-funded project "ESAPCA" high-level functions High-level machine-learning algorithms linalg
Projects
None yet
Milestone
1.6
Development

Successfully merging this pull request may close these issues.

Add incremental SVD/PCA
4 participants
@mrfh92 @JuanPedroGHM @mtar @ClaudiaComito