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

Detection IoU #1284

Merged
merged 211 commits into from
Apr 18, 2023
Merged

Detection IoU #1284

merged 211 commits into from
Apr 18, 2023

Conversation

twsl
Copy link
Contributor

@twsl twsl commented Oct 21, 2022

What does this PR do?

Fixes #659

Continues the work from #761 and adds IoU, GIoU, DIoU and CIoU

Things I did so far:

  • Implementation follows naming convention
  • The input and its validation is the same as for mAP so all detection metrics can be used in a MetricsCollection
  • Threshold is set to None by default to enable proper progress logging, especially for IoUs with negative values
  • Use .diag().mean() for single value calculation if all labels are equal, .mean() and all permutations otherwise
  • Implemented additional metrics via inheritance
  • Check for correct labels is supported and enabled by default
  • IoUs are aggregated per image first and then averaged

Things to do:

  • There is still no checking whether the labels are correct, how to handle them in calculation?
  • Decide what to do with multiple boxes per ground truth, usually in object detection this is handled by NMS
  • More test cases are required
  • Currently IoU calculation takes all boxes and doesn't evaluate on a per image basis. Should it rather calculate on a per image basis or keep evaluation per box?
  • is .diag().mean() the correct behaviour if the number if ground truth boxes != predicted boxes?

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 to update the docs?
  • Did you write any new necessary tests?

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 🙃

cc: @SkafteNicki @jscottcronin @tkupek @Borda

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.

This is great work :) really looking forward to this being in TM
For now I have just taken an overall look, will try to deep dive into the code sometime during the next couple of days

src/torchmetrics/detection/ciou.py Outdated Show resolved Hide resolved
src/torchmetrics/detection/ciou.py Show resolved Hide resolved
src/torchmetrics/detection/diou.py Show resolved Hide resolved
src/torchmetrics/detection/giou.py Show resolved Hide resolved
src/torchmetrics/detection/helpers.py Outdated Show resolved Hide resolved
src/torchmetrics/functional/detection/ciou.py Outdated Show resolved Hide resolved
src/torchmetrics/functional/detection/ciou.py Show resolved Hide resolved
src/torchmetrics/functional/detection/diou.py Show resolved Hide resolved
src/torchmetrics/functional/detection/giou.py Show resolved Hide resolved
src/torchmetrics/functional/detection/iou.py Show resolved Hide resolved
twsl and others added 17 commits October 24, 2022 13:24
Co-authored-by: Nicki Skafte Detlefsen <[email protected]>
Co-authored-by: Nicki Skafte Detlefsen <[email protected]>
Co-authored-by: Nicki Skafte Detlefsen <[email protected]>
Co-authored-by: Nicki Skafte Detlefsen <[email protected]>
Co-authored-by: Nicki Skafte Detlefsen <[email protected]>
@mergify mergify bot removed the ready label Apr 14, 2023
@mergify mergify bot removed the has conflicts label Apr 15, 2023
@Borda Borda enabled auto-merge (squash) April 17, 2023 15:14
@mergify mergify bot removed the ready label Apr 17, 2023
@mergify mergify bot added the ready label Apr 17, 2023
@Borda Borda merged commit 25d347b into Lightning-AI:master Apr 18, 2023
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.

Add IoU/GIoU for boxes
5 participants