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

Iter2: store my output on the same device as the first input #572

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

njaard
Copy link

@njaard njaard commented Nov 18, 2022

In order for Iter2 to be usable with a GPU, you needed to do Iter2::new(...).to_device(vs.device()).shuffle()

Now with this patch, the to_device is not necessary, which is consistent with my expectations.

However, it has virtually no performance difference, positive or negative.

@vcarpani
Copy link
Contributor

vcarpani commented Dec 22, 2022

Thanks for this PR, what happens if xs and ys are on different devices?

Also, make sure to update the changelog adding an unreleased section.

In order for Iter2 to be usable with a GPU, you needed to do
Iter2::new(...).to_device(vs.device()).shuffle()

Now with this patch, the to_device is not necessary, which
is consistent with my expectations.

However, it has virtually no performance difference, positive
or negative.
@njaard
Copy link
Author

njaard commented Dec 22, 2022

@vcarpani It doesn't work, which is "philosophically" the same behavior as right now, but incompatible, because right now if the first parameter is GPU and the second is CPU, then Iter2 will work. With my patch. The first parameter must be the same as the second or you'll need to move the second it to the GPU.

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.

2 participants