Skip to content

A few small fixes for multi-gpu PCA/TruncatedSVD#7560

Merged
rapids-bot[bot] merged 2 commits intorapidsai:release/25.12from
jcrist:fixup-sign-flip
Dec 3, 2025
Merged

A few small fixes for multi-gpu PCA/TruncatedSVD#7560
rapids-bot[bot] merged 2 commits intorapidsai:release/25.12from
jcrist:fixup-sign-flip

Conversation

@jcrist
Copy link
Copy Markdown
Member

@jcrist jcrist commented Dec 2, 2025

  • Don't branch the sign flipping behavior based on the version of sklearn installed. This somehow slipped through in Fix PCA sign flip #7331. We always want cuml behavior to be the same regardless of sklearn version - the only thing we branch on is the testing where we don't assert sign matches for sklearn < 1.5 (this matches the single-gpu testing strategy as well).
  • Adds a sync point in multi-gpu PCA before calling signFlipComponents. The multi-gpu implementation makes use of multiple streams, but before only the first stream was passed to signFlipComponents (without any sync beforehand) leading to potential stream ordering issues. It's hard to prove a negative, but with this change I can no longer reproduce an issue reported in rapids_singlecell. A similar fix isn't needed for TruncatedSVD since that implementation only uses one stream.

@jcrist jcrist self-assigned this Dec 2, 2025
@jcrist jcrist added the bug Something isn't working label Dec 2, 2025
@jcrist jcrist requested review from a team as code owners December 2, 2025 23:22
@jcrist jcrist added the Dask / cuml.dask Issue/PR related to Python level dask or cuml.dask features. label Dec 2, 2025
@jcrist jcrist requested a review from viclafargue December 2, 2025 23:22
@jcrist jcrist added the non-breaking Non-breaking change label Dec 2, 2025
@github-actions github-actions Bot added Cython / Python Cython or Python issue CUDA/C++ labels Dec 2, 2025
@jcrist jcrist requested review from csadorf and removed request for viclafargue December 2, 2025 23:22
@jcrist jcrist changed the title A few small fixes for multi-gpu PCA/TruncatedSVD` A few small fixes for multi-gpu PCA/TruncatedSVD Dec 2, 2025
@jcrist
Copy link
Copy Markdown
Member Author

jcrist commented Dec 3, 2025

/merge

@rapids-bot rapids-bot Bot merged commit ebb8e80 into rapidsai:release/25.12 Dec 3, 2025
218 of 220 checks passed
@jcrist jcrist deleted the fixup-sign-flip branch December 3, 2025 15:25
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++ Cython / Python Cython or Python issue Dask / cuml.dask Issue/PR related to Python level dask or cuml.dask features. non-breaking Non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants