refine device_context #2814
refine device_context #2814
Conversation
QiJune
changed the title
QiJune
force-pushed
the
feature/refine_device_context
branch
from
a15eab4
to
ca23d86
Compare
| GPUPlaceGuard guard(gpu_place_); | ||
| paddle::platform::throw_on_error(cudaStreamCreate(&stream_), | ||
| "cudaStreamCreate failed"); | ||
| eigen_stream_ = new Eigen::CudaStreamDevice(&stream_); |
There was a problem hiding this comment.
Maybe std::unique_ptr is a better idea?
|
|
||
| cudaStream_t stream() { return stream_; } | ||
|
|
||
| Eigen::GpuDevice eigen_device() { return *eigen_device_; } |
There was a problem hiding this comment.
Maybe return a reference or pointer is better? because Eigen::GpuDevice's copy ctor is invoked when eigen_device is invoked.
There was a problem hiding this comment.
Eigen::GpuDevice only contains a pointer and int, so return by value is acceptable
|
|
||
| template <> | ||
| Eigen::GpuDevice DeviceContext::get_eigen_device<Eigen::GpuDevice>() { | ||
| return dynamic_cast<CUDADeviceContext*>(this)->eigen_device(); |
There was a problem hiding this comment.
dynamic_cast --> reinterpret_cast.
|
|
||
| cudnnHandle_t dnn_handle_{nullptr}; | ||
|
|
||
| int random_seed_; |
There was a problem hiding this comment.
random_seed_ cannot be set or not properly initialized?
There was a problem hiding this comment.
I check the cuRand doc.
Set the seed value of the pseudorandom number generator. All values of seed are valid. Different seeds will produce different sequences. Different seeds will often not be statistically correlated with each other, but some pairs of seed values may generate sequences which are statistically correlated.
So, the random_seed_ can be any value. And caffe2 does so.
paddle/platform/device_context.h
Outdated
| paddle::platform::SetDeviceId(new_place.device); | ||
| Eigen::DefaultDevice eigen_device() { | ||
| if (!eigen_device_) { | ||
| eigen_device_ = new Eigen::DefaultDevice(); |
There was a problem hiding this comment.
Also, unique_ptr is better.
paddle/platform/device_context.h
Outdated
| explicit GPUPlaceGuard(GPUPlace new_place) : previous_(GetCurrentDeviceId()) { | ||
| if (previous_ != new_place) { | ||
| paddle::platform::SetDeviceId(new_place.device); | ||
| Eigen::DefaultDevice eigen_device() { |
There was a problem hiding this comment.
Eigen::DefaultDevice have no data member. So it's fine to return by value
paddle/platform/CMakeLists.txt
Outdated
|
|
||
| cc_library(device_context SRCS device_context.cc DEPS place eigen3 ${GPU_CTX_DEPS}) |
There was a problem hiding this comment.
Make device_context a library is a better idea, otherwise, each library which deps on device_context should handle complex GPU libraries.
No description provided.