Random op #3060

dzhwinter · 2017-07-25T16:52:29Z

there is two part need to be discussed.
One is the dynamc_cast part of implementing polynomial. Another one is removing the const qualify key in Op.Run interface.

dzhwinter · 2017-07-25T16:58:39Z

paddle/framework/operator.h

@@ -88,7 +88,7 @@ class OperatorBase {

  /// Net will call this function to Run an op.
  virtual void Run(const std::shared_ptr<Scope>& scope,
-                   const platform::DeviceContext& dev_ctx) const = 0;
+                   platform::DeviceContext& dev_ctx) const = 0;


由于cuda的randGenerator和stream绑定，stream只存在于CUDADeviceContext里，只能在Op设置种子，生成Generator，违反了Op Run的时候不能修改context。因此去掉了const 约束。

尝试过解决办法，全局一个static generator。random_seed可以只在创建CUDADeviceContext一次指定，但是不能创建一个全局的Generator，否则随机性有问题。

如果seed固定的话，生成的序列也是一样的。
目前paddle/caffe2的随机数生成也都是提前设定好一个seed，后续不会改变。请教一下@hedaoyuan，不知道这样是否有问题。

GPU的每device环境的seed只需要设置一次就行了，但要保证各个device的seed是不一样的，否则每个device上获得的随机数就会是一样的。

const DeviceContext是否必要？ cublas_handle 等函数需要运行期间修改context

JiayiFeng · 2017-07-25T17:44:14Z

paddle/operators/random_op.h

