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.

Sign up for GitHub

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

Jump to bottom

Fix _cumsum helper function in multi-gpu #2636

Merged
merged 5 commits into from
Jul 22, 2024
Merged

Fix _cumsum helper function in multi-gpu #2636

merged 5 commits into from
Jul 22, 2024

Conversation

SkafteNicki
Copy link
Member

@SkafteNicki SkafteNicki commented Jul 21, 2024 •
edited by github-actions bot
Loading

What does this PR do?

Fixes #2608

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

@SkafteNicki SkafteNicki added the bug / fix Something isn't working label Jul 21, 2024
@SkafteNicki SkafteNicki added this to the v1.4.x milestone Jul 21, 2024
@codecov Codecov
Copy link

codecov bot commented Jul 21, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 0% with 1 line in your changes missing coverage. Please review.

Project coverage is 69%. Comparing base (55f036e) to head (c969b4e).
Report is 100 commits behind head on master.

Additional details and impacted files 
@@          Coverage Diff           @@
##           master   #2636   +/-   ##
======================================
  Coverage      69%     69%           
======================================
  Files         316     316           
  Lines       17881   17881           
======================================
  Hits        12331   12331           
  Misses       5550    5550

Borda
Borda approved these changes Jul 21, 2024
View reviewed changes
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.

Can we have a test for it please?

@mergify mergify bot added the ready label Jul 21, 2024
@SkafteNicki
Copy link
Member Author

Can we have a test for it please?

Added, but only because you asked nicely :P

@Borda Borda requested a review from baskrahmer July 22, 2024 07:57
@Borda Borda merged commit f7eeace into master Jul 22, 2024
61 of 65 checks passed
@Borda Borda deleted the bugfix/cumsum branch July 22, 2024 10:29
baskrahmer
baskrahmer reviewed Jul 22, 2024
View reviewed changes
Copy link
Collaborator

@baskrahmer baskrahmer left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM! Edit: Oh its merged already 😛

@baskrahmer baskrahmer self-requested a review July 22, 2024 16:15
Borda pushed a commit that referenced this pull request Aug 2, 2024
@SkafteNicki @Borda
Fix _cumsum helper function in multi-gpu (#2636)
75d1cf3 
* fixing staff
* add test case
* Apply suggestions from code review

---------

Co-authored-by: Jirka Borovec <6035284+Borda@users.noreply.github.com>
(cherry picked from commit f7eeace)
Borda pushed a commit that referenced this pull request Aug 2, 2024
@SkafteNicki @Borda
Fix _cumsum helper function in multi-gpu (#2636)
7b223ea 
* fixing staff
* add test case
* Apply suggestions from code review

---------

Co-authored-by: Jirka Borovec <6035284+Borda@users.noreply.github.com>
(cherry picked from commit f7eeace)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Borda Borda Borda approved these changes

@justusschock justusschock Awaiting requested review from justusschock justusschock is a code owner

@stancld stancld Awaiting requested review from stancld stancld is a code owner

@baskrahmer baskrahmer Awaiting requested review from baskrahmer

Assignees
No one assigned
Labels
bug / fix Something isn't working ready
Projects
None yet
Milestone
v1.4.x
Development

Successfully merging this pull request may close these issues.

_cumsum moves the data to default GPU when deterministic flag is on
3 participants
@SkafteNicki @Borda @baskrahmer