apeforest commented on a change in pull request #12374: Fix/public internal
header
URL: https://github.com/apache/incubator-mxnet/pull/12374#discussion_r216435843
##########
File path: include/mxnet/random_generator.h
##########
@@ -150,14 +149,9 @@ class RandGenerator<gpu, DType> {
curandStatePhilox4_32_10_t state_;
}; // class RandGenerator<gpu, DType>::Impl
- static void AllocState(RandGenerator<gpu, DType> *inst) {
- CUDA_CALL(cudaMalloc(&inst->states_,
- kNumRandomStates *
sizeof(curandStatePhilox4_32_10_t)));
- }
+ static void AllocState(RandGenerator<gpu, DType> *inst);
Review comment:
@azai91 Thanks for your efforts in refactoring this. Since the main purpose
of this PR is refactoring, I hope we can do it in the most elegant way. If we
are making this file a public header, I would extract the implementation class
Impl to another internal header file so that we do not expose the internal
implementation details. By doing that, we can have put the implementation
details for both CPU and GPU there. Please let me know your thoughts.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
