ENH: Implement GPU device selection functionality - #106
Conversation
79cb52f to
daf94ff
Compare
SimonRit
left a comment
SimonRit
left a comment
There was a problem hiding this comment.
Thanks. I'm not too fond of the proposed implementation. If I understand it well, you're using the environment to store the value. I don't think ITKCudaCommon should change the env. Moreover, this is probably not thread safe. I would recommend something similar to m_GlobalDefaultNumberOfThreads in ITK:
- if initialized, use the values initialized,
- else if defined, read it from env
- else from the system specifications (current behavior)
See https://github.com/InsightSoftwareConsortium/ITK/blob/main/Modules/Core/Common/src/itkMultiThreaderBase.cxx#L271-L341.
As far as I understand, the value is stored in a staticitk::MultiThreaderBaseGlobalsmember structure. We can do that or have two static members (the default value and the mutex to change it).
To be discussed if you'd like :-).
daf94ff to
0c126ee
Compare
|
Since SWIG can't wrap free functions, the device selection is exposed as static CudaDataManager methods, surfaced in Python through the convenient itk.set_default_cuda_device helper. |
|
I think we need to handle the case where a gpu is selected for example on torch but another on cudacommon. |
Add the ability to select the GPU used by all freshly created itk::CudaDataManager / itk::CudaImage objects instead of implicitly picking the device with the maximum FLOPS. A value of -1 (the default) keeps the previous automatic behavior (max FLOPS device). The device is resolved with the following precedence: 1. The value set explicitly via itk::SetDefaultCudaDevice; 2. Otherwise the ITK_CUDA_DEFAULT_DEVICE environment variable, if set; 3. Otherwise the device with the maximum FLOPS. The value is stored in a mutex-protected static, mirroring itk::MultiThreaderBase's pattern, and the environment variable is only read, never written, by the module. Passing an out-of-range index throws an itk::ExceptionObject. Since ITK's wrapping cannot bind free functions, the selection is also exposed through static CudaDataManager::SetDefaultDevice/GetDefaultDevice methods, which the Python itk.set_default_cuda_device helper calls. Add C++ and Python tests covering the precedence and error cases.
0c126ee to
93eb2b1
Compare
SimonRit
left a comment
SimonRit
left a comment
There was a problem hiding this comment.
Thanks, much better. Some minor changes requested. I'm not very fond of the delegates function, wouldn't wrapping the function work, like e.g. in PCT's MostLikelyPathFunction.i ?
| } | ||
|
|
||
| /** Set the default device used by all subsequently created CudaDataManagers. | ||
| * Use -1 to reset to automatic selection (max FLOPS device). Delegates to itk::SetDefaultCudaDevice. */ |
There was a problem hiding this comment.
I'd say "Use -1 to automatically select the (first) device with the max FLOPS."
| static void | ||
| SetDefaultDevice(int device); | ||
|
|
||
| /** Get the current default device (-1 means auto / max FLOPS). Delegates to itk::GetDefaultCudaDevice. */ |
There was a problem hiding this comment.
Idem "Get the current default device. -1 means automated selection of the (first) device with the max FLOPS."
| CudaGetMaxFlopsDev(); | ||
|
|
||
| /** Set the default device used by all subsequently created CudaDataManagers. | ||
| * Use -1 to reset to automatic (max FLOPS) selection. Overrides the |
There was a problem hiding this comment.
Idem "Use -1 to automatically select the (first) device with the max FLOPS."
| void CudaCommon_EXPORT | ||
| SetDefaultCudaDevice(int device); | ||
|
|
||
| /** Get the current default device (-1 means auto / max FLOPS). |
There was a problem hiding this comment.
Idem "Get the current default device. -1 means automated selection of the (first) device with the max FLOPS."
| SetDefaultCudaDevice(int device); | ||
|
|
||
| /** Get the current default device (-1 means auto / max FLOPS). | ||
| * Precedence: explicitly set value > ITK_CUDA_DEFAULT_DEVICE env var > auto. */ |
There was a problem hiding this comment.
A bit cryptic to me. "Explicitely set value takes precedence over the ITK_CUDA_DEFAULT_DEVICE which itself takes precedence over the automated value."
| m_Device = itk::CudaGetMaxFlopsDev(); | ||
| } |
There was a problem hiding this comment.
This block should be moved to GetDefaultCudaDevice in my opinion.
| { | ||
| // The explicitly-set default device (-1 = auto). Only meaningful once | ||
| // IsInitialized is true. Env var is only a fallback until then. | ||
| bool IsInitialized{ false }; |
There was a problem hiding this comment.
I would have dropped the bool and raised an exception if the env variable is not strictly positive.
2 participants
No description provided.