【PaddlePaddle Hackathon 4 No.17】为 Paddle 新增 cummax / cummin API #401

Patrick-Star125 · 2023-03-02T08:07:24Z

为 Paddle 新增 cummax / cummin API

【PaddlePaddle Hackathon 第四期】任务总览 Paddle#50629

cummax和cummin类似，因此大体内容和上期提案类似

paddle-bot · 2023-03-02T08:07:29Z

你的PR提交成功，感谢你对开源项目的贡献!
请检查PR提交格式和内容是否完备，具体请参考示例和模版。
Your PR has been submitted. Thanks for your contribution!
Please check its format and content. For this, you can refer to Template and Demo.

GGBond8488 · 2023-03-03T06:25:12Z

rfcs/APIs/20230302_api_design_for_cummin.md

+
+### 实现方法
+
+在实现方法上, PyTorch 通用实现采用的遍历，[CPU](https://github.com/pytorch/pytorch/blob/master/aten/src/ATen/native/ReduceOps.cpp#L638)，CUDA 采用的[GPU](https://github.com/pytorch/pytorch/blob/master/aten/src/ATen/native/cuda/ScanKernels.cpp#L17)。


这一句写的有点混乱，链接不太准确，cummax同

GGBond8488 · 2023-03-03T06:26:34Z

rfcs/APIs/20230302_api_design_for_cummin.md

+
+## 底层OP设计
+
+参考 paddle.cumsum 实现。


补充一下详细的设计吧，例：cummax/cummin是否由通用的函数实现？

补充一下具体的op的函数签名和设计

GGBond8488 · 2023-03-06T06:41:21Z

rfcs/APIs/20220316_api_design_for_cummax.md

@@ -50,11 +50,11 @@ Keyword Arguments
 ```
 即输入参数为 Tensor 和指定的维，两个值和索引的切片。


这里描述也有点混乱，修改一下

GGBond8488 · 2023-03-06T06:52:53Z

rfcs/APIs/20230302_api_design_for_cummin.md

+
+## PyTorch
+
+PyTorch 中有 API（https://pytorch.org/docs/stable/generated/torch.cummin.html）


这里链接多了)

GGBond8488 · 2023-03-06T06:53:28Z

rfcs/APIs/20230302_api_design_for_cummin.md

+Keyword Arguments
+ - out (tuple, optional) – the result tuple of two output tensors (values, indices)
+```
+即输入参数为 Tensor 和指定的维，两个值和索引的切片。


描述混乱，修改一下

GGBond8488 · 2023-03-06T06:54:17Z

rfcs/APIs/20230302_api_design_for_cummin.md

+API设计为`paddle.cummin(x, axis , dtype, name)`以及`paddle.Tensor.cummin(axis, dtype, name)`。参数设计参考`paddle.cumsum`。
+- x (Tensor) - 需要进行累积最大值统计的 Tensor。
+- axis (int, 可选) - 指明需要统计的维度。-1代表最后一维。默认：None，将输入展开为一维变量再进行累加计算。
+- dtype (str，可选) - 输出Tensor的数据类型，支持int32、int64、float32、float64. 如果指定了，那么在执行操作之前，输入张量将被转换为dtype. 这对于防止数据类型溢出非常有用。默认为：None。


这里dtype参数是否有必要？max/min操作并不会出现溢出现象

GGBond8488 · 2023-03-06T07:12:05Z

rfcs/APIs/20230302_api_design_for_cummin.md

+
+cpu：
+前向计算，可调用cumsum算子所实现的ScanKernel作为核心，增加一些细节处理即可
+后向计算，定义CummaxGradKernel函数实现cummax函数subgradient计算，或者参考其它不可导函数的subgradient计算方法


这里对应的名字写对吧，然后反向计算应该不能参考cummax的梯度计算方式，但是实现也不复杂，这里直接给出反向的计算方式吧

是的，需要单独实现，主要参考pytorch的实现

GGBond8488 · 2023-03-06T09:00:53Z

rfcs/APIs/20230302_api_design_for_cummin.md

+    const ArgumentMappingContext& ctx) {
+  return KernelSignature("cummin",
+                         {"X"},
+                         {"axis", "flatten", "exclusive", "reverse"},


这里 exclusive以及reverse参数的考虑是什么样的呢

已删除，exclusive和reverse参数我猜测是cumsum算子开发时添加的超出其功能的设计，在cummax/cummin中应该不需要

GGBond8488 · 2023-03-06T09:03:30Z

rfcs/APIs/20230302_api_design_for_cummin.md

+## 底层OP设计
+
+cpu：
+前向计算，可调用cumsum算子所实现的ScanKernel作为核心，增加一些细节处理即可


注意，使用前向的cumsum的Scankernel需要查看Eigen是否有对应的MaxReduce/MinReduce，如若没有则需要自己实现。GPU同样需要看对应cuda下是否提供对应的函数

重新构思了一下，应该不能以ScanKernel为核心来实现，主要还是参考pytorch的实现，计算出index方便反向梯度计算

GGBond8488

cummax同cummin

GGBond8488 · 2023-03-13T12:30:24Z

rfcs/APIs/20230302_api_design_for_cummin.md

+前向函数签名
+
+~~~cpp
+KernelSignature CumminOpArgumentMapping(


op 都写成kernel定义的形式，类似下面这种:
template <typename T, typename Context>
void CumsumKernel(const Context& dev_ctx,
const DenseTensor& x,
const Scalar& axis,
bool flatten,
bool exclusive,
bool reverse,
DenseTensor* out)

GGBond8488 · 2023-03-13T12:31:33Z

rfcs/APIs/20230302_api_design_for_cummin.md

+- x (Tensor) - 需要进行累积最大值统计的 Tensor。
+- axis (int, 可选) - 指明需要统计的维度。-1代表最后一维。默认：None，将输入展开为一维变量再进行累加计算。
+- name  (str，可选) - 操作的名称（可选，默认值为None）。
+


写明返回值；以写中文文档形式写这里的命名以及参数设计

且返回值中有标的api，需要提供dtype参数,限制返回的下标数据类型(int32, int64),默认为int64 具体参考 https://www.paddlepaddle.org.cn/documentation/docs/zh/dev_guides/api_contributing_guides/api_design_guidelines_standard_cn.html API命名与设计规范

GGBond8488 · 2023-03-13T12:32:35Z

rfcs/APIs/20230302_api_design_for_cummin.md

+
+# 六、测试和验收的考量
+
+测试考虑的case如下：


前向反向都需要测试

GGBond8488 · 2023-03-20T07:45:26Z

rfcs/APIs/20230302_api_design_for_cummin.md

+
+## 1、相关背景
+
+cummin 是指求累积最小值（cumulative min）的功能。即求


这句话好像不通顺

GGBond8488 · 2023-03-20T07:47:27Z

rfcs/APIs/20230302_api_design_for_cummin.md

+- x (Tensor) - 累积最小值的输入，需要进行累积最小值操作的 Tensor。
+- axis (int, 可选) - 指明需要统计的维度。-1代表最后一维。默认：None，将输入展开为一维变量再进行累加计算。
+- name  (str，可选) - 具体用法请参见 [Name](https://www.paddlepaddle.org.cn/documentation/docs/zh/api_guides/low_level/program.html#api-guide-name)，一般无需设置，默认值为 None。
+返回


返回索引的api，需要对索引的dtype进行限制，增加一个索引输出的dtype参数吧
具体可参照这里的数据类型规范 https://www.paddlepaddle.org.cn/documentation/docs/zh/dev_guides/api_contributing_guides/api_design_guidelines_standard_cn.html

GGBond8488 · 2023-03-20T07:48:12Z

rfcs/APIs/20230302_api_design_for_cummin.md

+参数
+:::::::::
+- x (Tensor) - 累积最小值的输入，需要进行累积最小值操作的 Tensor。
+- axis (int, 可选) - 指明需要统计的维度。-1代表最后一维。默认：None，将输入展开为一维变量再进行累加计算。


累加计算描述不准确

GGBond8488 · 2023-03-24T03:39:13Z

rfcs/APIs/20230302_api_design_for_cummin.md

+
+~~~cpp
+template <typename T, typename Context>
+void CumminKernel(const Context& dev_ctx,


kernel也需要添加dtype参数，注意这里的dtype类型应该为int

添加了dtype，类型约束可以由python端完成

attr_dtype = convert_np_dtype_to_dtype_(dtype) check_dtype(dtype, 'dtype', ['int32', 'int64'], 'cummax')

cummax/cummin和对应的rfc对应好

GGBond8488

修改一下吧，其他的没问题了

GGBond8488 · 2023-03-27T11:51:31Z

rfcs/APIs/20230302_api_design_for_cummin.md

@@ -21,7 +21,7 @@ PyTorch、NumPy 和 Pandas 提供了相似算子。

 ## 2、功能目标

-cummin API 是一个按轴寻找累计最大值和最大值所在位置的 API。此任务的目标是在 Paddle 框架中，新增 cummin API，调用路径为：`paddle.cummin`和 `paddle.tensor.cummin`。
+cummin API 是一个按轴寻找累计最大值和最大值所在位置的 API。此任务的目标是在 Paddle 框架中，新增 cummin API，调用路径为：`paddle.cummin`和 `paddle.Tensor.cummin`。


这里不对

不应该是寻找最大值

GGBond8488 · 2023-03-27T11:52:30Z

rfcs/APIs/20230302_api_design_for_cummin.md

+Keyword Arguments
+ - out (tuple, optional) – the result tuple of two output tensors (values, indices)
+```
+输入数据Tensor和cummax操作的维度dim，输出一个tuple包含计算结果values和索引indices


GGBond8488 · 2023-03-27T11:53:22Z

rfcs/APIs/20230302_api_design_for_cummin.md

+
+## 命名与参数设计
+
+API设计为`paddle.cummin(x, axis, name)`以及`paddle.Tensor.cummin(axis, name)`。


这里也要补上dtype

GGBond8488 · 2023-03-27T11:54:34Z

rfcs/APIs/20230302_api_design_for_cummin.md

+
+~~~cpp
+template <typename T, typename Context>
+void CumminKernel(const Context& dev_ctx,


cummax/cummin和对应的rfc对应好

Patrick-Star125 · 2023-03-28T05:11:13Z

OK，应该都对应了

GGBond8488

LGTM

new api for cummin

d0b5182

paddle-bot bot added contributor status: proposed labels Mar 2, 2023

Patrick-Star125 mentioned this pull request Mar 2, 2023

【PaddlePaddle Hackathon 第四期】任务总览 PaddlePaddle/Paddle#50629

Closed

luotao1 assigned luotao1, GGBond8488 and cloud2009 Mar 2, 2023

GGBond8488 reviewed Mar 3, 2023

View reviewed changes

update

0c610ff

GGBond8488 reviewed Mar 6, 2023

View reviewed changes

luotao1 assigned Ligoml Mar 6, 2023

Ligoml mentioned this pull request Mar 7, 2023

【PaddlePaddle Hackathon 第四期】任务总览 PaddlePaddle/Paddle#51281

Closed

partially adjust

63af315

GGBond8488 reviewed Mar 13, 2023

View reviewed changes

fix problems

4abcf39

GGBond8488 reviewed Mar 20, 2023

View reviewed changes

fix problems

c0d01ce

GGBond8488 reviewed Mar 24, 2023

View reviewed changes

fix some problems

6390852

GGBond8488 reviewed Mar 27, 2023

View reviewed changes

fix some problems

662425e

GGBond8488 approved these changes Mar 29, 2023

View reviewed changes

luotao1 approved these changes Mar 29, 2023

View reviewed changes

luotao1 merged commit bc5e224 into PaddlePaddle:master Mar 29, 2023

This was referenced May 6, 2023

【Hackathon 4 No.17】Add cummax / cummin API to Paddle PaddlePaddle/Paddle#53546

Merged

[rfc] modify cummax / cummin API design #531

Merged


		### 实现方法

		在实现方法上, PyTorch 通用实现采用的遍历，[CPU](https://github.com/pytorch/pytorch/blob/master/aten/src/ATen/native/ReduceOps.cpp#L638)，CUDA 采用的[GPU](https://github.com/pytorch/pytorch/blob/master/aten/src/ATen/native/cuda/ScanKernels.cpp#L17)。

		@@ -50,11 +50,11 @@ Keyword Arguments
		```
		即输入参数为 Tensor 和指定的维，两个值和索引的切片。


		## PyTorch

		PyTorch 中有 API（https://pytorch.org/docs/stable/generated/torch.cummin.html）


		# 六、测试和验收的考量

		测试考虑的case如下：


		## 1、相关背景

		cummin 是指求累积最小值（cumulative min）的功能。即求


		## 命名与参数设计

		API设计为`paddle.cummin(x, axis, name)`以及`paddle.Tensor.cummin(axis, name)`。

【PaddlePaddle Hackathon 4 No.17】为 Paddle 新增 cummax / cummin API #401

【PaddlePaddle Hackathon 4 No.17】为 Paddle 新增 cummax / cummin API #401

Conversation

Patrick-Star125 commented Mar 2, 2023

paddle-bot bot commented Mar 2, 2023

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

GGBond8488 Mar 6, 2023 • 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

GGBond8488 Mar 6, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GGBond8488 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

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

GGBond8488 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

Patrick-Star125 commented Mar 28, 2023

GGBond8488 left a comment

Choose a reason for hiding this comment

GGBond8488 Mar 6, 2023 •

edited

Loading

GGBond8488 Mar 6, 2023 •

edited

Loading