Skip to content

Port sklearn.decomposition to new proxy estimator structure#6695

Merged
rapids-bot[bot] merged 6 commits intorapidsai:branch-25.06from
jcrist:port-sklearn-decomposition
May 13, 2025
Merged

Port sklearn.decomposition to new proxy estimator structure#6695
rapids-bot[bot] merged 6 commits intorapidsai:branch-25.06from
jcrist:port-sklearn-decomposition

Conversation

@jcrist
Copy link
Copy Markdown
Member

@jcrist jcrist commented May 12, 2025

This PR:

  • Ports PCA and TruncatedSVD to subclass from Base & InteropMixin instead of UniversalBase
  • Ports our wrappers in cuml.accel to subclass from ProxyBase instead of ProxyMixin
  • Updates ProxyBase to handle the sparse support checks in the base class rather than delegating these to the _gpu_* override methods. These checks are common, for now it makes sense to keep them in the base class (this is also how the old dispatch_func method worked).
  • Updates the xfail list accordingly.

Fixes #6706.

@jcrist jcrist requested a review from a team as a code owner May 12, 2025 20:37
@jcrist jcrist requested review from dantegd and teju85 May 12, 2025 20:37
@github-actions github-actions Bot added the Cython / Python Cython or Python issue label May 12, 2025
@jcrist jcrist added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change cuml-accel Issues related to cuml.accel labels May 12, 2025
@jcrist jcrist self-assigned this May 12, 2025
@jcrist jcrist force-pushed the port-sklearn-decomposition branch from e2e6d2b to 5c39a7e Compare May 13, 2025 02:57
@csadorf csadorf self-requested a review May 13, 2025 16:39
Copy link
Copy Markdown
Contributor

@csadorf csadorf left a comment

Choose a reason for hiding this comment

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

LGTM! I'm slightly surprised by the sklearn test regressions. Are those new tests, because we support more or what's going on?

Comment thread python/cuml/cuml/accel/tests/scikit-learn/xfail-list.yaml
@jcrist
Copy link
Copy Markdown
Member Author

jcrist commented May 13, 2025

/merge

@rapids-bot rapids-bot Bot merged commit 7ec19ec into rapidsai:branch-25.06 May 13, 2025
93 of 94 checks passed
@jcrist jcrist deleted the port-sklearn-decomposition branch May 13, 2025 21:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cuml-accel Issues related to cuml.accel 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.

Port sklearn.decomposition proxies to use ProxyBase

3 participants