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

[REVIEW] PCA with sparse inputs #2392

Merged
merged 23 commits into from
Jun 17, 2020
Merged

[REVIEW] PCA with sparse inputs #2392

merged 23 commits into from
Jun 17, 2020

Conversation

divyegala
Copy link
Member

No description provided.

@divyegala divyegala requested a review from a team as a code owner June 10, 2020 22:58
@GPUtester
Copy link
Contributor

Please update the changelog in order to start CI tests.

View the gpuCI docs here.

@divyegala divyegala added 3 - Ready for Review Ready for review by team Cython / Python Cython or Python issue labels Jun 10, 2020
cjnolet
cjnolet requested changes Jun 11, 2020
View reviewed changes
Copy link
Member

@cjnolet cjnolet left a comment

Choose a reason for hiding this comment

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

Overall looking very good. I love the RawKernel! Just minor things really.

python/cuml/test/test_stats.py Outdated Show resolved Hide resolved
python/cuml/prims/stats/covariance.py Outdated Show resolved Hide resolved
python/cuml/decomposition/pca.pyx Show resolved Hide resolved
python/cuml/test/test_pca.py Show resolved Hide resolved
python/cuml/decomposition/pca.pyx Show resolved Hide resolved
python/cuml/decomposition/pca.pyx Outdated Show resolved Hide resolved
@cjnolet
Copy link
Member

cjnolet commented Jun 11, 2020

Also, we don't need to provide a lengthy breakdown of the potential instabilities here, but let's at least reference this as a potential problem: https://en.wikipedia.org/wiki/Covariance#Numerical_computation

@divyegala
Copy link
Member Author

rerun tests

@cjnolet cjnolet mentioned this pull request Jun 17, 2020
Closed
@cjnolet
Copy link
Member

cjnolet commented Jun 17, 2020

rerun tests

@divyegala divyegala mentioned this pull request Jun 17, 2020
Open
cjnolet
cjnolet approved these changes Jun 17, 2020
View reviewed changes
Copy link
Member

@cjnolet cjnolet left a comment

Choose a reason for hiding this comment

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

LGTM

@cjnolet cjnolet merged commit f2f2845 into rapidsai:branch-0.15 Jun 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cjnolet cjnolet cjnolet approved these changes

Assignees
No one assigned
Labels
3 - Ready for Review Ready for review by team Cython / Python Cython or Python issue
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@divyegala @GPUtester @cjnolet