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

[MRG] Add Penalty factors for each coefficient in enet ( see #11566) #11671

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

doaa-altarawy
Copy link

This pull request implements the feature in issue #11566

Also, I added a test for this feature.

@amueller
Copy link
Member

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),

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use plain assert

@agramfort
Copy link
Member

@doaa-altarawy would you have a good illustration of this feature to make an example?

@doaa-altarawy
Copy link
Author

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
Copy link
Member

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

@amueller amueller added Needs Benchmarks A tag for the issues and PRs which require some benchmarks Stalled labels Aug 5, 2019
@ivarzap
Copy link

ivarzap commented Sep 9, 2020

Is there any progress on this issue?
I would be most interested in using the penalties.

@doaa-altarawy
Copy link
Author

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.

@lorentzenchr
Copy link
Member

@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.

@RNKuhns
Copy link

RNKuhns commented Nov 10, 2020

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.

Base automatically changed from master to main January 22, 2021 10:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants