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 slow calculations of classification metrics #2876

Merged
merged 2 commits into from
Dec 24, 2024

Conversation

Isalia20
Copy link
Contributor

@Isalia20 Isalia20 commented Dec 23, 2024

What does this PR do?

Fixes #2868

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? Yes

Make sure you had fun coding 🙃

Details

The flag for the _TORCH_GREATER_EQUAL_1_12 was removed on the changed line in the following commit: 346bcdc
Now problem with the removal is that if before condition for checking was:
if torch.are_deterministic_algorithms_enabled() or _XLA_AVAILABLE or _TORCH_GREATER_EQUAL_1_12 and x.is_mps:

which meant if tensor was on mps and torch was greater or equal 1.12 it would choose the conditional path. With removal of the flag _TORCH_GREATER_EQUAL_1_12, now for the code to enter this path the XLA should be available and tensor should be on mps and I don't think there is such a device at all that supports both XLA and mps tensors. After changing the code the performance issue on the issue posted above gets fixed


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

Copy link

codecov bot commented Dec 23, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 69%. Comparing base (4d9c843) to head (7279d82).
Report is 3 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #2876    +/-   ##
=======================================
- Coverage      69%     69%    -0%     
=======================================
  Files         346     332    -14     
  Lines       19142   18966   -176     
=======================================
- Hits        13230   13055   -175     
+ Misses       5912    5911     -1     

@Borda Borda merged commit 6ba395d into Lightning-AI:master Dec 24, 2024
24 of 27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Large computation speed regression in version 1.6.0 on MPS device
2 participants