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

Adds support for torch MPS backend #11

Merged
merged 4 commits into from
Sep 15, 2023
Merged

Conversation

magnusross
Copy link
Contributor

I think the only required change is to how the random states/generators are dealt with.

I have done a bit of a hacky initial implementation, there is probably a cleaner way to do it. I didn't yet workout how to write some tests for it, but can have a go at that if needed.

Cheers!

Copy link
Owner

@wesselb wesselb left a comment

Choose a reason for hiding this comment

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

@magnusross thanks for much for this. :)

I think this is the right way to go about this. I've left a few comments/suggestions.

In addition, I think a unit test to test the logic here would be helpful. I could also implement that myself, if you'd like.

lab/torch/random.py Outdated Show resolved Hide resolved
lab/torch/random.py Outdated Show resolved Hide resolved
lab/torch/random.py Show resolved Hide resolved
lab/torch/random.py Outdated Show resolved Hide resolved
@magnusross
Copy link
Contributor Author

Hey Wessel, thanks for looking at this, I've fixed the suggestions, but I haven't yet done any tests, I couldn't immediately see how to do it, and am a bit busy with some other stuff right now. Happy to have a go at it later in the week!

@wesselb
Copy link
Owner

wesselb commented Sep 15, 2023

@magnusross absolutely not a problem! Happy to implement a simple test myself. I think your implementation is basically ready to go, so I'm merging this right away :)

Thanks again for the contribution!!

@wesselb wesselb merged commit 8c45363 into wesselb:master Sep 15, 2023
4 of 5 checks passed
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