【PaddlePaddle Hackathon 3 No.16】为 Paddle 新增 API paddle.take #44741

S-HuaBomb · 2022-07-29T13:06:34Z

PR types

New features

PR changes

APIs

Describe

完成飞桨黑客松第三期第16项目开发任务: #44073 (comment)

增加 API paddle.take，对于输入的 Tensor，将输入 Tensor 视为一维 Tensor，实现根据索引返回指定索引上的元素集合组成的新 Tensor。返回结果与索引的形状相同。

RFC 设计文档: https://github.com/PaddlePaddle/community/blob/master/rfcs/APIs/20220714_api_design_for_take.md
docs 中文文档：PaddlePaddle/docs#5099

… new-api

paddle-bot · 2022-07-29T13:06:39Z

你的PR提交成功，感谢你对开源项目的贡献!
请关注后续CI自动化测试结果，详情请参考Paddle-CI手册。
Your PR has been submitted. Thanks for your contribution!
Please wait for the result of CI firstly. See Paddle CI Manual for details.

luotao1 · 2022-08-04T03:57:55Z

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

+
+
+class TestTakeType(TestTakeAPI):
+    """Test take Error"""


缺少index索引越界的报错

我直接通过 paddle.index_select 来报错。
Done

luotao1 · 2022-08-04T03:59:10Z

python/paddle/tensor/math.py

+    The result takes the same shape as the indices.
+
+    Args:
+        input (Tensor): An N-D Tensor, which data type should be int32, int64, float32, float64. 


which data type-》its data type

luotao1 · 2022-08-04T04:03:10Z

python/paddle/tensor/math.py

+                "The data type of 'index' must be one of ['int32', 'int64'], but got {}".format(
+                    index.dtype))
+    else:
+        check_variable_and_dtype(index, 'index', ['int32', 'int64'], 'take')


index索引越界时需要报错

luotao1 · 2022-08-04T04:08:19Z

python/paddle/tensor/math.py

+            x.take(idx)
+            # Tensor(shape=[2, 3], dtype=int64, place=Place(cpu), stop_gradient=True,
+            #        [[4, 5, 6],
+            #         [7, 8, 9]])


示例可增加一个negative index和float类型的input

… new-api

luotao1

LGTM

Ligoml · 2022-08-05T08:06:17Z

python/paddle/tensor/math.py

+
+            n = np.arange(0, 12).reshape([3, 4])
+            x_int = paddle.to_tensor(n, dtype='int64')
+            x_float = paddle.to_tensor(n, dtype='float64')


可以使用paddle API直接生成输入的情况下，尽量避免引入第三方库哈~

docs 的 pr 也需要改一下

… new-api

Ligoml

LGTM for docs

luotao1 · 2022-08-09T02:52:46Z

@S-HuaBomb 和 @jeff41404 讨论后，需要根据 PaddlePaddle/community#186 (review) 重新修改下RFC和PR

… new-api

SigureMo · 2022-08-23T09:26:20Z