+    auto place = context.GetPlace();
+    if (platform::is_cpu_place(place)) {
+      Gaussian(
+          dynamic_cast<platform::CPUDeviceContext*>(context.device_context_),


dynamic_cast效率可能比较低下，应该避免。
这里目前还需要dynamic_cast的原因是KernalContext里的成员变量：

const platform::DeviceContext& device_context_;

用父类DeviceContext的引用去保存子类CPUDeviceContext和CUDADeviceContext。这导致子类具体的类型信息丢失，无法用来特化模板和重载函数。在这里只能用dynamic_cast来手动显式转换。

所以，是否可以考虑在将上面的成员变量替换为：

using DeviceContext = boost::variant<CPUDeviceContext, CUDADeviceContext>; const DeviceContext device_context_;

Guassian函数对于CPU和GPU的接口统一为：

bool Gaussian(const DeviceContext& ctx, T* output, const int size, const T& mean, const T& std, const T& seed) { ... }

在函数内部进行GPU和CPU实现上的switch。

Place和DeviceContext的类型是可以一一对应的。可以在编译期根据Place的类型确定DeviceContext的类型。
在.h里面声明

template <typename Place, typename T> class RandomOpKernel : public framework::OpKernel { public: void Compute(const framework::KernelContext& context) const override; }

分别在.cc和.cu里面偏特化

template <typename T> class RandomOpKernel<CPUPlace, T> : public framework::OpKernel { void Compute() { CPUDeviceContext* dc = static_cast<CPUDeviceContext*>(KernelContext.device_context_); } }

.cu

template <typename T> class RandomOpKernel<GPUPlace, T> : public framework::OpKernel { void Compute() { CUDADeviceContext* dc = static_cast<CUDADeviceContext*>(KernelContext.device_context_); } }

Thanks for your comments! The template specialization can fix it elegantly. fix Done.

emailweixu · 2017-07-25T21:37:27Z

paddle/operators/random_op.cc

+  }
+};
+
+class RandomOpMaker : public framework::OpProtoAndCheckerMaker {


"random" is too general. There can be many different types of random. We should use a more specific name such as "GaussionRandom"

gangliao · 2017-07-31T16:32:55Z

paddle/operators/gaussian_random_op.cc

+        static_cast<const platform::CPUDeviceContext*>(context.device_context_);
+    // generator need to modify context
+    auto g = const_cast<platform::CPUDeviceContext*>(ctx)->RandGenerator();
+    std::normal_distribution<T> distribution(mean, std);


Only std normal distribution in here. Do you plan to implement other distribution soon? like uniform distribution.

std::uniform_real_distribution<T> uniform(0.0, 1.0);

curand also support curandGenerateUniform

@reyoung implement the uniform random generator in thrust.
see #3293 for detail.

JiayiFeng · 2017-07-31T17:58:08Z

paddle/platform/device_context.h

@@ -40,7 +41,10 @@ class DeviceContext {
 class CPUDeviceContext : public DeviceContext {
 public:
  typedef std::mt19937 random_generator_type;
-  CPUDeviceContext() { eigen_device_.reset(new Eigen::DefaultDevice()); }
+  CPUDeviceContext() {
+    random_seed_ = std::chrono::system_clock::now().time_since_epoch().count();


So now we are not able to manually specify random seed?

deleted the seeding function in context.
it is the place that we need to modify the Execution context. currently, move the set seed to random op. We need a global configuration to unify the random seed in the future. Maybe we can fix it in next PR.

JiayiFeng · 2017-08-08T18:19:11Z

paddle/operators/gaussian_random_op.cc

+ public:
+  void Compute(const framework::ExecutionContext& context) const override {
+    T mean = static_cast<T>(context.op_.GetAttr<T>("mean"));
+    T std = static_cast<T>(context.op_.GetAttr<T>("std"));


Aren't "mean" and "std" always float?
See https://github.com/PaddlePaddle/Paddle/pull/3060/files#diff-86aabb5beb3f333cf191423101025d1fR67

just unified style with the uniform random operator. Revert these things to old code.

JiayiFeng · 2017-08-08T18:36:57Z

paddle/operators/gaussian_random_op.cc

+    }
+    std::mt19937 g(seed);
+    std::normal_distribution<T> distribution(mean, std);
+    for (int i = 0; i < framework::product(tensor->dims()); ++i) {


Can we accelerate the code by calculating framework::product() before the loop? I'm not sure whether the compiler will do the optimization for us.

the compiler can't be optimized at all. tensor shape only can be referenced in runtime. Done.

JiayiFeng · 2017-08-08T18:38:13Z

paddle/operators/gaussian_random_op.cu

+ public:
+  void Compute(const framework::ExecutionContext& context) const override {
+    T mean = static_cast<T>(context.op_.GetAttr<T>("mean"));
+    T std = static_cast<T>(context.op_.GetAttr<T>("std"));


The same question in https://github.com/PaddlePaddle/Paddle/pull/3060/files#diff-86aabb5beb3f333cf191423101025d1fR26

JiayiFeng

Although the PR is not perfect, we have spent lots of time on it and it's becoming larger. So I prefer to merge it for now and refine the code in later PRs.

dzhwinter added 13 commits July 24, 2017 10:36

add random op

5ad9474

Merge remote-tracking branch 'origin/develop' into random_op

c110f56

"add template fill function"

0d554f1

"move to template function"

6f80b5f

Merge remote-tracking branch 'origin/develop' into random_op

d263cce

"random op test"

32c15a2

"link pybind11"

30a47fe

"template specialization link include"

2b3e362

"fix operator"

984225e

"fix const dependency hell"

11f9f5f

"remove const qualify"

9a16327

"cpu only macro"

69b1b26

"fix almost equal error"

a22567e

dzhwinter commented Jul 25, 2017

View reviewed changes

JiayiFeng reviewed Jul 25, 2017

View reviewed changes

emailweixu reviewed Jul 25, 2017

View reviewed changes

dzhwinter added 7 commits July 30, 2017 22:13

"update the compute kernel"

5721334

"fix const hell"

36d7e1f

"fix bind python error"

0253f2c

"update"

4d8ece8

"remove unused code"

4755668

"fix register error"

4973926

fix conflict

933e55e

gangliao reviewed Jul 31, 2017

View reviewed changes

JiayiFeng reviewed Jul 31, 2017

View reviewed changes

dzhwinter added 5 commits August 6, 2017 15:22

merge origin/develop

2447c34

device context pointer

0f8c9db

Merge remote-tracking branch 'origin/develop' into random_op

58561d8

"redefine random op"

fcd6f64

"keep style same with uniform operators"

e2c08d2

dzhwinter added 8 commits August 8, 2017 16:40

"test gaussian random in python side"

52d2ebd

Merge remote-tracking branch 'origin/develop' into random_op

8804b24

"format code"

555af4d

Merge remote-tracking branch 'origin/develop' into random_op

23ac845

Merge remote-tracking branch 'origin/develop' into random_op

6535a7b

"keep same with uniform random op"

d98e299

Merge remote-tracking branch 'origin/develop' into random_op

6fc6647

"remove context random seeding "

7082550

JiayiFeng reviewed Aug 8, 2017

View reviewed changes

JiayiFeng approved these changes Aug 8, 2017

View reviewed changes

dzhwinter added 4 commits August 9, 2017 14:47

"remove attribute"

df4fe67

"remove unused test net modified"

6bac3e1

"ci job failed weired. restart ci job."

bbd7378

"relauch ci"

f702e79

dzhwinter merged commit 56faf51 into PaddlePaddle:develop Aug 9, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Random op #3060

Random op #3060

dzhwinter commented Jul 25, 2017 •

edited

Loading

dzhwinter Jul 25, 2017

QiJune Jul 26, 2017

hedaoyuan Jul 26, 2017 •

edited

Loading

dzhwinter Jul 30, 2017

dzhwinter Jul 30, 2017

JiayiFeng Jul 25, 2017 •

edited

Loading

QiJune Jul 26, 2017

dzhwinter Jul 30, 2017

emailweixu Jul 25, 2017

dzhwinter Jul 30, 2017

gangliao Jul 31, 2017 •

edited

Loading

gangliao Jul 31, 2017

dzhwinter Aug 8, 2017

JiayiFeng Jul 31, 2017

dzhwinter Aug 8, 2017

JiayiFeng Aug 8, 2017 •

edited

Loading

dzhwinter Aug 9, 2017

JiayiFeng Aug 8, 2017

dzhwinter Aug 9, 2017

JiayiFeng Aug 8, 2017

dzhwinter Aug 9, 2017

JiayiFeng left a comment •

edited

Loading

Random op #3060

Random op #3060

Conversation

dzhwinter commented Jul 25, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hedaoyuan Jul 26, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JiayiFeng Jul 25, 2017 • edited Loading

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

gangliao Jul 31, 2017 • edited Loading

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

JiayiFeng Aug 8, 2017 • edited Loading

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

JiayiFeng left a comment • edited Loading

Choose a reason for hiding this comment

dzhwinter commented Jul 25, 2017 •

edited

Loading

hedaoyuan Jul 26, 2017 •

edited

Loading

JiayiFeng Jul 25, 2017 •

edited

Loading

gangliao Jul 31, 2017 •

edited

Loading

JiayiFeng Aug 8, 2017 •

edited

Loading

JiayiFeng left a comment •

edited

Loading