-
-
Notifications
You must be signed in to change notification settings - Fork 25.8k
Calculation of alphas in ElasticNetCV doesn't use sample_weight #22914
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 agree, the max The change policy might be tricky as it corresponds to changing a model (once fixed, |
I could simply modify the function (1) I would always make the First we generate the data and the array of from sklearn.linear_model import ElasticNet, enet_path
from sklearn.datasets import make_regression
X, y = make_regression(1_000_000, n_features=50, noise=0.1, random_state=0)
alphas, _, _ = enet_path(X, y) Now we time the alternative approaches: %%timeit
enet_path(X, y) Output: %%timeit
enet = ElasticNet(alpha = alphas[-1])
enet.fit(X, y) Output: (2) Thus, Therefore, the normalization of |
I agree with that |
I think it's better to first fix the issue, if there's the possibility of a simple fix as it seems here, with a good non-regression test like you showed in your issue. Then we can think about making some small refactorings in a separate PR. |
Describe the bug
In ElasticNetCV, the first and largest value of
alpha
, call italpha_max
, should be just large enough to force all of the coefficients to become zero. The existing code works correctly whensample_weight
is not specified. However, the computation ofalpha_max
does not take into accountsample_weight
.Steps/Code to Reproduce
Expected Results
If the correct value of
alpha_max
is computed, thenenet.coef_
should be right at the cusp of zero, such that any smaller value ofalpha
makes it nonzero:Actual Results
enet.coef_
is[0.1970807 0.19708023]
.Versions
The text was updated successfully, but these errors were encountered: