BUG: Remove CudaContextManager and use cudaSetDevice

[laurendeaumatthieu]

No description provided.

[SimonRit]

Thanks for the contribution. I think you are actually cleaning up legacy code which is no longer relevant since the introduction of a primary context in 2015... See Cuda 7 release notes.
I would suggest further cleaning up the code:

• remove the ContextManager class
• replace all calls to cuSetSetCurrent to calls to cudasetDevice following what is here.

Should I handle it or do you want to give it a try?

[laurendeaumatthieu]

I can give a try.
What do you mean by "replace all calls to cuSetSetCurrent to calls to cudasetDevice following what is here" ? I have removed all the cuCtxSetCurrent, you would like I replace them by CUDA_CHECK(cudaSetDevice(0)); ?

[SimonRit]

Sorry for the typo. Yes, that's what I mean and what the link suggests. It shouldn't be 0 but the number of the selected device. This selection could be done in the constructor.

[laurendeaumatthieu]

I made the changes. I let you run the tests. Please let me know if everything is ok for you

Matthieu

[SimonRit]

Thanks looks good. The CI indicates that you didn't commit the CMakeLists.txt file: https://my.cdash.org/build/2487527/configure. Can you also squash the changes in one commit only?
Be careful to have a commit subject "below 72 c, ideally 50" (see here). And it would be nice to have a bit more substance in the commit message to explain why we don't need a Cuda context anymore and why we think we need to reinitialize the device before every access (with the links I provided).

[laurendeaumatthieu]

Should be ok now

[SimonRit]

Thanks. I'm surprised that it compiles locally because there are still references to the context manager. At least the CI doesn't build https://my.cdash.org/viewBuildError.php?buildid=2488764, can you fix it?

I also think the commit body is wrong as is (since the Cuda context was not destroyed) + the list of CHANGES is not necessary so I suggest the following message for the commit body:

Use the primary context with cudaSetDevice() introduced by Cuda 7 (https://developer.download.nvidia.com/compute/cuda/7_0/Prod/doc/CUDA_Toolkit_Release_Notes.pdf) instead of a new one. cudaSetDevice is called before every memory transfer between the CPU and GPU to be sure that the context is set for the current thread, see https://developer.nvidia.com/blog/cuda-pro-tip-always-set-current-device-avoid-multithreading-bugs/.

[SimonRit]

Oops, erroneous deletion? https://my.cdash.org/viewBuildError.php?buildid=2488836

[laurendeaumatthieu]

Yes, sorry

[SimonRit]

Can you remove this long comment too? I think it was already obsolete. Sorry, I should have caught this before...

[laurendeaumatthieu]

Done

[SimonRit]

Thanks. Failing python packages are being solved in a separate issue, I'm merging this one.