-
Notifications
You must be signed in to change notification settings - Fork 93
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
fix Issue #481, ASSET tries to use a backend that is not present in the environment #485
fix Issue #481, ASSET tries to use a backend that is not present in the environment #485
Conversation
…_opencl_capability() to utils, to detect openCL devices
elephant/utils.py
Outdated
|
||
if len(platforms) == 0: | ||
return False | ||
if len(platforms) > 0: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This conditional is not needed, as the length is >= 0. If the length was zero, return
was triggered ending the function. If this part of the code is reached, then it is assumed to be greater than zero already, and the function can directly return True
. This also prevents the situation that it is still possible for the function to return None (if analyzing only the logic: in case any of the conditions are not met, no return statement is found)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, turned the conditional into a descriptive inline comment.
elephant/utils.py
Outdated
Returns | ||
------- | ||
bool | ||
True: if openCL platform detected, False: if OpenCL is not found or if |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is good to be explicit: True if OpenCL platform detected and at least one OpenCL device is found (as both conditions must apply to return True)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch, thank you. Updated doc-string accordingly.
I tested all the changes and everything works as expected now. Perhaps one thing to do in the future, if we end up having CI runners with GPU support and CUDA, is to add more unit tests to the current one. Thus, we can cover all cases for backend selection in the class. But I consider that this PR can be merged as it is now since the GPU CI is a further enhancement for Elephant and not part of this fix. |
This PR introduces a check for openCL similar to
get_cuda_capability_major()
to detect openCL devices.Issue #481 is addressed with this.