python/paddle/tensor/math.py

    """
-    Returns a new tensor with the elements of input at the given index.
-    The input tensor is treated as if it were viewed as a 1-D tensor. 
+    Returns a new tensor with the elements of tnput tensor x at the given index.


tnput？是个 typo 嘛？

是的，fixed，done

… new-api

luotao1

LGTM 请 @Ligoml review下文档部分

SigureMo · 2022-08-23T12:41:49Z

python/paddle/tensor/math.py

+        name (str, optional): Name for the operation (optional, default is None). For more information, please refer to :ref:`api_guide_Name`.
+
+    Returns:
+        Tensor: Tensor with the same shape as index, the data type is the same with input.


Tensor 后使用 ,，以避免解析出 Return Type

SigureMo · 2022-08-23T12:43:00Z

python/paddle/tensor/math.py

+        x (Tensor): An N-D Tensor, its data type should be int32, int64, float32, float64.
+        index (Tensor): An N-D Tensor, its data type should be int32, int64.
+        mode (str, optional): Specifies how out-of-bounds index will behave.
+                the candicates are ``'raise'`` | ``'wrap'`` | ``'clip'``.


这里用 , 分隔即可，下面的内容按照中文文档那边的意见统一改成列表吧～

… new-api

Ligoml · 2022-08-25T09:22:31Z

python/paddle/tensor/math.py

+            - ``'raise'``: raise an error (default);
+            - ``'wrap'``: wrap around;
+            - ``'clip'``: clip to the range. ``'clip'`` mode means that all indices that are too large are replaced by
+            the index that addresses the last element. Note that this disables indexing with negative numbers.


啊哦，这里报了一个warning，需要在这一行和参数详解之间增加一个空行来解决

Ligoml · 2022-08-25T09:24:30Z

另外解决一下冲突~

S-HuaBomb · 2022-08-25T12:46:14Z

另外解决一下冲突~

Done.

Ligoml

LGTM for docs

jeff41404 · 2022-08-26T04:06:04Z

python/paddle/tensor/math.py

@@ -4776,3 +4775,107 @@ def sgn(x, name=None):
        return paddle.as_complex(output)
    else:
        return paddle.sign(x)
+
+def take(x, index, mode='raise', name=None):


the name of parameter needs to be consistent with rfc, input in rfc while x here, and mode is not in rfc.

@jeff41404 根据之前的修改意见 PaddlePaddle/community#186 (review) 更新过RFC：PaddlePaddle/community#217
参数的名字按照新的RFC内容进行修改的。

@S-HuaBomb 请先修改完RFC的评审意见吧。

rfc is still old now, should update and merge rfc first

This is the modified RFC PaddlePaddle/community#217 with instructions added

… new-api

luotao1 · 2022-08-29T02:26:10Z

python/paddle/tensor/math.py

-        # Negative indexes can be enabled,
-        # but out-of-range indexes will report an error in the following paddle.index_select
-        index_1d = paddle.where(index_1d < 0, index_1d % max_index, index_1d)
+        index_1d = paddle.where(index_1d < 0, index_1d + max_index, index_1d)


@S-HuaBomb 这个修改是哪个case会出bug呢？

这里本来就应该是 + 号，这样确保负值索引是在合理范围内的，我只需要 + 号把它转成对应的负值索引。如果使用取余 %，那么超出范围的负值索引也会被约束到合理范围，那样是不对的。

那么超出范围的负值索引也会被约束到合理范围，那样是不对的。

Got it. 可以针对这个case补充一个报错的单测么？

已加，done.

… new-api

luotao1 · 2022-08-29T06:18:14Z

LGTM
@jeff41404 PaddlePaddle/community#217 修改后的RFC文档已经合入，请再次审核～

jeff41404

LGTM

S-HuaBomb added 4 commits July 15, 2022 10:41

add paddle.take api

982d01e

Merge branch 'develop' of https://github.com/PaddlePaddle/Paddle into…

69b0a3e

… new-api

fix paddle.take

b07c062

Merge branch 'develop' of https://github.com/PaddlePaddle/Paddle into…

09d2836

… new-api

paddle-bot bot added contributor External developers status: proposed labels Jul 29, 2022

remove from pip import main

c8482f6

This was referenced Jul 30, 2022

【PaddlePaddle Hackathon 第三期】任务总览 #43938

Closed

【PaddlePaddle Hackathon 3】16 新增 API paddle.take PaddlePaddle/docs#5099

Merged

luotao1 reviewed Aug 4, 2022

View reviewed changes

S-HuaBomb added 4 commits August 4, 2022 20:28

test index out of range error

0665e50

test index out of range error and fix conflict

c5a9e16

fix Examples

10b41c4

Merge branch 'develop' of https://github.com/PaddlePaddle/Paddle into…

6852760

… new-api

luotao1 self-requested a review August 5, 2022 02:27

luotao1 reviewed Aug 5, 2022

View reviewed changes

luotao1 requested a review from Ligoml August 5, 2022 03:46

Ligoml reviewed Aug 5, 2022

View reviewed changes

S-HuaBomb added 2 commits August 5, 2022 16:29

fix Examples

9649b87

Merge branch 'develop' of https://github.com/PaddlePaddle/Paddle into…

ec1cfd7

… new-api

Ligoml previously approved these changes Aug 5, 2022

View reviewed changes

luotao1 added the PaddlePaddle Hackathon label Aug 5, 2022

luotao1 self-assigned this Aug 5, 2022

paddle-bot bot removed the status: proposed label Aug 5, 2022

luotao1 assigned Ligoml Aug 5, 2022

S-HuaBomb mentioned this pull request Aug 22, 2022

Hackathon No.16】add RFC for take API PaddlePaddle/community#217

Merged

S-HuaBomb added 2 commits August 22, 2022 21:44

add param mode to take api

6806a8f

fix conflict ad merge

27b6943

Merge branch 'develop' of https://github.com/PaddlePaddle/Paddle into…

aaee858

… new-api

SigureMo reviewed Aug 23, 2022

View reviewed changes

S-HuaBomb added 2 commits August 23, 2022 17:54

fix typo

ca2604f

Merge branch 'develop' of https://github.com/PaddlePaddle/Paddle into…

5979d5f

… new-api

luotao1 reviewed Aug 23, 2022

View reviewed changes

SigureMo requested changes Aug 23, 2022

View reviewed changes

S-HuaBomb added 3 commits August 24, 2022 15:14

fix 嵌套列表

7b3fc1d

Merge branch 'develop' of https://github.com/PaddlePaddle/Paddle into…

668964d

… new-api

fix Tensor,

64b688a

Ligoml reviewed Aug 25, 2022

View reviewed changes

S-HuaBomb added 2 commits August 25, 2022 20:35

fix docs warning

cdd1080

fix conflict

eca0483

luotao1 previously approved these changes Aug 26, 2022

View reviewed changes

Ligoml previously approved these changes Aug 26, 2022

View reviewed changes

luotao1 requested a review from jeff41404 August 26, 2022 03:30

jeff41404 reviewed Aug 26, 2022

View reviewed changes

S-HuaBomb added 2 commits August 27, 2022 12:38

fix raise bug

4ca5c41

Merge branch 'develop' of https://github.com/PaddlePaddle/Paddle into…

9fb6896

… new-api

S-HuaBomb dismissed stale reviews from Ligoml and luotao1 via 9fb6896 August 27, 2022 05:02

luotao1 reviewed Aug 29, 2022

View reviewed changes

S-HuaBomb added 2 commits August 29, 2022 11:13

add test case for negative index out of range error

7fd6c85

Merge branch 'develop' of https://github.com/PaddlePaddle/Paddle into…

046ff44

… new-api

luotao1 approved these changes Aug 29, 2022

View reviewed changes

jeff41404 approved these changes Aug 30, 2022

View reviewed changes

luotao1 merged commit 5f1a8e4 into PaddlePaddle:develop Aug 30, 2022

SigureMo mentioned this pull request Sep 7, 2022

【社区治理】SigureMo 发起 Committer 身份申请 #45841

Closed

【PaddlePaddle Hackathon 3 No.16】为 Paddle 新增 API paddle.take #44741

【PaddlePaddle Hackathon 3 No.16】为 Paddle 新增 API paddle.take #44741

Conversation

S-HuaBomb commented Jul 29, 2022 • edited Loading

PR types

PR changes

Describe

paddle-bot bot commented Jul 29, 2022

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

luotao1 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

Ligoml left a comment

Choose a reason for hiding this comment

luotao1 commented Aug 9, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

luotao1 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

Ligoml commented Aug 25, 2022

S-HuaBomb commented Aug 25, 2022

Ligoml 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

luotao1 commented Aug 29, 2022

jeff41404 left a comment

Choose a reason for hiding this comment

S-HuaBomb commented Jul 29, 2022 •

edited

Loading