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.

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

Classification metrics overhaul: precision & recall (4/n) #4842

Merged
merged 92 commits into from
Jan 18, 2021
Merged

Classification metrics overhaul: precision & recall (4/n) #4842

merged 92 commits into from
Jan 18, 2021

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
Member

@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 :]

tests/deprecated_api/test_remove_1-4.py Outdated Show resolved Hide resolved
@SkafteNicki SkafteNicki added ready PRs ready to be merged and removed has conflicts labels Jan 15, 2021
@@ -120,55 +87,6 @@ def test_get_num_classes(pred, target, num_classes, expected_num_classes):
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
Member

@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 ;]

docs/source/metrics.rst Show resolved Hide resolved
tests/metrics/classification/test_precision_recall.py Outdated Show resolved Hide resolved
pytorch_lightning/metrics/classification/helpers.py Outdated Show resolved Hide resolved
pytorch_lightning/metrics/classification/helpers.py Outdated Show resolved Hide resolved
pytorch_lightning/metrics/functional/classification.py Outdated Show resolved Hide resolved
pytorch_lightning/metrics/functional/classification.py Outdated Show resolved Hide resolved
@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