add cuda generator #26786

yaoxuefeng6 · 2020-08-29T05:13:45Z

PR types

Function optimization

PR changes

Others

Describe

add cuda generator

paddle-bot-old · 2020-08-29T05:13:53Z

Thanks for your contribution!
Please wait for the result of CI firstly. See Paddle CI Manual for details.

zhiqiu · 2020-09-01T02:55:33Z

paddle/fluid/framework/generator.cc

+static int64_t num_cuda_devices = -1;
+static std::once_flag num_devices_init_flag;
+static std::deque<std::once_flag> cuda_device_flags;
+
+static std::vector<std::shared_ptr<Generator>> default_cuda_generators;


Try to avoid global variables that are not of POD Type. You can use static local variables initialized inside functions instead.

zhiqiu · 2020-09-01T02:56:12Z

paddle/fluid/framework/generator.cc

+#endif
+}
+
+const std::shared_ptr<Generator>& getDefaultCUDAGenerator(int64_t device_id) {


getDefaultCUDAGenerator -> GetDefaultCUDAGenerator

zhiqiu · 2020-09-01T03:08:58Z

paddle/fluid/framework/generator.h

@@ -38,6 +38,7 @@ static uint64_t GetRandomSeed() {
 struct GeneratorState {
  int64_t device = -1;
  uint64_t current_seed = 34342423252;
+  uint64_t thread_offset;


Set the default value of thread_offset.

zhiqiu · 2020-09-01T03:11:14Z

paddle/fluid/framework/generator.h

    this->engine_ = engine;
    VLOG(4) << "initial seed: " << this->state_.current_seed
            << ", cpu engine: " << &this->state_.cpu_engine;
    this->is_init_py_ = true;  // TODO(zhiqiu): remove it in future
  }
+  explicit Generator(uint64_t seed, uint64_t device_id) {


explicit is not necessary for constructor that has more than one argument?

zhiqiu · 2020-09-01T03:15:35Z

paddle/fluid/operators/dropout_op.cu

+      int64_t device_id = -1;
+      auto gen_cuda = framework::getDefaultCUDAGenerator(device_id);
+      if (gen_cuda->GetIsInitPy() && (context.Attr<bool>("fix_seed"))) {
+        std::cout << ">>>>>>>>CUDA DROPOUT GENERATOR" << std::endl;
+        auto seed_offset = gen_cuda->IncrementOffset(1);


Can we use a common function to abstract this logic?

zhiqiu · 2020-09-01T03:16:20Z

python/paddle/framework/random.py

+    return state_list
+
+
+def set_cuda_state(state_list):


Same above.

zhiqiu · 2020-09-01T03:16:56Z

python/paddle/framework/random.py

    core.default_cpu_generator()._is_init_py = True
    return core.default_cpu_generator().manual_seed(seed)


+def get_cuda_state():


If it is a public API, please add some docs.

zhiqiu · 2020-09-01T03:20:12Z

paddle/fluid/operators/bernoulli_op.cu

+    int64_t device_id = -1;
+    auto gen_cuda = framework::getDefaultCUDAGenerator(device_id);
+    if (gen_cuda->GetIsInitPy() && seed_flag) {
+      std::cout << ">>>>>>>>CUDA bernoulli GENERATOR" << std::endl;
+      // auto seed_offset = gen_cuda->IncrementOffset(1);
+      auto seed_gen = static_cast<int>(gen_cuda->GetCurrentSeed());
+      // int offset_step = 0;
+      // NOTE(xuefeng): Currently, we let offset step fixed to avoid
+      // unexpected results which may cause ut fail.
+      // we will fix this in future.
+      // int gen_offset = offset_step * seed_offset.second;
+      trans(*context, index_sequence_begin, index_sequence_begin + size,
+            in_data, out_data, BernoulliCudaFunctor<T>(seed_gen));


Why only use seed instead of the random cuda engine?

Fixed. This is in debug.

zhiqiu · 2020-09-01T03:22:11Z

python/paddle/framework/random.py

+        for i in range(core.get_cuda_device_count()):
+            core.default_cuda_generator(i).set_state(state_list[i])
+
+
 def _manual_program_seed(seed):


Can we delete _manual_program_seed after the CUDA generator is implemented, and remove the usage in unit tests?

We can do this in next pr

jzhang533

get_cuda_state / set_cuda_state
只看这两个API的名字，用户是不是会误以为是设置cuda的每个状态的，而不会联想到随机数？

zhiqiu · 2020-09-03T07:30:00Z

python/paddle/fluid/tests/unittests/test_cuda_random_seed.py

+        #print("x: {}".format(x_np))
+        #print("x1: {}".format(x1_np))
+        #print("x2: {}".format(x2_np))
+        #print("x3: {}".format(x3_np))


Remove unused code.

zhiqiu · 2020-09-03T07:32:15Z

paddle/fluid/framework/generator.h

@@ -77,7 +77,7 @@ struct Generator {
    this->engine_ = engine;
    VLOG(4) << "initial seed: " << this->state_.current_seed
            << ", cpu engine: " << &this->state_.cpu_engine;
-    this->is_init_py_ = true;  // TODO(zhiqiu): remove it in future
+    this->is_init_py_ = false;  // TODO(zhiqiu): remove it in future


Why the default is false?

zhiqiu · 2020-09-03T07:36:33Z

paddle/fluid/operators/dropout_op.cu

-      auto gen_cuda = framework::getDefaultCUDAGenerator(device_id);
-      if (gen_cuda->GetIsInitPy() && (context.Attr<bool>("fix_seed"))) {
-        std::cout << ">>>>>>>>CUDA DROPOUT GENERATOR" << std::endl;
+      auto gen_cuda = framework::GetDefaultCUDAGenerator(-1);


Why the device_id is -1 here? I think it should be got by the context.

zhiqiu · 2020-09-03T07:38:05Z

paddle/fluid/framework/generator.cc

+    num_cuda_devices = paddle::platform::GetCUDADeviceCount();
+    cuda_device_flags.resize(num_cuda_devices);
+    default_cuda_generators.resize(num_cuda_devices);
+  });
  platform::Place place;
  if (device_id == -1)
    device_id = BOOST_GET_CONST(platform::CUDAPlace, place).GetDeviceId();


In the case, the originally given device_id is -1, what the number after this?

zhiqiu · 2020-09-03T07:40:07Z

python/paddle/framework/random.py

+        .. code-block:: python
+
+            import paddle
+            gen = paddle.manual_seed(102)


This line seems unused.

zhiqiu · 2020-09-03T07:41:57Z

paddle/fluid/pybind/generator_py.cc

 }  // end Generator
 }  // end namespace pybind
 }  // namespace paddle


Maybe not introduced by your PR. It seems the namespace endings are not of the same style.

zhiqiu · 2020-09-03T07:46:08Z

get_cuda_state / set_cuda_state
只看这两个API的名字，用户是不是会误以为是设置cuda的每个状态的，而不会联想到随机数？

同意，建议 paddle.set_cuda_rng_state/get_cuda_rng_state or paddle.cuda.set_rng_state/paddle.cuda.get_rng_state

XiaoguangHu01

LGTM

jzhang533

lgtm

add cuda generator

869c885

yaoxuefeng6 added 2 commits August 31, 2020 13:18

support initializer with cuda generator

3f74c1e

fix mac buid fail

5ce3939

PaddlePaddle locked and limited conversation to collaborators Aug 31, 2020

PaddlePaddle unlocked this conversation Aug 31, 2020

yaoxuefeng6 requested review from zhiqiu and jzhang533 August 31, 2020 08:13

yaoxuefeng6 added 3 commits August 31, 2020 17:55

fix build error

d8c58a3

fix ut fail

b7df9fc

fix ut fail

dfa5d03

zhiqiu reviewed Sep 1, 2020

View reviewed changes

yaoxuefeng6 added 9 commits September 1, 2020 12:30

fix ut

9a8e89b

fix ut fail

c8e3dca

fix ut fail

09e31a8

fix ut fail

95aa055

fix ut fail

f4946e3

fix ut fail

8f8bf9c

polish code and fix ut fail

288eaef

fix ut fail and polish codes

a412135

fix ci fail

158c5fc

yaoxuefeng6 requested a review from XiaoguangHu01 September 3, 2020 06:55

jzhang533 reviewed Sep 3, 2020

View reviewed changes

zhiqiu reviewed Sep 3, 2020

View reviewed changes

yaoxuefeng6 added 2 commits September 3, 2020 16:50

fix device id and add get/set_cuda_rng_state api

766799e

fix style

c8360b6

XiaoguangHu01 approved these changes Sep 3, 2020

View reviewed changes

jzhang533 approved these changes Sep 4, 2020

View reviewed changes

yaoxuefeng6 merged commit 7f3e6ca into PaddlePaddle:develop Sep 4, 2020

yaoxuefeng6 added a commit to yaoxuefeng6/Paddle that referenced this pull request Sep 4, 2020

add cuda generator (PaddlePaddle#26786)

fc40ea3

yaoxuefeng6 mentioned this pull request Sep 4, 2020

add cuda generator (#26786) #27014

Merged

yaoxuefeng6 added a commit that referenced this pull request Sep 4, 2020

add cuda generator (#26786) (#27014)

0a9f9f9

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add cuda generator #26786

add cuda generator #26786

yaoxuefeng6 commented Aug 29, 2020

paddle-bot-old bot commented Aug 29, 2020

zhiqiu Sep 1, 2020

yaoxuefeng6 Sep 3, 2020

zhiqiu Sep 1, 2020

yaoxuefeng6 Sep 3, 2020

zhiqiu Sep 1, 2020

yaoxuefeng6 Sep 3, 2020

zhiqiu Sep 1, 2020

yaoxuefeng6 Sep 3, 2020

zhiqiu Sep 1, 2020

zhiqiu Sep 1, 2020

zhiqiu Sep 1, 2020

yaoxuefeng6 Sep 3, 2020

zhiqiu Sep 1, 2020

yaoxuefeng6 Sep 3, 2020

zhiqiu Sep 1, 2020

yaoxuefeng6 Sep 3, 2020

jzhang533 left a comment

zhiqiu Sep 3, 2020

zhiqiu Sep 3, 2020

zhiqiu Sep 3, 2020

zhiqiu Sep 3, 2020

zhiqiu Sep 3, 2020

zhiqiu Sep 3, 2020

zhiqiu commented Sep 3, 2020 •

edited

XiaoguangHu01 left a comment

jzhang533 left a comment

add cuda generator #26786

add cuda generator #26786

Conversation

yaoxuefeng6 commented Aug 29, 2020

PR types

PR changes

Describe

paddle-bot-old bot commented Aug 29, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jzhang533 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zhiqiu commented Sep 3, 2020 • edited

XiaoguangHu01 left a comment

Choose a reason for hiding this comment

jzhang533 left a comment

Choose a reason for hiding this comment

zhiqiu commented Sep 3, 2020 •

edited