Skip to content

Cleanup SGD/MBSGDClassifier/MBSGDRegressor#7504

Merged
rapids-bot[bot] merged 6 commits intorapidsai:release/25.12from
jcrist:cleanup-sgd
Nov 19, 2025
Merged

Cleanup SGD/MBSGDClassifier/MBSGDRegressor#7504
rapids-bot[bot] merged 6 commits intorapidsai:release/25.12from
jcrist:cleanup-sgd

Conversation

@jcrist
Copy link
Copy Markdown
Member

@jcrist jcrist commented Nov 18, 2025

This:

  • Cleans up SGD, MBSGDClassifier, and MBSGDRegressor, following all the guidelines in [TRACKER] Cleanup python estimator implementations #7317.
  • Adds a new fit_sgd function to handle fitting a linear model using SGD. This was the last part of Deprecate and remove solver_model #6938 sans deprecation/removal of the solver classes themselves.
  • Removes the undocumented solver_model attribute in favor of storing the fitted attributes on the models themselves.
  • Adds support for all label types to MBSGDClassifier, bringing it in line with our other classifiers
  • Adds a validation check to MBSGDClassifier to ensure it's fitting a binary classification problem, since multiclass is currently not supported.
  • Removes the SGD.predictClass method. This method is now unused. It didn't validate the SGD represented a classification problem, didn't handle non [0, 1] classes, and didn't match any standard method name or interface. Our other solvers only support regression problems, with the caller required to convert the output to solve a classification problem when needed. I dropped it as a breaking change here since I doubt anyone is using it, but could back off to a deprecation if people feel strongly. Dropping it lets us rip out target_dtype sooner/easier.

Breaking Change Summary:

  • Removal of SGD.predictClass
  • MBSGDClassifier.classes_ is now always a numpy.ndarray (mirroring the recent work on our other classifiers)

With this cleanup, target_dtype is no longer used. After this is in we can remove that bit from our api decorators/base class to simplify our internals further.

Part of #7317.
Fixes #6938.

@jcrist jcrist self-assigned this Nov 18, 2025
@jcrist jcrist requested a review from a team as a code owner November 18, 2025 19:57
@jcrist jcrist requested a review from divyegala November 18, 2025 19:57
@jcrist jcrist added Cython / Python Cython or Python issue improvement Improvement / enhancement to an existing function breaking Breaking change algo: linear-model labels Nov 18, 2025
@jcrist jcrist requested a review from csadorf November 18, 2025 19:59
Comment thread python/cuml/tests/test_mbsgd_classifier.py
@jcrist
Copy link
Copy Markdown
Member Author

jcrist commented Nov 18, 2025

All test failures are due to a current GitHub incident. Given other tests passed I suspect that they'll pass when rerun, but I'm going to hold off on doing that until review in case I need to push changes. edit: incident is resolved, all green now

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, just a few minor comments for posterity and one recommendation.

Comment thread python/cuml/cuml/solvers/sgd.pyx
Comment thread python/cuml/cuml/linear_model/mbsgd_classifier.py
Comment thread python/cuml/cuml/solvers/sgd.pyx
This method doesn't make sense since:
- It only works on binary classification problems
- It doesn't validate that the model is a classifier
- It doesn't follow our typical conventions

Proposing removing it as a breaking change. FWIW no other solver has a
classification method, it's unused, and doesn't match a typical name of
any sklearn method.
@jcrist
Copy link
Copy Markdown
Member Author

jcrist commented Nov 19, 2025

/merge

@rapids-bot rapids-bot Bot merged commit 4ed9e85 into rapidsai:release/25.12 Nov 19, 2025
101 checks passed
@jcrist jcrist deleted the cleanup-sgd branch November 19, 2025 23:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

algo: linear-model breaking Breaking change Cython / Python Cython or Python issue improvement Improvement / enhancement to an existing function

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants