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 4 No.23】为 paddle 新增 vander API #386

Merged
merged 5 commits into from
Mar 1, 2023

Conversation

Li-fAngyU
Copy link
Contributor

Add vander rfc file

@paddle-bot
Copy link

paddle-bot bot commented Feb 27, 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.

@Li-fAngyU Li-fAngyU changed the title 【Hackathon 4】23、为 paddle 新增vander AP{ 【Hackathon 4 No.23】为 paddle 新增 vander API Feb 27, 2023
return out
```
## 单测及文档填写
在` python/paddle/fluid/tests/unittests/`中添加`test_vander_op.py`文件进行单测,测试代码使用numpy计算结果后对比,与numpy对齐。
Copy link
Collaborator

Choose a reason for hiding this comment

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

如果有反向的话,测试部分需要完善,要包括C++ 算子单元测试

测试代码使用numpy计算结果后对比,与numpy对齐。

这个只是代表前向:在 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.

好的


# 二、飞桨现状

飞桨中还没有 vander 的实现,但可以利用已有的API组合进行实现。
Copy link
Collaborator

Choose a reason for hiding this comment

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

但可以利用已有的API组合进行实现。

从下文看,没有提到如何使用paddle的cumprod API来组合实现。如果需要用到cumprod相关函数,请详细描述下

Copy link
Contributor Author

@Li-fAngyU Li-fAngyU Feb 28, 2023

Choose a reason for hiding this comment

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

好的,我后面添加上。

这里有个额外的问题,vander 是可以直接通过python API组合实现的,还要为它写C++算子吗?

code example:

import paddle
import numpy as np

def vander(x, N=None, increasing=False):
    if x.dim() != 1:
        raise ValueError(
                "The input of x is expected to be a 1-D Tensor."
                "But now the dims of Input(X) is %d."
                % x.dim())
    
    if N < 0:
        raise ValueError("N must be non-negative.")

    if N is None:
        N = len(x)
    
    tmp = paddle.empty([len(x), N], dtype=x.dtype)

    if N > 0:
        tmp[:, 0] = 1
    if N > 1:
        tmp[:, 1:] = x[:, None]
        tmp[:, 1:] = paddle.cumprod(tmp[:, 1:], dim=-1)
    tmp = tmp[:, ::-1] if not increasing else tmp
    return tmp

def test():
    x = np.array([1., 2., 3.])
    a = paddle.to_tensor(x)
    N = [0,1,2,3,4,5]
    for n in N:
        np.isclose(vander(a, n).numpy(), np.vander(x, n))
        np.isclose(vander(a, n, increasing=True).numpy(), np.vander(x, n, increasing=True))
    print('test success!')

test()
# test success!

Copy link
Collaborator

Choose a reason for hiding this comment

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

vander 是可以直接通过python API组合实现的,还要为它写C++算子吗?

如果可以用python API组合实现,就不需要写C++算子,且单测部分不需要反向测试。后面的底层Op设计部分也要改一下。


在`paddle/phi/kernels/vander_kernel.h`中声明核函数的原型。

分别在 `paddle/phi/kernels/cpu/vander_kernel.cc` 和`paddle/phi/kernels/gpu/vander_kernel.cu`注册和实现核函数
Copy link
Collaborator

Choose a reason for hiding this comment

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

  • 是否要用到cumprod相关函数?
  • 这里的文件列全了么?没有看到反向相关的文件。(这个算子有反向么?)

Copy link
Contributor Author

@Li-fAngyU Li-fAngyU Feb 28, 2023

Choose a reason for hiding this comment

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

第一个问题:利用cumprod累乘API可以方便构建vander矩阵。

第二个问题:vander API,在torch上是不支持反向的,所以我就没有添加反向相关的kernel文件。后续会在rfc上补充说明。

example code:

import torch
a = torch.Tensor([1.,2.,3.])
a.requires_grad = True
b = torch.vander(a,3)
b.sum().backward()
# 报错信息:
# RuntimeError                              Traceback (most recent call last)
# /tmp/ipykernel_3602051/2253650649.py in 
#      3 a.requires_grad = True
#      4 b = torch.vander(a,3)
# ----> 5 b.sum().backward()
# RuntimeError: one of the variables needed for gradient computation has been modified by an inplace operation: [torch.FloatTensor [3, 2]], which is output 0 of SliceBackward, is at version 3; expected version 2 instead. Hint: enable anomaly detection to find the operation that failed to compute its gradient, with torch.autograd.set_detect_anomaly(True)。

@Li-fAngyU Li-fAngyU requested a review from luotao1 March 1, 2023 02:31
@Li-fAngyU
Copy link
Contributor Author

已修改!

@Li-fAngyU
Copy link
Contributor Author

我发现tensor.math 文件下的函数,对于输入x都仅支持Tensor,那么是否需要与其保持一致,还是扩展输入x使其支持list|tuple|np.ndarray|Tensor ?


# 二、飞桨现状

飞桨中还没有 vander 的实现,但可以利用已有的 paddle.cumprod API进行实现。
Copy link
Collaborator

Choose a reason for hiding this comment

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

这段代码在下面API实现方案里有,因此这里不用再贴一遍了。可以文字说明下参考下面的API实现方案

Copy link
Contributor Author

Choose a reason for hiding this comment

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

好的

通过上述分析可以发现,`numpy.vander`和`torch.linalg.vander`的核心实现都是依据累乘API来实现的,且`numpy.vander`和`torch.vander`的输入参数和返回值除类型分别为`numpy.nparray`和`torch.Tensor`之外基本一致。但是`torch.vander`仅能支持输入`x`为Tensor,不像`numpy.vander`能够额外支持`list和tuple`。

# 五、设计思路与实现方案
经测试,`paddle.vander`可以利用已有的API组合实现,因此不需要写C++算子。
Copy link
Collaborator

Choose a reason for hiding this comment

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

这里还需要明确说一下,没有反向(保持和torch一致)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

好的

@luotao1
Copy link
Collaborator

luotao1 commented Mar 1, 2023

我发现tensor.math 文件下的函数,对于输入x都仅支持Tensor,那么是否需要与其保持一致,还是扩展输入x使其支持list|tuple|np.ndarray|Tensor ?

保持一致即可

@Li-fAngyU
Copy link
Contributor Author

收到,已修改

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