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

Return input argument if no GPUs werde detected in get_gpu #4623

Merged
merged 4 commits into from
Dec 21, 2021

Conversation

masterleinad
Copy link
Contributor

Fixes #4621. @jczhang07 Can you please take a look if this works for you?

Essentially, the changes here are only relevant for the SYCL backend. If a specific device is requested (id>=0) we still error out if there are no GPUs. If that is not the case (id<0), we use the default selector to run on a host queue if no GPUs are detected. This (intended) behavior is not changed here but we have use_gpu return the input argument when no GPU is detected (instead of assigning GPU 0 which doesn't exist in that case).

Copy link
Member

@dalg24 dalg24 left a comment

Choose a reason for hiding this comment

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

This makes that code even less readable

@masterleinad
Copy link
Contributor Author

This makes that code even less readable

What is it specifically that you want me to change? Are you criticizing the intended behavior or do you merely want to change the implementation to be more readable?

In the latter case, we could copy

return sycl::device::get_devices(sycl::info::device_type::gpu).size();

before calling get_gpu in the SYCL case (although this function is then called twice for the regular case of running on a GPU). Would that make you accept the changes?

@jczhang07
Copy link
Contributor

@masterleinad The fix works for me. Thanks.

@masterleinad
Copy link
Contributor Author

@dalg24 I avoided modifying get_gpu now but moved the logic to the respective SYCL call site.

@masterleinad
Copy link
Contributor Author

Retest this please.

@jczhang07
Copy link
Contributor

jczhang07 commented Dec 20, 2021

Retest this please.
@masterleinad Retested and it worked. Thanks.

core/src/SYCL/Kokkos_SYCL.cpp Outdated Show resolved Hide resolved
Co-authored-by: Bruno Turcksin <bruno.turcksin@gmail.com>
@dalg24
Copy link
Member

dalg24 commented Dec 20, 2021

Retest this please

@jczhang07
Copy link
Contributor

Retest this please

Works here.

@masterleinad
Copy link
Contributor Author

Retest this please.

@masterleinad
Copy link
Contributor Author

Works here.

@jczhang07 The comments are not for you but for our CI. 🙂

@dalg24 dalg24 merged commit b9fe0c1 into kokkos:develop Dec 21, 2021
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.

Kokkos/SYCL: default initialization value (-1) does not work on machines without GPUs
4 participants