ENH avoid futile recomputation of R_sum in sparse_enet_coordinate_descent - #31387
Merged
Conversation
…cent * R_sum is np.sum(residual) and won't change by a coordinate upate if X_mean is provided.
OmarManzoor
approved these changes
May 22, 2025
OmarManzoor
left a comment
Contributor
There was a problem hiding this comment.
LGTM. Thanks @lorentzenchr
ogrisel
approved these changes
May 27, 2025
ogrisel
left a comment
Member
There was a problem hiding this comment.
The code is quite low level, it's not easy to check for math invariants. So I wrote this script to empirically validate the claims of the PR.
# %%
import scipy.sparse as sp
import numpy as np
from sklearn.linear_model import ElasticNet
from time import perf_counter
rng = np.random.default_rng(42)
X = rng.uniform(size=(1_000, 100_000))
X[X < 0.9] = 0.0 # Sparsify the matrix
X_sparse = sp.csc_array(X)
w_true = np.zeros(X.shape[1])
w_true[0:300] = 10.0
assert X.mean(axis=0)[0:5].min() > 0.01
y = X_sparse @ w_true + 1 + rng.normal(size=X.shape[0]) * 0.1
# %%
reg = ElasticNet(
alpha=0.1, l1_ratio=0.5, fit_intercept=True, selection="random", random_state=42
)
coef_dense = reg.fit(X, y).coef_
intercept_dense = reg.intercept_
print("10 first dense coefficients:", coef_dense[:10])
print("dense intercept:", intercept_dense)
# %%
for i in range(5):
tic = perf_counter()
coef_sparse = reg.fit(X_sparse, y).coef_
toc = perf_counter()
print(f"Time for sparse fit: {toc - tic:.3f} seconds")
intercept_sparse = reg.intercept_
print("10 first sparse coefficients:", coef_sparse[:10])
print("sparse intercept:", intercept_sparse)
assert np.allclose(coef_dense, coef_sparse)
assert np.allclose(intercept_dense, intercept_sparse)The results are:
- both
mainand this branch yields the same coefficients/intercept as expected, and the assertions always pass (they use the dense implementation as a reference that is not touched by this PR). - the sparse fit time is approximately 3x faster with this optimization on this data (it strongly depends on the sparsity level).
Good job @lorentzenchr!
/cc @agramfort @mathurinm.
Member
Author
|
@ogrisel Thanks for the benchmark and review. I would not have guessed the size of the improvement 😇. The equivalence of different solvers is already checked in a test😉 |
lorentzenchr
added a commit
to lorentzenchr/scikit-learn
that referenced
this pull request
May 27, 2025
ogrisel
pushed a commit
that referenced
this pull request
May 28, 2025
jeremiedbb
pushed a commit
to jeremiedbb/scikit-learn
that referenced
this pull request
May 30, 2025
…cent (scikit-learn#31387) Co-authored-by: Omar Salman <omar.salman2007@gmail.com> Co-authored-by: Omar Salman <omar.salman@arbisoft.com>
jeremiedbb
pushed a commit
to jeremiedbb/scikit-learn
that referenced
this pull request
May 30, 2025
elhambbi
pushed a commit
to elhambbi/scikit-learn
that referenced
this pull request
Jun 1, 2025
…cent (scikit-learn#31387) Co-authored-by: Omar Salman <omar.salman2007@gmail.com> Co-authored-by: Omar Salman <omar.salman@arbisoft.com>
elhambbi
pushed a commit
to elhambbi/scikit-learn
that referenced
this pull request
Jun 1, 2025
jeremiedbb
pushed a commit
that referenced
this pull request
Jun 5, 2025
…cent (#31387) Co-authored-by: Omar Salman <omar.salman2007@gmail.com> Co-authored-by: Omar Salman <omar.salman@arbisoft.com>
jeremiedbb
pushed a commit
that referenced
this pull request
Jun 5, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reference Issues/PRs
None
What does this implement/fix? Explain your changes.
This PR removes the unnecessary updates of
R_sum=np.sum(residuals), because it does not change by a coordinate update ifX_meanis provided, i.e.,np.sum(X[:, j] - X_mean[j])equals 0.Any other comments?
Should improve runtime performance of
LassoandElasticNetfor sparse inputXa bit.