【PTen】Add Scalar and ScalarArray in pten #37409

zyfncg · 2021-11-22T03:59:30Z

PR types

New features

PR changes

Others

Describe

Add Scalar and ScalarArray in pten

Scalar用来表示只有一个元素的0维Tensor，只支持CPU设备，可以通过Tensor或普通类型变量进行构造。

ScalarArray用来表示1维Tensor(现阶段只支持使用CPU，且只支持整数类型数据)，可以通过vector<int>，Tensor以及vector<Tensor>进行ScalarArray的构造。当前ScalarArray的主要用途是在PTen中代替原框架中表示shape或者索引类型的参数，即将vector<int>，Tensor和vector<Tensor>三种表示shape或者索引的数据类型统一为使用ScalarArray表示。

paddle-bot-old · 2021-11-22T03:59:34Z

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

chenwhql · 2021-11-23T02:11:09Z

paddle/pten/api/include/creation.h

@@ -28,6 +29,12 @@ PD_DLL_DECL Tensor full(const std::vector<int64_t>& shape,
                        Backend backend = Backend::CPU,
                        DataLayout layout = DataLayout::NCHW);

+PD_DLL_DECL Tensor full_new(const ScalarArray& shape,


while not replace original full？

Done, remove original full.

chenwhql · 2021-11-23T02:12:53Z

paddle/pten/api/lib/tensor.cc

@@ -214,17 +214,22 @@ const T *Tensor::data() const {
 template PD_DLL_DECL const float *Tensor::data<float>() const;
 template PD_DLL_DECL const double *Tensor::data<double>() const;
 template PD_DLL_DECL const int64_t *Tensor::data<int64_t>() const;
+template PD_DLL_DECL const uint64_t *Tensor::data<uint64_t>() const;


why need uint64 and uint32? If there is no current demand, over-design is not recommended

Done, removed.

chenwhql · 2021-11-23T02:21:35Z

paddle/pten/common/scalar.h


+ private:
+  DataType data_type_;


use dtype？

chenwhql · 2021-11-23T02:29:28Z

paddle/pten/common/scalar_array.h

+    }
+  }
+
+  template <typename TT>


TT is not a good name, maybe we can use InT, or OtherT?

chenwhql · 2021-11-23T02:30:02Z

paddle/pten/common/scalar_array.h

+    }
+  }
+
+  // template <typename TT>


if need in the future, add some comment

这里暂时先去掉了，如果在后续使用中有提供此类接口的需求再进行添加

chenwhql · 2021-11-23T02:30:50Z

paddle/pten/common/scalar_array.h

+  //     const ScalarArrayBase<TT>& scalar_array);
+
+ private:
+  std::vector<int64_t> array_;


maybe here need a TODO, we will not use vector finally

chenwhql · 2021-11-23T02:33:57Z

paddle/pten/kernels/cpu/creation.h

@@ -33,4 +34,10 @@ void FillConstant(const CPUContext& dev_ctx,
                  const Scalar& val,
                  DenseTensor* out);

+template <typename T>
+void FillConstantNew(const CPUContext& dev_ctx,


new is not a good suffix

暂时修改为FillConstantDynamicShape，后续PR将尝试去掉原来的FillConstant函数，把这里的函数名改为FillConstant

Shixiaowei02 · 2021-11-23T02:39:57Z

paddle/pten/common/scalar.h

 namespace paddle {
 namespace experimental {

-class Scalar {
+template <typename T>


为了与之前的设计统一，推理希望优先使用继承而非模板，如果限于排期可暂时放松设计，后续再进行修改或给出文档说明

当前设计仅作为解决参数问题的临时方案，后续可在解决问题的前提下对方案进行调整完善或者重新设计

Shixiaowei02 · 2021-11-23T02:40:30Z

paddle/pten/common/scalar.h

+    CopyScalar(other, this);
+  }
+
+  template <typename RT>


可能有类型安全问题

[TODO] 在后续方案的优化迭代中进行完善

YuanRisheng · 2021-11-23T03:07:20Z

paddle/pten/common/scalar_array.h

+          for (size_t i = 0; i < n; i++) {
+            PD_CHECK(tensor_list[i].dtype() == data_type,
+                     "The data_type of tensors in the list isn't consistent."
+                     "the first tensor is`",
+                     data_type,
+                     "` but `",
+                     i,
+                     "`th tensor is`",
+                     tensor_list[i].dtype(),
+                     "`.");
+            array_.push_back(*tensor_list[i].template data<int32_t>());
+          }


这部分或许可以抽出来，减少后续拓展类型产生重复代码

目前设计属于临时方案，暂不考虑类型扩展，如果后续有扩展的必要，再将这里的代码进行合并整理

YuanRisheng · 2021-11-23T03:10:57Z

paddle/pten/kernels/cpu/creation.cc

@@ -57,6 +57,15 @@ void FillConstant(const CPUContext& dev_ctx,
  eigen::fill<CPUContext, T>(dev_ctx, out, val.to<T>());
 }

+template <typename T>
+void FillConstantNew(const CPUContext& dev_ctx,


***New，奇怪的命名

暂时修改为FillConstantDynamicShape，后续PR将尝试去掉原来的FillConstant函数，把这里的函数名改为FillConstant

YuanRisheng · 2021-11-23T03:16:44Z

paddle/pten/kernels/cpu/creation.cc

@@ -87,3 +96,19 @@ PT_REGISTER_KERNEL("fill_constant.scalar",
                   paddle::platform::bfloat16,
                   paddle::platform::complex<float>,
                   paddle::platform::complex<double>) {}
+
+PT_REGISTER_KERNEL("fill_constant.new",


目前的注册kernel后缀最好能体现出一些输入特点，以便后边沉淀变成规则，规范化后续kernel迁移

已将.new后缀去掉

MingMingShangTian · 2021-11-23T03:31:03Z

paddle/pten/common/scalar_array.h

+  void AssignData(const TYPE* value_data, int64_t n) {
+    if (value_data) {
+      array_.reserve(n);
+      for (auto i = 0; i < n; i++) {


循环遍历一般是 ++i

MingMingShangTian · 2021-11-23T03:31:42Z

paddle/pten/common/scalar_array.h

+//     const ScalarArrayBase<T>& scalar_array) {
+//   return paddle::framework::make_ddim(scalar_array.array_);
+// }
+


MingMingShangTian · 2021-11-23T03:32:18Z

paddle/pten/common/scalar_array.h

+
+class DenseTensor;
+using ScalarArray = paddle::experimental::ScalarArrayBase<DenseTensor>;
+}


加上 // namespace pten

MingMingShangTian · 2021-11-23T03:33:13Z

paddle/pten/common/scalar_array.h

+
+class DenseTensor;
+using ScalarArray = paddle::experimental::ScalarArrayBase<DenseTensor>;
+}


加上 //namespace pten

MingMingShangTian · 2021-11-23T03:39:28Z

paddle/pten/common/scalar.h

    } else {
-      data_.d = std::stod(str_value);
+      data_.f64 = std::stod(str_value);


这里str_value 会是非法字符串吗？

这里的str_value是为了适配原来fill_constant op的参数，目前不会出现非法字符串的情况，完成op的迁移后这个接口应该会去掉，已补充注释

MingMingShangTian

LGTM

chenwhql

LGTM

* add scalar and scalar_array * remove DenseTensor include from Scalar and ScalarArray * remove inner header from scalar_array * refactor the method of fill_constant and add some comment

add scalar and scalar_array

a6218c4

zyfncg added 2 commits November 22, 2021 11:36

remove DenseTensor include from Scalar and ScalarArray

4df29cf

remove inner header from scalar_array

81fb5d4

chenwhql reviewed Nov 23, 2021

View reviewed changes

Shixiaowei02 reviewed Nov 23, 2021

View reviewed changes

YuanRisheng reviewed Nov 23, 2021

View reviewed changes

MingMingShangTian reviewed Nov 23, 2021

View reviewed changes

refactor the method of fill_constant and add some comment

e213512

MingMingShangTian previously approved these changes Nov 23, 2021

View reviewed changes

YuanRisheng previously approved these changes Nov 23, 2021

View reviewed changes

chenwhql previously approved these changes Nov 23, 2021

View reviewed changes

Merge branch 'develop' into scalar_and_array

72aa162

zyfncg dismissed stale reviews from chenwhql, YuanRisheng, and MingMingShangTian via 72aa162 November 24, 2021 03:30

zyfncg requested review from chenwhql and MingMingShangTian November 24, 2021 07:28

chenwhql approved these changes Nov 24, 2021

View reviewed changes

zyfncg merged commit 0f24de8 into PaddlePaddle:develop Nov 24, 2021

zyfncg mentioned this pull request Mar 26, 2022

[Phi] Rename ScalarArray to IntArray #40975

Merged

【PTen】Add Scalar and ScalarArray in pten #37409

【PTen】Add Scalar and ScalarArray in pten #37409

Conversation

zyfncg commented Nov 22, 2021

PR types

PR changes

Describe

paddle-bot-old bot commented Nov 22, 2021

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

Shixiaowei02 Nov 23, 2021 • 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

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

MingMingShangTian left a comment

Choose a reason for hiding this comment

chenwhql left a comment

Choose a reason for hiding this comment

Shixiaowei02 Nov 23, 2021 •

edited

Loading