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

Inflated runtime on (s)PCA #250

Closed
Max-Bladen opened this issue Oct 6, 2022 · 0 comments
Closed

Inflated runtime on (s)PCA #250

Max-Bladen opened this issue Oct 6, 2022 · 0 comments
Assignees
@Max-Bladen
Labels
bug Something isn't working

Comments

@Max-Bladen
Copy link
Collaborator

🐞 Describe the bug:

Due to the way in which the base::svd() function undergoes decomposition, an unnecessary (and excessive) amount of singular vector (left and right) and eigenvalues are returned from the function. These are then trimmed to include only the useful values, making these extra vectors pointless.

🤔 Expected behavior:
By adjusting the svd() function, runtime and memory usage efficiency could be improved.

💡 Possible solution:
Using the RSpectra package svds() function, the number of vectors and eigenvalues to calculate can be stipulated. By calculating only the necessary quantity, runtime and memory efficiency should be improved
@Max-Bladen Max-Bladen added the bug Something isn't working label Oct 6, 2022
@Max-Bladen Max-Bladen linked a pull request Oct 6, 2022 that will close this issue
(s)PCA Build using RSpectra::svds() #251
Closed
@Max-Bladen Max-Bladen assigned Max-Bladen and unassigned aljabadi Oct 6, 2022
Max-Bladen added a commit that referenced this issue Oct 6, 2022
@Max-Bladen
Fix for Issue #250
e00c586 
docs: updated DESCRIPTION with new import
Max-Bladen added a commit that referenced this issue Oct 6, 2022
@Max-Bladen
Fix for Issue #250
c4f828f 
refactor: import and utilise RSpectra::svds() instead of base::svd().

svds() only calculates the required number of left and right singular vector as eigenvalues. svd() calculates an excessive amount which can cause runtime inflation for large datasets
Max-Bladen added a commit that referenced this issue Oct 6, 2022
@Max-Bladen
Fix for Issue #250
b6afcba 
refactor: import and utilise RSpectra::svds() instead of base::svd(). This is now also applied to spca()
Max-Bladen added a commit that referenced this issue Oct 6, 2022
@Max-Bladen
Fix for Issue #250
bfa34aa 
docs: updated NAMESPACE and DESCRIPTION to include new dependency
Max-Bladen added a commit that referenced this issue Oct 6, 2022
@Max-Bladen
Fix for Issue #250
81399a0 
docs: updated DESCRIPTION with new import
Max-Bladen added a commit that referenced this issue Oct 6, 2022
@Max-Bladen
Fix for Issue #250
62c7f10 
fix: when ILR/CLR was used, X was of invalid class. Added check to ensure `svds()` can operate on X
Max-Bladen added a commit that referenced this issue Oct 7, 2022
@Max-Bladen
Fix for Issue #250
0adc5cc 
fix: when ILR/CLR was used, X was of invalid class. Added check to ensure `svds()` can operate on X. Implemented this for `spca()`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Max-Bladen Max-Bladen

Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

(s)PCA Build using RSpectra::svds() mixOmicsTeam/mixOmics
2 participants
@aljabadi @Max-Bladen