Skip to content

Conversation

@tadejsv
Copy link
Contributor

@tadejsv tadejsv commented Nov 24, 2020

This PR is a spin-off from #4835.

What does this PR do?

Recall, Precision

These are all metrics that can be represented as a (quotient) function of "stat scores" - thanks to subclassing StatScores their code is extremely simple. Here are the parameters common to all of them:

  • average: this builds on the reduce parameter in StatScores. The options here (micro, macro, weighted, none or None, samples) are exactly equivalent to the sklearn counterparts, so I won't go into details.

  • mdmc_average: builds on the mdmc_reduce from StatScores. This decides how to average scores for multi-dimensional multi-class inputs. Already discussed in mdmc_reduce.

Both also get the top_k parameter, enabling their use as Recall@K and Precision@K - very useful for information retrieval.

Deprecations

I have deprecated precision_recall metric, as well as the old precision and recall (in case someone was importing them using the full path, otherwise they are replaced by the new precision and recall)

@pep8speaks
Copy link

pep8speaks commented Nov 24, 2020

Hello @tadejsv! Thanks for updating this PR.

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2021-01-17 18:02:19 UTC

@tadejsv tadejsv changed the title Classification metrics overhaul: precision & recall (3/n) Classification metrics overhaul: precision & recall (4/n) Nov 24, 2020
Copy link
Collaborator

@SkafteNicki SkafteNicki left a comment

Choose a reason for hiding this comment

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

More comments...

Copy link
Collaborator

@SkafteNicki SkafteNicki left a comment

Choose a reason for hiding this comment

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

LGTM :]

@SkafteNicki SkafteNicki added ready PRs ready to be merged and removed has conflicts labels Jan 15, 2021
assert get_num_classes(pred, target, num_classes) == expected_num_classes


@pytest.mark.parametrize(['pred', 'target', 'expected_tp', 'expected_fp',
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did we remove those tests ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am removing tests for deprecated functions. This was also done in other PRs before, see #4704

Copy link
Contributor

@rohitgr7 rohitgr7 left a comment

Choose a reason for hiding this comment

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

Looks good. Just some comments.

Copy link
Contributor

@teddykoker teddykoker left a comment

Choose a reason for hiding this comment

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

Nice work :)

Copy link
Collaborator

@Borda Borda left a comment

Choose a reason for hiding this comment

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

some minor comments, but otherwise it LGTM ;]

@rohitgr7 rohitgr7 enabled auto-merge (squash) January 18, 2021 08:00
@rohitgr7 rohitgr7 merged commit c8f605e into Lightning-AI:release/1.2-dev Jan 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature Is an improvement or enhancement ready PRs ready to be merged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants