-
-
Notifications
You must be signed in to change notification settings - Fork 25.8k
[MRG] Add Penalty factors for each coefficient in enet ( see #11566) #11671
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
base: main
Are you sure you want to change the base?
Conversation
Thanks for the PR. Tests are failing because the order of the parameters in the documentation doesn't match the order in the signature (I think). We should make this error message better. Also flake8 errors. You should describe this feature in the user guide, and possibly add it to an example if you can come up with a good use-case or demonstration. Finally it would be great if you could provide a rough benchmark showing that in the case of specifying the same penalty for all parameters, the performance doesn't degenerate much with your change. |
# Accuracy of the model with prior knowledge should be higher | ||
# than the model without prior knowledge for a hard data set | ||
# (much less samples than features) | ||
assert_greater(clf_with_prior.score(X_test, y_test), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use plain assert
@doaa-altarawy would you have a good illustration of this feature to make an example? |
Yes, I'll be working on the example, user guide, and benchmarks (as Andrew suggested) after next week. I'm currently giving a software best practices summer camp to some Computational Chemistry students, so I'll have time afterward. |
"""Compute elastic net path with coordinate descent | ||
|
||
The elastic net optimization function varies for mono and multi-outputs. | ||
|
||
For mono-output tasks it is:: | ||
|
||
1 / (2 * n_samples) * ||y - Xw||^2_2 | ||
+ alpha * l1_ratio * ||w||_1 | ||
+ alpha * l1_ratio * l1_weights * ||w||_1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the l1_weights
should be inside the L1 norm: ||l1_weights*w||__1
Is there any progress on this issue? |
There is another PR working on this, for me I didn't have time to add the required benchmarks. I hope the other PR will be merged soon. |
@doaa-altarawy Which PR are you referring to? #9405 has this feature, but is only a proof of concept/draft PR that won‘t be merged. So I think continuing this PR here is still welcome, in particular providing a good motivating use case/example. |
A good use case of the penalties is Adaptive Lasso, where I believe the penalty factors would be set to 1 / abs(coef from Ridge) or 1 / abs(coef from OLS). See Hui Zou (2006). The Adaptive Lasso and Its Oracle Properties. Journal of the American Statistical Association, 101:476, 1418-1429, DOI: 10.1198/016214506000000735. |
This pull request implements the feature in issue #11566
Also, I added a test for this feature.