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

Add torch device interface and clean up. #16

Closed
wants to merge 2 commits into from
Closed

Add torch device interface and clean up. #16

wants to merge 2 commits into from

Conversation

ehsanmok
Copy link
Contributor

@ehsanmok ehsanmok commented Mar 6, 2019

The current device implementation doesn't do proper error handling, since unsafe_torch! were used everywhere instead of unsafe_torch_err! so for example, cuda_is_available or Device::cuda_if_available() shouldn't just panic due to any reason in the client code. Also device_wrapped.rs doesn't provide any safety and is redundant.

This PR fixes these issues.

@ehsanmok
Copy link
Contributor Author

ehsanmok commented Mar 6, 2019

@LaurentMazare ping.

@ehsanmok ehsanmok mentioned this pull request Mar 7, 2019
26 tasks
@LaurentMazare
Copy link
Owner

Actually I don't think these functions should ever fail on the C++ side so i'm not sure we should make them fallible on the rust side neither. I recently tried to add some fallible versions for the functions that I believe could fail.

@ehsanmok
Copy link
Contributor Author

@LaurentMazare I also checked the ATen related parts and I didn't find any exception handling, so they seem be unwrapped safely. I changed those back to unsafe_torch!. Please check.

@ehsanmok ehsanmok changed the title Idiomatic device type with error handling methods. Add torch device interface and clean up. Mar 10, 2019
@LaurentMazare
Copy link
Owner

Closing this as the code has significantly diverged since then (e.g. device now include a gpu id for multi-gpu training).

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.

None yet

2 participants