Skip to content

[MRG] Fix top_k_accuracy_score ignoring labels for "multiclass" case #19300

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

Closed
wants to merge 0 commits into from
Closed

Conversation

joclement
Copy link
Contributor

@joclement joclement commented Jan 29, 2021

Reference Issues/PRs

Fixes small bug in #16625

What does this implement/fix? Explain your changes.

  1. A small typo
  2. A bug when an actual "multiclass" type is passed, which detected as a "binary" type

Details on the bug:
I stumbled upon this problem when using the LOGO cross validator. When not all the classes of a multiclass problem are contained in the y_true parameter, the top_k_accuracy_score function raises the following error:

ValueError: y should be a 1d array, got an array of shape (4, 4) instead.

In order to replicate this bug I added a test, which fails, if my fix is not added. The error above is caused by that test.
The reason is that the function call type_of_target(y_true) determines the type "binary", if the number of unique values in y_true is <= 2.

I believe that this error doesn't need to happen, if the labels parameter is passed. Then the function can determine the classes and the type of the problem by inspecting that parameter as well.

Copy link
Member

@thomasjpfan thomasjpfan 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 @flyingdutchman23 !

Please add an entry to the change log at doc/whats_new/v1.0.rst with tag |Fix|. Like the other entries there, please reference this pull request with :pr: and credit yourself (and other contributors if applicable) with :user:.

@joclement
Copy link
Contributor Author

Thank you @thomasjpfan for reviewing this PR.

@joclement joclement changed the title Fix top_k_accuracy_score neglecting labels for "multiclass" case WIP: Fix top_k_accuracy_score neglecting labels for "multiclass" case Jan 29, 2021
@joclement joclement changed the title WIP: Fix top_k_accuracy_score neglecting labels for "multiclass" case Fix top_k_accuracy_score neglecting labels for "multiclass" case Jan 29, 2021
@joclement joclement changed the title Fix top_k_accuracy_score neglecting labels for "multiclass" case [MRG] Fix top_k_accuracy_score ignoring labels for "multiclass" case Jan 30, 2021
@joclement joclement closed this Mar 19, 2021
@joclement
Copy link
Contributor Author

I accidently closed this PR, because it was on my main branch. I will open a new one with the same changes.

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.

2 participants