Skip to content

reduce memory_footprint for sparse PCA transform - #5964

Merged
rapids-bot[bot] merged 3 commits into
NVIDIA:branch-24.08from
Intron7:sparse_pca-update
Jul 28, 2024
Merged

rapids-bot[bot] merged 3 commits into
NVIDIA:branch-24.08from
Intron7:sparse_pca-update

Conversation

@Intron7

@Intron7 Intron7 commented Jul 12, 2024 •

Copy link
Copy Markdown
Contributor

The sparse PCA still densified X during the transform step. This defeats the purpose of a sparse PCA in a sense. However

precomputed_mean_impact = self.mean_ @ self.components_.T
mean_impact = cp.ones((X.shape[0], 1)) @ precomputed_mean_impact.reshape(1, -1)
X_transformed = X.dot(self.components_.T) -mean_impact

is the same as

X = X - self.mean_
X_transformed = X.dot(self.components_.T)

The new implementation is faster (but mainly due to the fact that we don't have to rely on cupy's to_array()) and uses a lot less memory.

@Intron7
Intron7 requested a review from a team as a code owner July 12, 2024 14:35
@copy-pr-bot

copy-pr-bot Bot commented Jul 12, 2024

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@github-actions github-actions Bot added the Cython / Python Cython or Python issue label Jul 12, 2024
@lowener

lowener commented Jul 15, 2024

Copy link
Copy Markdown
Contributor

/ok to test

@lowener lowener added 3 - Ready for Review Ready for review by team improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Jul 15, 2024
@Intron7

Intron7 commented Jul 15, 2024

Copy link
Copy Markdown
Contributor Author

ok I added the right year in the cupyright

@lowener

lowener commented Jul 15, 2024

Copy link
Copy Markdown
Contributor

/ok to test

@Intron7

Intron7 commented Jul 17, 2024

Copy link
Copy Markdown
Contributor Author

I think I can't fix the issue on the PR since it's in regards to the new layout

@dantegd

dantegd commented Jul 17, 2024

Copy link
Copy Markdown
Contributor

@Intron7 yeah, I think the issue is that we just merged the PR to nest the python package one level lower #5944 so it should be an easy resolution of conflict I hope

@Intron7 Intron7 closed this Jul 18, 2024
@Intron7
Intron7 force-pushed the sparse_pca-update branch from 121945b to 99b20f6 Compare July 18, 2024 10:07
@Intron7 Intron7 reopened this Jul 18, 2024
@Intron7

Intron7 commented Jul 18, 2024

Copy link
Copy Markdown
Contributor Author

@lowener I merged in the updated main branch

@dantegd

dantegd commented Jul 20, 2024

Copy link
Copy Markdown
Contributor

/ok to test

@dantegd

dantegd commented Jul 28, 2024

Copy link
Copy Markdown
Contributor

/ok to test

@dantegd dantegd changed the title reduce memory_footprint for transform reduce memory_footprint for sparse PCA transform Jul 28, 2024
@dantegd

dantegd commented Jul 28, 2024

Copy link
Copy Markdown
Contributor

/ok to test

@dantegd

dantegd commented Jul 28, 2024

Copy link
Copy Markdown
Contributor

/merge

@rapids-bot
rapids-bot Bot merged commit d4535d2 into NVIDIA:branch-24.08 Jul 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3 - Ready for Review Ready for review by team Cython / Python Cython or Python issue improvement Improvement / enhancement to an existing function non-breaking Non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants