Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

【Hackathon 5th No.1】 为 Paddle 新增 copysign API #619

Merged
merged 4 commits into from
Sep 15, 2023
Merged

【Hackathon 5th No.1】 为 Paddle 新增 copysign API #619

merged 4 commits into from
Sep 15, 2023

Conversation

cocoshe
Copy link
Contributor

@cocoshe cocoshe commented Sep 13, 2023

No description provided.

@paddle-bot
Copy link

paddle-bot bot commented Sep 13, 2023

你的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.

@Ligoml Ligoml changed the title 【PaddlePaddle Hackathon 5th No. 1】为 Paddle 新增 copysign API 【Hackathon No.1】 为 Paddle 新增 copysign API Sep 14, 2023
@Ligoml Ligoml changed the title 【Hackathon No.1】 为 Paddle 新增 copysign API 【Hackathon 5th No.1】 为 Paddle 新增 copysign API Sep 14, 2023
其中

+ x(Tensor) - 需要取用绝对值作为输出数值部分的Tensor
+ y(Tensor, int, float 等 number)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里辛苦明确下x支持的数据类型有哪些,y是number时是否包含complex,y是tensor是数据类型支持哪些,以及shape上是否有条件

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

已修改,辛苦review~


## 底层OP设计

参考PyTorch与Numpy中的设计,调用底层cpp实现OP
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个地方辛苦明确写出下反向的规则吧

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

已修改,辛苦review~

- paddle.copysign(x, y) 作为独立的函数调用,非 inplace
- paddle.copysign_(x, y),作为独立的函数,inplace 地修改输入;
- Tensor.copysign(y)做为 Tensor 的方法使用,非 inplace;
- Tensor.copysign_(y)做为 Tensor 的方法使用, inplace 修改输入;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

辛苦参考其他API,添加下name参数

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

已修改,辛苦review~

@cocoshe
Copy link
Contributor Author

cocoshe commented Sep 15, 2023

@zoooo0820 btw,我粗略实现了一下,但是在过程中遇到了一些问题,想请教一下下面两类case的最佳实践,希望老师指点~

>>> x = paddle.to_tensor([1, 2, 3])
>>> x
Tensor(shape=[3], dtype=int64, place=Place(cpu), stop_gradient=True,
       [1, 2, 3])

>>> # 第一种情况,y为number的时候,如何让kernel兼容地接收 Tensor 和 Number 这种标量呢?如果kernel的形参为DenseTensor&,会有下面这种情况(y为标量)
>>> y = -1
>>> out = paddle.copysign(x, y)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/root/miniconda3/lib/python3.8/site-packages/paddle/tensor/math.py", line 7004, in copysign
    return _C_ops.copysign(x, y)
ValueError: (InvalidArgument) copysign(): argument 'y' (position 1) must be Tensor, but got int (at /root/Paddle/paddle/fluid/pybind/eager_utils.cc:1104)


>>> # 第二种情况,y为Tensor,不过shape与x不同,需要考虑广播,下面希望得到的out应该为[-1,-2,-3],需要对y进行广播
>>> y = paddle.to_tensor([-1])
>>> out = paddle.copysign(x, y)
>>> out
Tensor(shape=[3], dtype=int64, place=Place(cpu), stop_gradient=True,
       [-1,  2,  3])

@zoooo0820
Copy link
Contributor

@zoooo0820 btw,我粗略实现了一下,但是在过程中遇到了一些问题,想请教一下下面两类case的最佳实践,希望老师指点~

>>> x = paddle.to_tensor([1, 2, 3])
>>> x
Tensor(shape=[3], dtype=int64, place=Place(cpu), stop_gradient=True,
       [1, 2, 3])

>>> # 第一种情况,y为number的时候,如何让kernel兼容地接收 Tensor 和 Number 这种标量呢?如果kernel的形参为DenseTensor&,会有下面这种情况(y为标量)
>>> y = -1
>>> out = paddle.copysign(x, y)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/root/miniconda3/lib/python3.8/site-packages/paddle/tensor/math.py", line 7004, in copysign
    return _C_ops.copysign(x, y)
ValueError: (InvalidArgument) copysign(): argument 'y' (position 1) must be Tensor, but got int (at /root/Paddle/paddle/fluid/pybind/eager_utils.cc:1104)


>>> # 第二种情况,y为Tensor,不过shape与x不同,需要考虑广播,下面希望得到的out应该为[-1,-2,-3],需要对y进行广播
>>> y = paddle.to_tensor([-1])
>>> out = paddle.copysign(x, y)
>>> out
Tensor(shape=[3], dtype=int64, place=Place(cpu), stop_gradient=True,
       [-1,  2,  3])

第一个不用想得过于复杂,在OP层可以简单考虑y就是Tensor,如果API层传入的是个number,可以直接转为tensor再传入就好,这样也不会影响原本的梯度,当然也有其他的做法,可以参考topk的k参数

第二个请参考elementwise系列算子,例如add

其中

+ x(Tensor) - 需要取用绝对值作为输出数值部分的 Tensor , 支持 `bool`、`float16`、`float32`、`float64`、`uint8`、`int8`、`int16`、`int32`、`int64`、`bfloat16`
+ y(Tensor | Number) - 为 Tensor 时,shape 需要与 x 相同,或者可广播成 x.shape;为 Number 时,支持 `bool`、`float16`、`float32`、`float64`、`uint8`、`int8`、`int16`、`int32`、`int64`、`bfloat16`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这些数据类型是否应该是y为tensor时的情况,number时应该只有python的几种类型

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

是的,已修正,辛苦review~

@cocoshe
Copy link
Contributor Author

cocoshe commented Sep 15, 2023

@zoooo0820 btw,我粗略实现了一下,但是在过程中遇到了一些问题,想请教一下下面两类case的最佳实践,希望老师指点~

>>> x = paddle.to_tensor([1, 2, 3])
>>> x
Tensor(shape=[3], dtype=int64, place=Place(cpu), stop_gradient=True,
       [1, 2, 3])

>>> # 第一种情况,y为number的时候,如何让kernel兼容地接收 Tensor 和 Number 这种标量呢?如果kernel的形参为DenseTensor&,会有下面这种情况(y为标量)
>>> y = -1
>>> out = paddle.copysign(x, y)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/root/miniconda3/lib/python3.8/site-packages/paddle/tensor/math.py", line 7004, in copysign
    return _C_ops.copysign(x, y)
ValueError: (InvalidArgument) copysign(): argument 'y' (position 1) must be Tensor, but got int (at /root/Paddle/paddle/fluid/pybind/eager_utils.cc:1104)


>>> # 第二种情况,y为Tensor,不过shape与x不同,需要考虑广播,下面希望得到的out应该为[-1,-2,-3],需要对y进行广播
>>> y = paddle.to_tensor([-1])
>>> out = paddle.copysign(x, y)
>>> out
Tensor(shape=[3], dtype=int64, place=Place(cpu), stop_gradient=True,
       [-1,  2,  3])

第一个不用想得过于复杂,在OP层可以简单考虑y就是Tensor,如果API层传入的是个number,可以直接转为tensor再传入就好,这样也不会影响原本的梯度,当然也有其他的做法,可以参考topk的k参数

第二个请参考elementwise系列算子,例如add

明白了,tks~

Copy link
Contributor

@zoooo0820 zoooo0820 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants