-
-
Notifications
You must be signed in to change notification settings - Fork 25.8k
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
Comments
I also don't like that RidgeCV calls GridSearchCV under the hood. Everytime
we have ModelCV it's because it's faster than using GridSearchCV. Now I
think that for certain Ridge solvers you can gain some time by precomputing
LUs, using warmstarts in conjugate gradient etc. Of course this would need
some work ... :-/
… |
Yeah there's certainly ways to make RidgeCV behave more like the other EstimatorCVs. But right now it does something entirely different. |
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?
the LOO functionality is extremely efficient. For me it's the key
feature of RidgeCV and that's why it's the default.
GirdSearchCV was used in the general case because of lack of time /
interest. The right thing to do is
to adopt the path paradigm with warm starting.
my 2c
|
+1
Does it mean that If |
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?
yes if you use iterative solvers eg CG for sparse data.
If RidgeCV does not benefit from having an internal GridSearchCV, it
should make sense to remove the cv parameter.
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 |
But the RidgeCV using the LOO will still use this grid of alpha
sure but If I want to use Ridge with CV=3 to compare estimators that have
the same folds
then I need to "know" what is a good alpha grid when using GridSearchCV
|
I agree that we should deprecate cv and always use LOO in RidgeCV. |
I think we should deprecate
RidgeCV
forcv!=None
. It adds weird code paths and doesn't add anything over GridSearchCV. Also, changingcv
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.The text was updated successfully, but these errors were encountered: