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

bugfix: BoxUniform device handling. #854

Merged
merged 1 commit into from
Jul 6, 2023
Merged

Conversation

janfb
Copy link
Contributor

@janfb janfb commented Jul 4, 2023

potential fix for #853.

a) We raise an error if the device passed via low/high does not match the device kward. E.g., if low/high have been moved to GPU already, but device is still on default "cpu".
b) Alternatively, we could automatically use the device passed via low/high as the new device to be less verbose and to follow the user's obvious intention to have the prior on that device.

For now, I implemented b). what do you think @michaeldeistler?

@janfb janfb force-pushed the fix-boxuniform-device-handling branch 2 times, most recently from c84cc15 to b7b8815 Compare July 4, 2023 09:22
@codecov
Copy link

codecov bot commented Jul 4, 2023

Codecov Report

Merging #854 (8bb676b) into main (86e73a9) will increase coverage by 0.01%.
The diff coverage is 83.33%.

@@            Coverage Diff             @@
##             main     #854      +/-   ##
==========================================
+ Coverage   74.59%   74.61%   +0.01%     
==========================================
  Files          80       80              
  Lines        6236     6244       +8     
==========================================
+ Hits         4652     4659       +7     
- Misses       1584     1585       +1     
Flag Coverage Δ
unittests 74.61% <83.33%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
sbi/utils/torchutils.py 64.62% <83.33%> (+0.79%) ⬆️

... and 3 files with indirect coverage changes

@janfb janfb force-pushed the fix-boxuniform-device-handling branch from b7b8815 to b988581 Compare July 5, 2023 08:32
@janfb janfb force-pushed the fix-boxuniform-device-handling branch from b988581 to 8bb676b Compare July 5, 2023 08:55
@janfb janfb self-assigned this Jul 5, 2023
@janfb janfb added the bug Something isn't working label Jul 5, 2023
Copy link
Contributor

@michaeldeistler michaeldeistler left a comment

Choose a reason for hiding this comment

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

I prefer option b)

Thanks a lot, LGTM!

@janfb janfb merged commit 44722c6 into main Jul 6, 2023
@janfb janfb deleted the fix-boxuniform-device-handling branch July 6, 2023 08:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants