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

Memory optimization of perplexity metric #2346

Merged
merged 16 commits into from
Feb 8, 2024

Conversation

nsmlzl
Copy link
Contributor

@nsmlzl nsmlzl commented Feb 2, 2024

What does this PR do?

Fixes #2337 ; reduces the memory footprint when computing the perplexity by not creating a huge temporary tensor with n^2 elements (n: number of elements of target tensor) and subsequently only selecting the diagonal elements.

Instead, this change only computes the diagonal elements from the beginning (tensor with just n elements).

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

Copy link

codecov bot commented Feb 2, 2024

Codecov Report

Merging #2346 (ef31bf3) into master (dc115fe) will decrease coverage by 29%.
Report is 1 commits behind head on master.
The diff coverage is 100%.

Additional details and impacted files
@@           Coverage Diff            @@
##           master   #2346     +/-   ##
========================================
- Coverage      69%     40%    -29%     
========================================
  Files         305     305             
  Lines       17149   17149             
========================================
- Hits        11832    6851   -4981     
- Misses       5317   10298   +4981     

@mergify mergify bot added ready and removed ready labels Feb 2, 2024
@SkafteNicki SkafteNicki added the enhancement New feature or request label Feb 4, 2024
@SkafteNicki SkafteNicki added this to the v1.3.x milestone Feb 4, 2024
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.

LGTM :)

@Borda Borda enabled auto-merge (squash) February 6, 2024 10:33
@mergify mergify bot added the ready label Feb 6, 2024
@mergify mergify bot added the has conflicts label Feb 7, 2024
@mergify mergify bot removed the has conflicts label Feb 7, 2024
@Borda Borda merged commit c0d2d3a into Lightning-AI:master Feb 8, 2024
59 checks passed
Borda pushed a commit that referenced this pull request Feb 12, 2024
* reduce memory footprint when computing perplexity

(cherry picked from commit c0d2d3a)
Borda pushed a commit that referenced this pull request Feb 12, 2024
* reduce memory footprint when computing perplexity

(cherry picked from commit c0d2d3a)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

High memory usage of Perplexity metric
3 participants