Skip to content

[MRG] Normalize linear_model decision_function scores. #19142

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

Merged

Conversation

stootoon
Copy link
Contributor

Reference Issues/PRs

Fixes #19139.

What does this implement/fix? Explain your changes.

According to the docs, the decision_function scores in LinearClassifierMixin are supposed to be the distances of samples to corresponding hyperplanes. (I assume) the hyperplanes are defined by the coefficients and intercepts arrays. The scores are computed as the scalar product of each sample with each of the coefficients, plus the intercepts. The problem is that without normalizing these scores by the norm of their corresponding coefficients, the scores won't actually be the signed distances to the corresponding hyperplanes. This is because the signed distance of a point p to a hyperplane defined by c'x + b = 0 is (c'p + b)/|c|, not the (c'p + b) currently computed.

I fix this problem by normalizing the scores by the norm of their corresponding coefficients.

Any other comments?

I ran pytest on linear_model and a few of the tests are failing, some because computed accuracies are being compared to hard-coded values. This may be expected if the hard-coded values reflect desired outputs using the previous, potentially incorrect, method of computing the scores.

Also, I haven't done any checking for division by zero which would occur if any of the coefficients are all zeros, because I wasn't sure what sklearn best practices are for doing so, and it will be easy enough for whoever does know to add this. Some tests are failing because NaNs are appearing, presumably due to such division by zero.

@cmarmo
Copy link
Contributor

cmarmo commented Jan 11, 2021

Thanks @stootoon! Some lint errors need to be fixed.

Running flake8 on the diff in the range 1e46db669..21be8a215 (3 commit(s)):
--------------------------------------------------------------------------------
sklearn/linear_model/_base.py:289:1: W293 blank line contains whitespace
        
^
sklearn/linear_model/_base.py:290:56: W291 trailing whitespace
        coef_norms = np.linalg.norm(self.coef_, axis=1)        
                                                       ^
sklearn/linear_model/_base.py:291:15: E221 multiple spaces before operator
        scores    /= coef_norms
              ^
sklearn/linear_model/_base.py:292:1: W293 blank line contains whitespace
        
^

Exited with code exit status 1

CircleCI received exit code 1

@NicolasHug
Copy link
Member

Thanks for the PR @stootoon , but we can't change the output of decision_function #19139 (comment). Instead it would be better to just change the docstring, as suggested in the original issue.

Copy link
Member

@NicolasHug NicolasHug left a comment

Choose a reason for hiding this comment

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

Thanks!

@NicolasHug NicolasHug merged commit 9f86a25 into scikit-learn:master Jan 13, 2021
glemaitre pushed a commit to glemaitre/scikit-learn that referenced this pull request Jan 18, 2021
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.

Incorrect decision_function in linear_model?
4 participants