Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
Fix invalid mode changes during tests #2511
Conversation
flx42
added some commits
May 26, 2015
shelhamer
added the
testing
label
May 30, 2015
|
This looks good to me. Independent of deciding what's right for mode and device in general, templated tests seem like a more robust approach to making sure the mode is right. Exactly what to do with mode and device is an ongoing conversation, but I think diffusing mode + device to I don't know that we ever fully converged on this however. @longjon @jeffdonahue comment if you remember any threads. |
|
LGTM too. Not sure what we'll end up doing with |
shelhamer
added a commit
that referenced
this pull request
May 30, 2015
|
|
shelhamer |
3cc9bac
|
flx42 commentedMay 26, 2015
Some existing tests are modifying the Caffe mode halfway through the execution, this is documented to be invalid:
https://github.com/BVLC/caffe/blob/8df472/include/caffe/common.hpp#L140-L143
If, for performance reasons, host memory is allocated through
cudaMallocHost, changing the mode halfway can cause a pointer returned bycudaMallocHostto be freed byfree(2), resulting in undefined behavior. The reciprocal is also possible. Another possible issue is that if some tests incorrectly assume that the default mode is CPU, the test could actually run on the GPU if the previous test clobbered the global mode. See the full analysis of this issue in #2398The solution is, IMHO, to forbid calls to
Caffe::set_mode()in individual test cases, this function should only be called by the test framework in order to limit the risks of a misuse. To achieve this, the following patch set reuses the existingMultiDeviceTestclass and similarly add new classesGPUDeviceTestandCPUDeviceTest. In the case where we need to share code between CPU and GPU tests, the shared test code can directly derive from classMultiDeviceTestbut derived classes needs to be defined for CPU and GPU.