-
Notifications
You must be signed in to change notification settings - Fork 54
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
added device call after the bcast in the getitem #790
Conversation
Codecov Report
@@ Coverage Diff @@
## master #790 +/- ##
==========================================
- Coverage 90.85% 90.84% -0.01%
==========================================
Files 64 64
Lines 8932 8934 +2
==========================================
+ Hits 8115 8116 +1
- Misses 817 818 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
ignore the diff hit please, im not sure why it only hits it on the master branch. |
Hey @coquelin77 I'm a bit lost, is #784 fixed? In which case, we should be able to switch back to |
i thought that it was fixed because it solved this problem, but it still caused the same issue that @ben-bou pointed out before. so i left it as |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @coquelin77 , I'm OK with this, maybe just add a note that the fix won't (shouldn't) be necessary after solving #784?
Description
a minor bug was showing up where the
bcast
operation was not putting the data onto the proper GPU in the__getitem__
function. the code added may not be hit all the time, but it is safeguard for when this may happen.Issue/s resolved: n/a
Changes proposed:
bcast
when there are integers in the getitem keyType of change
Due Diligence
Does this change modify the behaviour of other functions? If so, which?
no