Repair nccl op test #8575
Repair nccl op test #8575
Conversation
| @@ -121,27 +134,11 @@ class NCCLTester : public ::testing::Test { | |||
| std::vector<p::DeviceContext *> dev_ctxs; | |||
| f::Scope g_scope; | |||
| std::mutex mu; | |||
| std::vector<int> gpu_list; | |||
There was a problem hiding this comment.
Data members of classes should be with a trailing underscore.
refer : Variable Names
| auto op = f::OpRegistry::CreateOp(*op1); | ||
| VLOG(1) << "invoke NCCLInitOp."; | ||
| op->Run(g_scope, cpu_place); | ||
| VLOG(1) << "NCCLInitOp finished."; | ||
| } | ||
|
|
||
| int GetGPUData(int gpu_id) { return gpu_id + 42; } |
There was a problem hiding this comment.
This function is necessary, because
Simply set GPU data = gpu_id won't expose the incorrect linking error. More specifically, Paddle might be compiled with NCCL1.3 header while dynamically linked with NCCL2.so.
| @@ -97,7 +108,7 @@ class NCCLTester : public ::testing::Test { | |||
| send_tensor->Resize(kDims); | |||
| send_tensor->mutable_data<T>(kDims, place); | |||
There was a problem hiding this comment.
Maybe line 108 is unnecessary.
| @@ -97,7 +108,7 @@ class NCCLTester : public ::testing::Test { | |||
| send_tensor->Resize(kDims); | |||
| send_tensor->mutable_data<T>(kDims, place); | |||
|
|
|||
| std::vector<T> send_vector(f::product(kDims), gpu_id); | |||
| std::vector<T> send_vector(f::product(kDims), GetGPUData(gpu_id)); | |||
| paddle::framework::TensorFromVector<T>(send_vector, *ctx, send_tensor); | |||
| ctx->Wait(); | |||
There was a problem hiding this comment.
Is it necessary to synchronize here? I think the copying will synchronize the GPU and CPU in line 179.
There was a problem hiding this comment.
The copying is cudaMemcpyAsync. Looks like we need to add a wait to line 179...
Paddle/paddle/fluid/memory/memcpy.cc
Lines 30 to 36 in 0d49b92
There was a problem hiding this comment.
I don't think so because the memory is pageable in CPU side, the copying doesn't return immediately until the copy has completed.
The current CUDA Runtime Documentation states:
Asynchronous(Memcpy):
- For transfers from device memory to pageable host memory, the function will return only once the copy has completed.
There was a problem hiding this comment.
fix #8582