8000 RFC Deprecate RidgeCV(cv=cv)? · Issue #14886 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]

Skip to content

RFC Deprecate RidgeCV(cv=cv)? #14886

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

Open
amueller opened this issue Sep 4, 2019 · 8 comments
Open

RFC Deprecate RidgeCV(cv=cv)? #14886

amueller opened this issue Sep 4, 2019 · 8 comments

Comments

@amueller
Copy link
Member
amueller commented Sep 4, 2019

I think we should deprecate RidgeCV for cv!=None. It adds weird code paths and doesn't add anything over GridSearchCV. Also, changing cv radically changes the behavior in this estimator.
I think deprecating cv here would make the behavior much clearer, and probably alert some people to the fact that they are just doing standard grid-search.

@agramfort
Copy link
Member
agramfort commented Sep 5, 2019 via email

@amueller
Copy link
Member Author
amueller commented Sep 9, 2019

Yeah there's certainly ways to make RidgeCV behave more like the other EstimatorCVs. But right now it does something entirely different.
Another option that might be even better would be to remove the current functionality and replace it by a warm-starting functionality and remove the LOO functionality?

@agramfort
Copy link
Member
agramfort commented Sep 12, 2019 via email

@glemaitre
Copy link
Member

the LOO functionality is extremely efficient.

+1

The right thing to do is to adopt the path paradigm with warm starting.

Does it mean that warm_start could be added in Ridge then?

If RidgeCV does not benefit from having an internal GridSearchCV, it should make sense to remove the cv parameter.

@agramfort
Copy link
Member
agramfort commented Sep 17, 2019 via email

@glemaitre
Copy link
Member

it still sets automatically a good default grid of alpha so removing it
seems a regression

But the RidgeCV using the LOO will still use this grid of alpha

< 8000 svg aria-label="Loading..." style="box-sizing: content-box; color: var(--color-icon-primary);" width="32" height="32" viewBox="0 0 16 16" fill="none" role="img" data-view-component="true" class="anim-rotate">

@agramfort
Copy link
Member
agramfort commented Sep 18, 2019 via email

@qinhanmin2014
Copy link
Member

I agree that we should deprecate cv and always use LOO in RidgeCV.

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

No branch 34F2 es or pull requests

6 participants
0