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.

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

BUG: remove sample parameter from pca call to mean #5980

Merged
merged 3 commits into from
Jul 26, 2024

Conversation

mfoerste4
Copy link
Contributor

@mfoerste4 mfoerste4 commented Jul 25, 2024

This fixes a call to raft::stats::mean() deactivating the sample parameter in the pca code.

CC @dantegd

@mfoerste4 mfoerste4 requested a review from a team as a code owner July 25, 2024 18:09
@mfoerste4 mfoerste4 self-assigned this Jul 25, 2024
@mfoerste4 mfoerste4 added bug Something isn't working non-breaking Non-breaking change and removed CUDA/C++ labels Jul 25, 2024
@dantegd
Copy link
Member

dantegd commented Jul 25, 2024

/merge

@mfoerste4
Copy link
Contributor Author

@dantegd @cjnolet , I had to modify another usage 'sample' within mean/variance to align results with scikitlearn for TruncatedSVD. I hope this is the last one...

@mfoerste4
Copy link
Contributor Author

The reason why these issues did not result in failures before is that the sample parameter was silently ignored for column based data in raft. Now that behavior was fixed in rapidsai/raft#2381 which now reveals issues downstream.

@vyasr
Copy link
Contributor

vyasr commented Jul 26, 2024

/merge

@rapids-bot rapids-bot bot merged commit 3e73853 into rapidsai:branch-24.08 Jul 26, 2024
61 of 63 checks passed
@mfoerste4 mfoerste4 deleted the fix_mean_in_pca branch July 26, 2024 10:53
jameslamb pushed a commit to jameslamb/cuml that referenced this pull request Jul 26, 2024
This fixes a call to `raft::stats::mean()` deactivating the sample parameter in the pca code. 

CC @dantegd

Authors:
  - Malte Förster (https://github.com/mfoerste4)

Approvers:
  - Bradley Dice (https://github.com/bdice)
  - Dante Gama Dessavre (https://github.com/dantegd)
  - Corey J. Nolet (https://github.com/cjnolet)

URL: rapidsai#5980
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working CUDA/C++ non-breaking Non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants