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.

Sign up for GitHub

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

Jump to bottom

FIX LinearRegression sparse + intercept + sample_weight #22891

Merged
merged 6 commits into from
Mar 18, 2022
Merged

FIX LinearRegression sparse + intercept + sample_weight #22891

merged 6 commits into from
Mar 18, 2022

Conversation

jeremiedbb
Copy link
Member

@jeremiedbb jeremiedbb commented Mar 18, 2022 •
edited
Loading

Partial fix for #15438
Fixes #19578

This PR fixes the issue for LinearRegression only. I intend to check other linear models in separate PR(s).

To account for sample weight, we rescale X and y. When X is sparse we also need to rescale X_offset because the centering is implicit.

ping @agramfort

@jeremiedbb jeremiedbb added this to the 1.1 milestone Mar 18, 2022
agramfort
agramfort approved these changes Mar 18, 2022
View reviewed changes
Copy link
Member

@agramfort agramfort left a comment

Choose a reason for hiding this comment

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

nice catch @jeremiedbb !

thx for digging into this.

ogrisel
ogrisel approved these changes Mar 18, 2022
View reviewed changes
Copy link
Member

@ogrisel ogrisel left a comment

Choose a reason for hiding this comment

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

LGTM, just a nitpick about a local variable name:

sklearn/linear_model/_base.py Outdated Show resolved Hide resolved
sklearn/linear_model/_base.py Outdated Show resolved Hide resolved
sklearn/linear_model/_base.py Outdated Show resolved Hide resolved
lorentzenchr
lorentzenchr approved these changes Mar 18, 2022
View reviewed changes
Copy link
Member

@lorentzenchr lorentzenchr left a comment

Choose a reason for hiding this comment

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

Very nice.

@lorentzenchr lorentzenchr changed the title Fix LinearRegression sparse + intercept + sample_weight FIX LinearRegression sparse + intercept + sample_weight Mar 18, 2022
@lorentzenchr lorentzenchr merged commit 7414a7c into scikit-learn:main Mar 18, 2022
@lorentzenchr
Copy link
Member

@jeremiedbb Did you already start with the same for _solve_sparse_cg in _ridge.py?

glemaitre pushed a commit to glemaitre/scikit-learn that referenced this pull request Apr 6, 2022
@jeremiedbb @glemaitre
FIX LinearRegression sparse + intercept + sample_weight (scikit-learn…
a4704b4 
…#22891)
mathijs02 pushed a commit to mathijs02/scikit-learn that referenced this pull request Dec 27, 2022
@jeremiedbb
FIX LinearRegression sparse + intercept + sample_weight (scikit-learn…
ded70bf 
…#22891)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thomasjpfan thomasjpfan thomasjpfan left review comments

@glemaitre glemaitre glemaitre left review comments

@ogrisel ogrisel ogrisel approved these changes

@agramfort agramfort agramfort approved these changes

@lorentzenchr lorentzenchr lorentzenchr approved these changes

Assignees
No one assigned
Labels
module:linear_model
Projects
None yet
Milestone
1.1
Development

Successfully merging this pull request may close these issues.

LinearRegression fits wrongly on csr sparse matrix with sample weights
6 participants
@jeremiedbb @lorentzenchr @ogrisel @agramfort @thomasjpfan @glemaitre