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 MAP metric for empty cases #624

Merged
merged 39 commits into from
Nov 25, 2021
Merged

Fix MAP metric for empty cases #624

merged 39 commits into from
Nov 25, 2021

Conversation

tkupek
Copy link
Contributor

@tkupek tkupek commented Nov 15, 2021

What does this PR do?

Fixes #601 (empty tensors in metric)
Fixes #626 (DDP stuck in multi GPU mode)
Fixes an issue if the metric is evaluated with empty lists

Tests where added for both cases

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 🙃

tobias-kupek-swarm and others added 21 commits October 29, 2021 21:54
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]>
Co-authored-by: Nicki Skafte Detlefsen <[email protected]>
@codecov
Copy link

codecov bot commented Nov 15, 2021

Codecov Report

Merging #624 (ff78b44) into master (6361a41) will increase coverage by 0%.
The diff coverage is 100%.

@@          Coverage Diff          @@
##           master   #624   +/-   ##
=====================================
  Coverage      95%    95%           
=====================================
  Files         160    160           
  Lines        5597   5605    +8     
=====================================
+ Hits         5326   5334    +8     
  Misses        271    271           

torchmetrics/detection/map.py Outdated Show resolved Hide resolved
@SkafteNicki
Copy link
Member

@tkupek How is it going here?

@SkafteNicki SkafteNicki enabled auto-merge (squash) November 24, 2021 12:30
Fix MAX DDP multi GPU issue
auto-merge was automatically disabled November 24, 2021 17:15

Head branch was pushed to by a user without write access

@tkupek
Copy link
Contributor Author

tkupek commented Nov 24, 2021

I just pushed another commit that fixes the issue mentioned in #626
@SkafteNicki helped me finding this

@Borda Borda enabled auto-merge (squash) November 24, 2021 17:21
auto-merge was automatically disabled November 24, 2021 17:22

Head branch was pushed to by a user without write access

@SkafteNicki SkafteNicki enabled auto-merge (squash) November 24, 2021 17:35
auto-merge was automatically disabled November 24, 2021 18:02

Head branch was pushed to by a user without write access

@mergify mergify bot added the ready label Nov 24, 2021
@SkafteNicki SkafteNicki merged commit 3aef7be into Lightning-AI:master Nov 25, 2021
Borda pushed a commit that referenced this pull request Dec 5, 2021
* add detection map code example

* update setup

* simplify named tuple

* Update tm_examples/detection_map.py

* Update tm_examples/detection_map.py

* Update tm_examples/detection_map.py

* Update tm_examples/detection_map.py

* Update tm_examples/detection_map.py

* Update tm_examples/detection_map.py

* add some more comments

* add some more comments

* add example hint in metric docstring

* fix evaluation for empty metric

* Update torchmetrics/detection/map.py

* fix deepsource stuff

* update changelog

* fix ddp issue in multi GPU setup for empty boxes

* update doc

* simplify empty tensors fix

* fix mypy

* fix failing unittests

Co-authored-by: Jirka <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Nicki Skafte Detlefsen <[email protected]>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Co-authored-by: Jirka Borovec <[email protected]>
(cherry picked from commit 3aef7be)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug / fix Something isn't working ready
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DDP mode stuck with multiple GPUs when calling MAP metric MAP handling an empty prediction array.
4 participants