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
Throw an exception in cms::cuda::chooseDevice() if CUDAService is disabled #32155
Conversation
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-32155/19850
|
A new Pull Request was created by @makortel (Matti Kortelainen) for master. It involves the following packages: HeterogeneousCore/CUDACore @makortel, @cmsbuild, @fwyzard can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
@cmsbuild, please test |
The tests are being triggered in jenkins.
|
+1 |
Comparison job queued. |
#include "HeterogeneousCore/CUDAServices/interface/CUDAService.h" | ||
|
||
#include "chooseDevice.h" | ||
|
||
namespace cms::cuda { | ||
int chooseDevice(edm::StreamID id) { | ||
edm::Service<CUDAService> cudaService; | ||
if (not cudaService->enabled()) { |
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.
Should this be
if (not cudaService->enabled()) { | |
if (not cudaService or not cudaService->enabled()) { |
to catch the case where the CUDAService
is not included in the process ?
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.
De-referencing the edm::Service<CUDAService>
without CUDAService
in the configuration gives the standard exception
Service Request unable to find requested service with compiler type name ' 11CUDAService'.
I thought that would be sufficient to cover that case.
Comparison is ready Comparison Summary:
|
+1 |
This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request will now be reviewed by the release team before it's merged. @silviodonato, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2) |
+1 |
PR description:
It was noticed in #31719 (comment) that there is a viable call path in modules using CUDA that leads to a crash instead of an exception when run on a machine without a GPU. This PR changes
cms::cuda::chooseDevice()
to require thatCUDAService
is enabled, and throw an explanatory exception if it is disabled.PR validation:
Edited
HeterogeneousCore/CUDATest/test/testCUDASwitch_cfg.py
to force CUDA being enabled to see the exception is being thrown on a machine without GPU.