10000 ENH `LinearRegression` Optimizations by jjerphan · Pull Request #22940 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]
Skip to content

ENH LinearRegression Optimizations #22940

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 18 commits into from
Closed

ENH LinearRegression Optimizations #22940

wants to merge 18 commits into from

Conversation

jjerphan
Copy link
Member
@jjerphan jjerphan commented Mar 24, 2022 •
edited
Loading

Reference Issues/PRs

Fixes #14268.
Fixes #22855.

Supersedes #17560

What does this implement/fix? Explain your changes.

Taken from #17560:

  • Changes scipy.linalg.lstsq call in the fit function in LinearRegression to include the check_finite=False flag. This is because a finiteness check is already completed in the data validation step. fit calls self._preprocess_data, which itself calls check_array, which is defined in utils/validation.py. check_array has a force_all_finite flag. The scipy.linalg.lstsq documentation can be found here.
  • Factors out Ridge code into _ridge_solvers.py so Ridge(solver = "cholesky", alpha = 0) can be used in LinearRegression without explicitly calling the Ridge estimator.
  • Enables optional solver parameter in LinearRegression, which if set to "cholesky" uses Cholesky Ridge method with alpha=0 for OLS fit.
  • Documentation and tests.

Any other comments?

TODO:

@jjerphan jjerphan mentioned this pull request Mar 24, 2022
Closed
rithvikrao added 17 commits March 25, 2022 15:46
@rithvikrao @jjerphan
Change linalg.lstsq call to include check_finite=False
9f75285 
Author:    Rithvik Rao <rithvik321@gmail.com>
@rithvikrao @jjerphan
Refactored ridge solvers into separate file, so Cholesky can be used …
08796a4 
…for linear regression

Author:    Rithvik Rao <rithvik321@gmail.com>
@rithvikrao @jjerphan
Cholesky default for linreg for 2 or fewer dimensions and positive-de…
f9bc8ab 
…finite X

Author:    Rithvik Rao <rithvik321@gmail.com>
@rithvikrao @jjerphan
Linted code
802a5bf 
Author:    Rithvik Rao <rithvik321@gmail.com>
@rithvikrao @jjerphan
Ravel coefficients instead of reshaping
149af21
@rithvikrao @jjerphan
Cholesky is param instead of heuristic. + Tests & doc
6f30e26 
Author:    Rithvik Rao <rithvik321@gmail.com>
@rithvikrao @jjerphan
More Cholesky refactor, param in __init__ not fit
2c86c5d 
Author:    Rithvik Rao <rithvik321@gmail.com>
@rithvikrao @jjerphan
Clean up dependencies on Cholesky
6bf9507
@rithvikrao @jjerphan
Add docstring, sparse tests, singular/sparse TypeError
e8a5943 
Author:    Rithvik Rao <rithvik321@gmail.com>
@rithvikrao @jjerphan
Parametrized tests correctly, sparse_equal_dense failing
809f362
@rithvikrao @jjerphan
Remove cholesky from sparse_equal_dense, lint
01b664a 
Author:    Rithvik Rao <rithvik321@gmail.com>
@rithvikrao @jjerphan
Change svd to lsqr for solver, add singular matrix test
89e94b6 
Author:    Rithvik Rao <rithvik321@gmail.com>
@rithvikrao @jjerphan
Lint
c195a48 
Author:    Rithvik Rao <rithvik321@gmail.com>
@rithvikrao @jjerphan
Simplified alpha set code
31417c7
@rithvikrao @jjerphan
ValueError and complexity analysis
5c7ef92 
Author:    Rithvik Rao <rithvik321@gmail.com>
@rithvikrao @jjerphan
Fixed tests and tested ValueError
2a4485c 
Author:    Rithvik Rao <rithvik321@gmail.com>
@rithvikrao @jjerphan
Doc fix
e5ca2c0
@jjerphan jjerphan force-pushed the linear-regression-solvers branch from 9bebad0 to e5ca2c0 Compare March 25, 2022 17:09
@lorentzenchr
Copy link
Member

@jjerphan I think this old PR does much too much refactoring.
I see 2 options:

  • Make LinearRegression a subclass of Ridge which sets alpha=0 and has one additional solver lstsq.
  • Call _ridge_regression for new solvers. It seems to do all the input checking, but no centering.

@jjerphan
Copy link
Member Author
jjerphan commented Dec 2, 2022

Closing this PR to restart this work from scratch one day.

@jjerphan jjerphan closed this Dec 2, 2022
@jjerphan jjerphan deleted the linear-regression-solvers branch January 12, 2023 08:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
module:linear_model No Changelog Needed
Projects
None yet
Milestone
No milestone
3 participants
@jjerphan @lorentzenchr @rithvikrao
0