ENH/FIX: make coordinate descent solver converge for ridge regression #32845
Conversation
|
@OmarManzoor @virchan @ogrisel you might be interested |
|
As teaser: I need this for a larger surprise PR... |
| assert se_ols <= 1e-20 | ||
| assert se_enet <= 1e-20 | ||
| else: | ||
| assert se_enet <= se_ols <= 1e-20 # Who would have thought that? |
There was a problem hiding this comment.
I find this quite surprising.
There was a problem hiding this comment.
Can we use assert_allclose here, since we want to check that ElasticNet(alpha=0.0) converges to Ridge? Also, I'm curious. --- what surprised you, aside from floating-point effects?
virchan
left a comment
There was a problem hiding this comment.
LGTM! Thanks, @lorentzenchr!
I just have a few questions for my own understanding.
| # This is Ridge regression, we use formulation B for the dual gap. | ||
| gap = R_norm2 + 0.5 * beta * w_l2_norm2 - Ry | ||
| gap += 1 / (2 * beta) * dual_norm_XtA |
There was a problem hiding this comment.
I understand that this is an implementation detail. However, would it be useful to include this mathematical detail in the user guide, given that Formulation A is already covered?
| assert se_ols <= 1e-20 | ||
| assert se_enet <= 1e-20 | ||
| else: | ||
| assert se_enet <= se_ols <= 1e-20 # Who would have thought that? |
There was a problem hiding this comment.
Can we use assert_allclose here, since we want to check that ElasticNet(alpha=0.0) converges to Ridge? Also, I'm curious. --- what surprised you, aside from floating-point effects?
Reference Issues/PRs
Warnings for
alpha=0/l1_ratio=0where introduced in #620. A note that Enet with dual gap as stopping criterion does not converge was mentioned in #19620 (comment).What does this implement/fix? Explain your changes.
This PR makes the coordinate descent solver of
LassoandElasticNetetc. converge for Ridge regression.Details
What prevents the CD solver in the main branch is that the dual gap is not valid for
alpha=0/l1_ratio=0. Fortunately, there is an alternative dual gap available, see #22836.Any other comments?
There are some surprising results, see the new
test_enet_ols_consistency.