search

scikit-learn-contrib / scikit-matter

A collection of scikit-learn compatible utilities that implement methods born out of the materials science and chemistry communities
https://scikit-matter.readthedocs.io/en/v0.2.0/
BSD 3-Clause "New" or "Revised" License
70 stars 18 forks source link

Precomputed y pcovr #136

Closed rosecers closed 1 year ago

rosecers commented 2 years ago

When doing a sweep of alpha parameters, finding that recomputing Yhat is the most costly step. I added a parameter to PCovR and KPCovR to use a "precomputed" regression, where the user can optionally supply the regression weights to the fit function (otherwise they will be computed via least squares).

I added tests and also did a tiny bit of cleaning.

Luthaf commented 2 years ago

I think having an example/small tutorial on how to use this would be nice. We are stating to pile up features and options, but it can be hard for newcomers to know how to properly use them.

rosecers commented 2 years ago

I think having an example/small tutorial on how to use this would be nice. We are stating to pile up features and options, but it can be hard for newcomers to know how to properly use them.

What do you think of the one added?

bhelfrecht commented 2 years ago

Is providing a precomputed Y functionally different than providing a pre-fitted regressor? For instance, if you fit a (Kernel)Ridge object, and pass the fitted (Kernel)Ridge instance to (K)PCovR with the regressor option, it should use the existing weights and avoid recomputing them. IIRC this is one reason we went with passing the regressor as an input, and I think we already have existing machinery to handle this use case.

On the other hand, using a pre-fitted regressor instead of a pre-computed Yhat requires a Yhat = X * W at each call of fit, but this shouldn't be too expensive. Would you be willing to do a quick benchmark to see this slowdown from the extra multiplication turns out to be significant? That might help us decide how to move forward.

bhelfrecht commented 2 years ago

Ah, after another look at the proposed code I see that the weights are passed to fit instead of Yhat. So I do think we already cover this with the pre-fitted regressors. Do both approaches give the same results with similar timings?

Luthaf commented 2 years ago

What do you think of the one added?

This looks good!

rosecers commented 1 year ago

Ah, after another look at the proposed code I see that the weights are passed to fit instead of Yhat. So I do think we already cover this with the pre-fitted regressors. Do both approaches give the same results with similar timings?

Yes, they do, although sometimes it can be incredibly costly to pickle the regression instance, and so that was the thought behind sending a precomputed W.