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

PyTorch MMDDrift kernel not being sent to device #586

Closed
ascillitoe opened this issue Aug 15, 2022 · 4 comments · Fixed by #587
Closed

PyTorch MMDDrift kernel not being sent to device #586

ascillitoe opened this issue Aug 15, 2022 · 4 comments · Fixed by #587
Assignees
Labels
Priority: High Type: Bug Something isn't working

Comments

@ascillitoe
Copy link
Contributor

The MMDDriftTorch detector's kernel attribute is not sent to the correct device. See this comment.

The following:

self.kernel = kernel(sigma) if kernel == GaussianRBF else kernel

should be changed to:

self.kernel = kernel(sigma).to(self.device) if kernel == GaussianRBF else kernel
@ascillitoe ascillitoe added Priority: High Type: Bug Something isn't working labels Aug 15, 2022
@ascillitoe ascillitoe self-assigned this Aug 15, 2022
@ascillitoe
Copy link
Contributor Author

@jklaise do you think we should release a patch for this?

@jklaise
Copy link
Contributor

jklaise commented Aug 15, 2022

@ascillitoe probably since the next release would be at least a few weeks away. I was assuming it's a genuine bug, but weird that not spotted before @arnaudvl

@arnaudvl
Copy link
Contributor

Yes definitely a genuine bug, was very surprised when I ran into it during the KeOps PR. Worthy of a patch release I think.

@ascillitoe
Copy link
Contributor Author

OK I shall do this today 👍🏻

Also agree its weird that we didn't spot it earlier. I assume the kernel was being implicity copied over to device when called or something? Maybe we'll see a speedup now!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority: High Type: Bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants