Skip to content

ENH add sparse and sample_weight support to MultiTaskElasticNet#33440

Open
lorentzenchr wants to merge 3 commits intoscikit-learn:mainfrom
lorentzenchr:cd_multitask_sparse
Open

ENH add sparse and sample_weight support to MultiTaskElasticNet#33440
lorentzenchr wants to merge 3 commits intoscikit-learn:mainfrom
lorentzenchr:cd_multitask_sparse

Conversation

@lorentzenchr
Copy link
Copy Markdown
Member

@lorentzenchr lorentzenchr commented Mar 2, 2026

Reference Issues/PRs

Closes #3702, finally.
Contributes to #33457

What does this implement/fix? Explain your changes.

This add support to fit

  • with sample_weight
  • on sparse X

for

  • MultiTaskElasticNet
  • MultiTaskLasso
  • MultiTaskElasticNetCV
  • MultiTaskLassoCV

AI usage disclosure

None

Any other comments?

#15436 (comment):

It's just that I hear something that we are consistent in linear models
with certain options implemented
only in certain estimators without clear reason. So it suggests we should
have sample_weights added to CV
classes very soon and ideally in the same release cycle

Only 6 years later and not in the same release 😄 (but note that the comment did not apply to multitask estimators of this PR).

@lorentzenchr
Copy link
Copy Markdown
Member Author

@OmarManzoor Are you interested in this PR?

Copy link
Copy Markdown
Contributor

@OmarManzoor OmarManzoor left a comment

Choose a reason for hiding this comment

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

Thank you for the PR @lorentzenchr. I had an overall look and left some comments. Overall I think it looks nice. Some tests are failing though

Comment on lines +769 to +772
reg = estimator(alphas=5)
reg.fit(csr_container(X), y)
reg1 = estimator(alphas=5)
reg1.fit(csr_container(X, dtype=np.float32), y)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What does reg stand for here?

Comment on lines +898 to +899
for i_ind in range(startptr, endptr):
tmp += R[X_indices[i_ind]] * X_data[i_ind]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

To keep it consistent with the rest of the changes above

Suggested change
for i_ind in range(startptr, endptr):
tmp += R[X_indices[i_ind]] * X_data[i_ind]
for i_ind in range(startptr, endptr):
i = X_indices[i_ind]
tmp += R[i] * X_data[i_ind]

Comment on lines +1549 to +1552
for i_ind in range(startptr, endptr):
i = X_indices[i_ind]
norm2_cols_X[j] += (X_data[i_ind] - X_mean[j]) ** 2
norm2_cols_X[j] += (n_samples - endptr + startptr) * X_mean[j] ** 2
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It doesn't seem like i is being used in this block

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

sample_weight for lasso, elastic etc

2 participants