Minimal Generalized linear models implementation (L2 + lbfgs) #14300
Conversation
* smarter initialization of intercept * PEP 257 -- Docstring Conventions * minor docstring changes
* P2 also accepts 1d array and interprets it as diagonal matrix * improved input checks for P1 and P2
…hecks * adapt examples of GeneralizedLinearModel to new defaults for P1, P2 and selection * fix precision/decimal issue in test_poisson_enet * use more robust least squares instead of solve in IRLS * fix sign error in input checks
* add Binomial distribution
* add Logit link
* tests for binomial against LogisticRegression
* option 'auto' for link
* reduce code duplication by replacing @abstractproperty by @Property
* refactor into function _irls_solver * refactor into function _cd_solver * replace of safe_sparse_dot by matmul operator @ * more efficient handling of fisher matrix * sparse coo matrices are converted to csc or csr * sample weights don't except sparse matrices * minor doc changes
* renamed option irls into guess * removed option least_squares * updated tests
* new estimator GeneralizedLinearRegressor * loss functions for Tweedie family and Binomial * elasitc net penalties * control of penalties by matrix P2 and vector P1 * new solvers: coordinate descent, irls * tests * documentation * example for Poisson regression
* make glm module private * typos * improve tests
There was a problem hiding this comment.
I made minor pushes to compare the perf of the 2 models side by side in the example (293214c)
CI is green, LGTM. I think most of the comments are either minor or addressed.
Thank you very much for the great work and for your patience @lorentzenchr @rth . Much appreciated. Looking forward to seeing the future improvements!
Feel free to review my last changes and merge when you see fit
|
And the second prize of chocolate goes to ... @NicolasHug 🥈 🍫 🎉 |
|
Thanks a lot for the review @NicolasHug and for quickly addressing comments @lorentzenchr ! Let's give it another couple of days in case @ogrisel has other suggestions. For the person who is going to merge this please edit the merge commit description to remove individual commits but to keep one of each of |
|
(Sorry accidental key press, updated message above with the missing part). |
|
Very excited for this! Sorry I've not been able to review in a while. |
|
@ogrisel As a last remark: How do you feel about reversing the sorting of the x-axis in the last plot of plot_tweedie_regression_insurance_claims.py from "safest to riskiest" to "riskiest to safest"? This way, both examples would have a comparable "rank" plot at the end. |
|
OK, merging with +2 and several partial reviews. Thanks @lorentzenchr and to all reviewers! I'm really glad this is done.
Feel free to open a follow up PR. Also looking forward to other additional features we could add from #9405. Will comment there. |
|
🍺 ! |
|
Many thanks to all of you! This was hard work and with enough perseverance we got it done. 👏 |
|
Awesome! Might be worth announcing on the mailing list and asking people to take it for a ride. |
|
Definitely some impressive perseverance. But you forgot to credit the
influential effect of timely chocolate, too.
|
@lorentzenchr Would you like to write that announcement email? Users can try it with nightly wheels https://scikit-learn.org/stable/developers/advanced_installation.html#installing-nightly-builds and the dev documentation https://scikit-learn.org/dev/modules/linear_model.html#generalized-linear-regression |
|
@rth Thanks for asking. As I'm not (yet) on the scikit-learn mailing list and as I can't find a precedent for new features as template, I'd rather let you go ahead 😊 |
…ikit-learn#14300) Co-authored-by: Christian Lorentzen <lorentzen.ch@googlemail.com> Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org> Co-authored-by: Nicolas Hug <contact@nicolas-hug.com>
…ikit-learn#14300) Co-authored-by: Christian Lorentzen <lorentzen.ch@googlemail.com> Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org> Co-authored-by: Nicolas Hug <contact@nicolas-hug.com>
A minimal implementation of Generalized Linear Models (GLM) from #9405 by @lorentzenchr
This only includes L2 penalty with the lbfgs solver. In other words, in excludes L1 penalties (or CD solver), matrix penalties, warm start, some distributions (e.g.
BinomialDistribution), newton-cg & irls solvers from the original GLM PR.The goals is to get an easier to review initial implementation. Benchmarks were done in #9405 (comment)
TODO
sample_weightvalidations MAINT Common sample_weight validation #14307sklearn.linear_modelconsider using a diagonal preconditioner as discussed in Scaling issues in l-bfgs for LogisticRegression #15556 and MRG Logistic regression preconditioning #15583 for logistic regressionEdit: Not in this PR -- Roman