Skip to content
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.

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

Make RidgeCV, LogisticRegressionCV, ... warn when found optimal regularization parameter lies at the boundary of the range #16398

Open
ogrisel opened this issue Feb 6, 2020 · 7 comments · May be fixed by #23031

Comments

@ogrisel
Copy link
Member

ogrisel commented Feb 6, 2020

I think the RidgeCV().fit(X_train, y_train) should warn the user if the found value for alpha_ is either alphas.min() or alphas.max(). E.g.

StatisticalWarning: the optimal value for the regularization parameter 'alpha' was 0.01 which lies at a boundary of the explored range (between 0.01 and 1.). Consider setting the 'alphas' parameter to explore a wider range. 

We could add a new boundary_warning=True constructor parameter to make it possible to silence the warning.

BTW, the default ranges could probably be extended between 1e-6 and 1e6 with 13 levels on the logspace whenever it is cheap to do so (e.g. for RidgeCV whos current default range much too narrow [0.1, 1., 10.]).

@divyaprabha123
Copy link
Contributor

Hi @ogrisel can I raise a pull request for this?
I also think it would be more appropriate if we extend the boundaries then warn the users.

@rth
Copy link
Member

rth commented Feb 6, 2020

Please do @divyaprabha123 ! I think it would be better to make 2 separate PRs: one to warn for such cases (which makes sense no matter the boundaries) and one to increase the default boundaries in a few models.

@divyaprabha123
Copy link
Contributor

Please do @divyaprabha123 ! I think it would be better to make 2 separate PRs: one to warn for such cases (which makes sense no matter the boundaries) and one to increase the default boundaries in a few models.

Sure! I will do for both

@divyaprabha123
Copy link
Contributor

take

@Reksbril
Copy link
Contributor

Reksbril commented Apr 3, 2020

@divyaprabha123 are you going to work on the other classes too, or only RidgeCV? In the first case, I've listed the ones that I think would also benefit from the change (in #16783).

@divyaprabha123
Copy link
Contributor

Yes I am! : ). @Reksbril

@bmreiniger
Copy link
Contributor

I think I can finish off the work by @divyaprabha123 toward RidgeCV in #16408. Depending on how that goes I may have a look at the others listed by @Reksbril at #16783.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment