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

[FEA] Removing "randomized" solver option from PCA and tSVD #98

Closed
oyilmaz-nvidia opened this issue Jan 15, 2019 · 1 comment
Closed

[FEA] Removing "randomized" solver option from PCA and tSVD #98

oyilmaz-nvidia opened this issue Jan 15, 2019 · 1 comment
Labels
feature request New feature or request

Comments

@oyilmaz-nvidia
Copy link
Contributor

Is your feature request related to a problem? Please describe.
Randomized option is not supported due to its high discrepancy. It was added to be compatible with SKL.

Describe the solution you'd like
Should be removed from python wrapper.
@oyilmaz-nvidia oyilmaz-nvidia added feature request New feature or request ? - Needs Triage Need team to review and classify labels Jan 15, 2019
@oyilmaz-nvidia oyilmaz-nvidia changed the title [FEA] Removing "randomized" solver option from PCA [FEA] Removing "randomized" solver option from PCA and tSVD Jan 15, 2019
@dantegd dantegd removed the ? - Needs Triage Need team to review and classify label Jan 17, 2019
@dantegd dantegd mentioned this issue Jan 23, 2019
Merged
@dantegd
Copy link
Member

dantegd commented Jan 23, 2019

Solved by #125

@dantegd dantegd closed this as completed Jan 23, 2019
@zbjornson zbjornson mentioned this issue Sep 17, 2020
Closed
zbjornson added a commit to zbjornson/cuml that referenced this issue Sep 21, 2020
@zbjornson
Remove remaining parts of RANDOMIZED PCA solver
7dc4f45 
This was partially removed in rapidsai#98/rapidsai#125. Only COV_EIG_DQ and COV_EIG_JACOBI are actually supported.

`random_state` is only used by ARPACK and randomized solvers, so can be remove also.
dantegd pushed a commit that referenced this issue Sep 22, 2020
@zbjornson
[REVIEW] Clean up paramsPCA (#2850)
1ded565 
* Remove remaining parts of RANDOMIZED PCA solver

This was partially removed in #98/#125. Only COV_EIG_DQ and COV_EIG_JACOBI are actually supported.

`random_state` is only used by ARPACK and randomized solvers, so can be remove also.

* Remove unused bool paramsSolver.trans_input

Unused, and confusing because there are lots of PCA functions with `float* trans_input` params.

* Remove unused max_sweeps and iterated_power params

Only `n_iterations` is used.

Ref #2833

* Add TODO for unused copy param

* Update changelog
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
feature request New feature or request
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@dantegd @oyilmaz-nvidia