Support CPU only memcpy#633
Conversation
|
@shelhamer, @jeffdonahue, @sguada, the very simple changes are tested and ready to be merged. |
|
See comments at #604; I'm inclined to merge this soon. To be super pedantic, I'd rather the check be for
|
|
Agreed with @longjon's #604 (comment) and the pedantry. Only obstacle to merge is that this causes a bus error on a lot of GPU tests if you do have a GPU? |
|
@shelhamer, @longjon, SyncedMemory automatically switches to GPU mode when data is transferred between different devices. Both platforms with and without GPU have been tested. There is no longer any problem. |
src/caffe/syncedmem.cpp
Outdated
|
https://github.com/kloudkl/caffe/commit/ec967a5e38e78b15e75184803d5b97f018cd78cc makes me nervous. It seems one could do some work in GPU mode, tell caffe to switch to CPU mode, but be silently rebuffed and end up back in GPU mode. (Or, even without mode switching, one could start in CPU mode, do some GPU fiddling unrelated to running the net, and suddenly be in GPU mode.) In general I think How did we get there? The tests crash without that last commit; It looks like some of them assume that it's fine to invoke GPU memory in CPU mode. So what should it mean to be in CPU mode?
Currently it means something more like the latter, but that means Note that if a GPU is physically present, it's always okay to call |
|
The last resort is cuPointerGetAttribute. |
|
cuPointerGetAttribute relies on the CUDA driver. It's not applicable on the CPU. libgpuarray recommended by @bhack wraps the devices pointers to abstract out the differences. The pointers must carry the pointer types with themselves to discriminate easily. |
|
It makes me a little nervous too... I feel that syncedmem actually abuses caffe_memcpy a little bit (if it was me who wrote the code, blame me :)). Since it knows exactly which direction the copy is being carried out, it should explicitly call cudaMemcpy, which I assume would partially solve the problem @kloudkl points out? Setting a global variable to change things is a little flaky (and may introduce thread safety issues). |
|
Happy ending! |
|
Calling Searching through the code, note that
Thoughts? (@shelhamer, how do feel about these options w.r.t. #555?) As these are now rather fine points which can be corrected down the road, I intend in any case to merge one of these options later today to fix the issue when running without GPU. |
|
I vote for caffe_gpu_memcpy as you suggested. caffe_memcpy is quite a This fixes the issue, maintains the calling convention, and still lets us Le mercredi 9 juillet 2014, longjon notifications@github.com a écrit :
|
|
Finally done. |
|
Passes tests, warn, and lint; merged. Thanks @kloudkl for your patience and keeping this up-to-date with the discussion. We should also add comments at some point explaining that mode should be set before calling |
Support CPU only memcpy
Support CPU only memcpy
On a machine without GPU and that can't install the CUDA driver, #555 causes the error
Check failed: error == cudaSuccess (35 vs. 0) CUDA driver version is insufficient for CUDA runtime version.This PR adds an option to fall back in such situation.