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

Multiclass-jaccard: fix off-by-one issue when ignore_index = num_classes + 1 #1860

Merged
merged 6 commits into from
Jun 28, 2023
Merged

Multiclass-jaccard: fix off-by-one issue when ignore_index = num_classes + 1 #1860

merged 6 commits into from
Jun 28, 2023

Conversation

martinmeinke
Copy link
Contributor

@martinmeinke martinmeinke commented Jun 27, 2023

What does this PR do?

Fixes #1861
If I interpret the code correctly, the condition determines whether the ignore_index lies within num_classes, in which case it is off-by-one.

Before submitting
  • Was this discussed/agreed 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 🙃


📚 Documentation preview 📚: https://torchmetrics--1860.org.readthedocs.build/en/1860/

@martinmeinke martinmeinke changed the title fix off-by-one issue when ignore_index = num_classes + 1 Multiclass-jaccard: fix off-by-one issue when ignore_index = num_classes + 1 Jun 27, 2023
@martinmeinke martinmeinke marked this pull request as ready for review June 27, 2023 23:45
@SkafteNicki SkafteNicki added the bug / fix Something isn't working label Jun 28, 2023
@SkafteNicki SkafteNicki added this to the v1.0.0 milestone Jun 28, 2023
@mergify mergify bot added the ready label Jun 28, 2023
@SkafteNicki SkafteNicki enabled auto-merge (squash) June 28, 2023 06:24
@codecov
Copy link

codecov bot commented Jun 28, 2023

Codecov Report

Merging #1860 (9ea35fb) into master (cc427ba) will decrease coverage by 37%.
The diff coverage is 100%.

Additional details and impacted files
@@           Coverage Diff            @@
##           master   #1860     +/-   ##
========================================
- Coverage      87%     51%    -37%     
========================================
  Files         256     256             
  Lines       14365   14365             
========================================
- Hits        12563    7274   -5289     
- Misses       1802    7091   +5289     

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.

added chlog entry

@SkafteNicki SkafteNicki merged commit 8fef294 into Lightning-AI:master Jun 28, 2023
@martinmeinke martinmeinke deleted the bugfix/multiclass_jaccard_ignore_off_by_1 branch June 29, 2023 19:42
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.

off-by-one in MultiClassJaccard ignore_index handling
3 participants