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 macro when ignore_index is set #2163

Draft
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

edumotya
Copy link

@edumotya edumotya commented Oct 10, 2023

What does this PR do?

Fixes #1691

Additionally, tests have been updated to exclude the ignored class from the macro accuracy averaging.

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--2163.org.readthedocs.build/en/2163/

@Borda Borda changed the title fix macro when ignore_index is set fix macro when ignore_index is set Oct 17, 2023
@@ -416,6 +416,8 @@ def _multiclass_stat_scores_update(
fp = confmat.sum(0) - tp
fn = confmat.sum(1) - tp
tn = confmat.sum() - (fp + fn + tp)
if ignore_index is not None:
fp[ignore_index] = 0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is probably correct, but since many metrics derive from the stat_scores class that means that basically all would need to have their unittests fixed

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what do you mean by it needs to be fixed?

@Borda
Copy link
Member

Borda commented Nov 14, 2023

@edumotya how is it going, mind checking comment from Nicki

@Borda
Copy link
Member

Borda commented Jan 9, 2024

@edumotya seems like most of the tests are off; could you pls check it...
turning it back to draft until most other checks are green... pls make it as ready to review when you resolve them... 🦩

@edumotya
Copy link
Author

sure @Borda, I am sorry for the late response. It might take me some time, it is not straightforward to fix the affected tests, I'll do my best

@ding3820
Copy link

Any update?

@Borda Borda force-pushed the master branch 2 times, most recently from d0a5568 to 9fc79ae Compare September 16, 2024 08:18
@Borda
Copy link
Member

Borda commented Oct 31, 2024

Any update?

I think this is related #2710

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.

Accuracy is wrong when ignore_index is set
4 participants