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

Fix average_precision metric #2319

Merged
merged 3 commits into from
Jun 23, 2020

Conversation

elias-ramzi
Copy link
Contributor

@elias-ramzi elias-ramzi commented Jun 22, 2020

What does this PR do?

This PR fixes pytorch_lightning.metrics.functional.average_precision. It also adds a test that would have failed with the old implementation

Parentheses were missing in the definition of the function.

Bug described in #2315.

Before submitting

  • Was this discussed/approved via a Github issue? (no need for typos and docs improvements)
  • Did you read the contributor guideline, Pull Request section?
  • Did you make sure your PR does only one thing, instead of bundling different changes together? Otherwise, we ask you create a separate PR for every change.
  • Did you make sure to update the documentation with your changes?
  • Did you write any new necessary tests?
  • Did you verify new and existing tests pass locally with your changes?
  • If you made a notable change (that affects users), did you update the CHANGELOG?

PR review

Anyone in the community is free to review the PR once the tests have passed.
If we didn't discuss your PR in Github issues there's a high chance it will not be merged.

Did you have fun?

Make sure you had fun coding 🙃

…test that failed with the old implementation
@mergify mergify bot requested a review from a team June 22, 2020 19:00
@codecov
Copy link

codecov bot commented Jun 22, 2020

Codecov Report

Merging #2319 into master will not change coverage.
The diff coverage is 100%.

@@          Coverage Diff           @@
##           master   #2319   +/-   ##
======================================
  Coverage      88%     88%           
======================================
  Files          70      70           
  Lines        5501    5501           
======================================
  Hits         4834    4834           
  Misses        667     667           

@elias-ramzi elias-ramzi force-pushed the fix_average_precision_metric branch from ee9e14f to cc8908f Compare June 22, 2020 19:39
@Borda Borda added the bug Something isn't working label Jun 22, 2020
@elias-ramzi
Copy link
Contributor Author

elias-ramzi commented Jun 22, 2020

Hi !
Could you help me on the CI checks. I don't understand why some checks are failing, and I don't understand the logs that are produced 😕

CHANGELOG.md Outdated Show resolved Hide resolved
@mergify mergify bot requested a review from a team June 22, 2020 19:57
@Borda
Copy link
Member

Borda commented Jun 22, 2020

Could you help me on the CI checks. I don't understand why some checks are failing, and I don't understand the logs that are produced

it was not related to your code, just random GitHub issue 🐰

Co-authored-by: Jirka Borovec <Borda@users.noreply.github.com>
@elias-ramzi
Copy link
Contributor Author

Ok thank you, I thought so but wasn't sure! ☺️

@mergify mergify bot requested a review from a team June 23, 2020 05:30
@Borda Borda requested review from tullie and williamFalcon June 23, 2020 06:03
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

@mergify mergify bot requested a review from a team June 23, 2020 11:13
@Borda Borda merged commit 92f122e into Lightning-AI:master Jun 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants