Skip to content

Cleanup ElasticNet, Lasso, and CD#7382

Merged
rapids-bot[bot] merged 3 commits intorapidsai:mainfrom
jcrist:cleanup-enet-and-lasso
Oct 28, 2025
Merged

Cleanup ElasticNet, Lasso, and CD#7382
rapids-bot[bot] merged 3 commits intorapidsai:mainfrom
jcrist:cleanup-enet-and-lasso

Conversation

@jcrist
Copy link
Copy Markdown
Member

@jcrist jcrist commented Oct 23, 2025

This is a general python/cython cleanup of our coordinate-descent based estimators (and the solver itself).

  • Updates the code to follow modern cuml and sklearn conventions
  • Moved all hyperparameter validation out of __init__ and into respective fits, keeping the inits simple (as per sklearn convention). Also improved hyperparameter validation a bit.
  • Improved data validation, including checking for minimal data input sizes. This leads to a few upstream sklearn tests passing.
  • Split out the fit from CD into a top-level function, as described in Deprecate and remove solver_model #6938. I didn't touch QN in this PR, that will be done in a followup.
  • Removed the solver_model attribute on ElasticNet/Lasso. Since this attribute was undocumented and (mostly) unaccessed in our tests, I view it as an implementation detail and not something that necessarily needs a deprecation period.
  • Cleaned up docstrings a bit
  • General code tidyness. Hopefully this is a bit more readable now.

Part of #7317.
Part of #6938.

@jcrist jcrist self-assigned this Oct 23, 2025
@jcrist jcrist requested a review from a team as a code owner October 23, 2025 21:42
@jcrist jcrist added Cython / Python Cython or Python issue improvement Improvement / enhancement to an existing function labels Oct 23, 2025
@jcrist jcrist requested a review from viclafargue October 23, 2025 21:43
@jcrist jcrist added the non-breaking Non-breaking change label Oct 23, 2025
@jcrist
Copy link
Copy Markdown
Member Author

jcrist commented Oct 23, 2025

  • Split out the fit from CD into a top-level function, as described in Deprecate and remove solver_model #6938. I didn't touch QN in this PR, that will be done in a followup.
  • Removed the solver_model attribute on ElasticNet/Lasso. Since this attribute was undocumented and (mostly) unaccessed in our tests, I view it as an implementation detail and not something that necessarily needs a deprecation period.

In the long run I hope we can deprecate our solver estimators as public interfaces (sklearn doesn't expose their solvers directly, why should we?), instead relying on pure functions for fitting and our generic linear model predict/decision_function implementations for prediction. There's no need to have a CD or QN specific predict/decision function, these are solvers for fitting alone, which should let us drop some C++ code and simplify the wrappers. That can all be done in followups though, for now the focus here was on tidying up ElasticNet and Lasso.

@jcrist jcrist requested a review from csadorf October 23, 2025 21:48
@jcrist jcrist force-pushed the cleanup-enet-and-lasso branch from d06cb0a to ae5687f Compare October 24, 2025 01:25
Copy link
Copy Markdown
Contributor

@viclafargue viclafargue left a comment

Choose a reason for hiding this comment

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

LGTM!

Comment thread python/cuml/cuml/linear_model/elastic_net.py
Comment thread python/cuml/cuml/linear_model/elastic_net.py
Comment thread python/cuml/cuml/linear_model/lasso.py
@jcrist
Copy link
Copy Markdown
Member Author

jcrist commented Oct 28, 2025

/merge

@rapids-bot rapids-bot Bot merged commit bb1c21e into rapidsai:main Oct 28, 2025
101 checks passed
@jcrist jcrist deleted the cleanup-enet-and-lasso branch October 28, 2025 01:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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.

3 participants