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

转换规则 No. 10 #170

Open
wants to merge 47 commits into
base: master
Choose a base branch
from
Open

转换规则 No. 10 #170

wants to merge 47 commits into from

Conversation

enkilee
Copy link
Contributor

@enkilee enkilee commented Jul 14, 2023

@paddle-bot
Copy link

paddle-bot bot commented Jul 14, 2023

Thanks for your contribution!

@paddle-bot paddle-bot bot added contributor External developers status: proposed labels Jul 14, 2023
@zhwesky2010
Copy link
Collaborator

单测未通过,也需要修复

@enkilee
Copy link
Contributor Author

enkilee commented Jul 17, 2023

@zhwesky2010 AssertionError: API (torch.stft): shape mismatch, torch shape is (1, 9, 9), paddle shape is (1, 16, 9)
我在pytorch测试的结果是(1, 9, 9),在aistudio相同数据的结果也是(1, 9, 9)。不知为什么这里是 (1, 16, 9)

paconvert/api_matcher.py Outdated Show resolved Hide resolved
paconvert/api_matcher.py Outdated Show resolved Hide resolved
@enkilee
Copy link
Contributor Author

enkilee commented Jul 25, 2023

@zhwesky2010
请问,这种要如何追问题?

2023-07-20 12:42:10   File "/workspace/a4bba61b-0eea-44b9-8d1b-5a4732f9836f/PaConvert/paconvert/../paconvert/transformer/basic_transformer.py", line 550, in visit_FunctionDef
2023-07-20 12:42:10     super(BasicTransformer, self).generic_visit(node)
2023-07-20 12:42:10   File "/root/anaconda3/lib/python3.10/ast.py", line 494, in generic_visit
2023-07-20 12:42:10     value = self.visit(value)
2023-07-20 12:42:10   File "/workspace/a4bba61b-0eea-44b9-8d1b-5a4732f9836f/PaConvert/paconvert/../paconvert/base.py", line 82, in visit
2023-07-20 12:42:10     node = super(BaseTransformer, self).visit(node)
2023-07-20 12:42:10   File "/root/anaconda3/lib/python3.10/ast.py", line 418, in visit
2023-07-20 12:42:10     return visitor(node)
2023-07-20 12:42:10   File "/root/anaconda3/lib/python3.10/ast.py", line 503, in generic_visit
2023-07-20 12:42:10     new_node = self.visit(old_value)
2023-07-20 12:42:10   File "/workspace/a4bba61b-0eea-44b9-8d1b-5a4732f9836f/PaConvert/paconvert/../paconvert/base.py", line 82, in visit
2023-07-20 12:42:10     node = super(BaseTransformer, self).visit(node)
2023-07-20 12:42:10   File "/root/anaconda3/lib/python3.10/ast.py", line 418, in visit
2023-07-20 12:42:10     return visitor(node)
2023-07-20 12:42:10   File "/workspace/a4bba61b-0eea-44b9-8d1b-5a4732f9836f/PaConvert/paconvert/../paconvert/transformer/basic_transformer.py", line 314, in visit_Call
2023-07-20 12:42:10     node_list = matcher.get_paddle_nodes(node.args, node.keywords)
2023-07-20 12:42:10   File "/workspace/a4bba61b-0eea-44b9-8d1b-5a4732f9836f/PaConvert/paconvert/../paconvert/base.py", line 397, in get_paddle_nodes
2023-07-20 12:42:10     new_code = self.generate_code(new_kwargs)
2023-07-20 12:42:10   File "/workspace/a4bba61b-0eea-44b9-8d1b-5a4732f9836f/PaConvert/paconvert/../paconvert/api_matcher.py", line 3170, in generate_code
2023-07-20 12:42:10     kwargs["onesided"],
2023-07-20 12:42:10 KeyError: 'onesided'

Copy link
Collaborator

@zhwesky2010 zhwesky2010 left a comment

Choose a reason for hiding this comment

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

这个API能否用GenericMatcher实现,而且你的实现和文档的转写思路也不太一样,看看转写文档 https://github.com/PaddlePaddle/docs/pull/6007/files#diff-bdd744e44f42895f618539ba1fe51a96e211dd9558d0a2967a453fdb89eec160

你可以看下GenericMatcher的功能,其搭配的 kwargs_change、paddle_default_kwargs、unsupport_args 这些字段的功能

paconvert/api_matcher.py Outdated Show resolved Hide resolved
paconvert/api_matcher.py Outdated Show resolved Hide resolved
paconvert/api_matcher.py Outdated Show resolved Hide resolved
obj.run(pytorch_code, ["result"])


def test_case_4():
Copy link
Collaborator

Choose a reason for hiding this comment

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

单测case需要加一些,这个API参数很多,可以写七八个case

infoflow 2023-08-03 12-31-08

Copy link
Contributor Author

Choose a reason for hiding this comment

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

已加。CI已过,请有空审核

],
"kwargs_change": {
"input": "x",
"return_complex": "onesided"
Copy link
Collaborator

@zhwesky2010 zhwesky2010 Aug 18, 2023

Choose a reason for hiding this comment

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

infoflow 2023-08-18 16-23-54

这两个参数不是对应的,需要针对return_complex单独处理下

@zhwesky2010
Copy link
Collaborator

return_complex 时,可使用as_real、as_complex进行转换处理

@enkilee
Copy link
Contributor Author

enkilee commented Aug 28, 2023

收到,我尽快处理,抱歉

@enkilee
Copy link
Contributor Author

enkilee commented Aug 30, 2023

按照文档的说法,不知道我理解的对不对:
pytorch:
return_complex : Changed in version 2.0: return_complex is now a required argument for real inputs, as the default is being transitioned to True.
也就是说,现在在pytorch中, return_complex 默认为true, 输入数据为实信号,而且是必须为true?
Deprecated since version 2.0: return_complex=False is deprecated, instead use return_complex=True
Note that calling torch.view_as_real() on the output will recover the deprecated output format.
如果把return_complex设置为false,则需要调用torch.view_as_real()把结果换成real形式展示输出。

在paddle中:
onesided (bool,可选) - 当输入为实信号时,选择是否只返回傅里叶变换结果的一半的频点值(输入信号和窗函数均为实数时,傅里叶变换结果具有共轭对称性)。如果输入的信号或者窗函数的数据类型是复数,则此时不能设置为 True。默认为 True;

也就是说,onesided为false时,输入的是复数,可是pytorch2.0的输入必须为实信号,所以paddle的onesided值只能是True吧。

@zhwesky2010
Copy link
Collaborator

zhwesky2010 commented Sep 4, 2023

但你这个和映射文档中的参数映射对不上吧,麻烦检查下文档是不是有问题 https://github.com/PaddlePaddle/docs/blob/develop/docs/guides/model_convert/convert_from_pytorch/api_difference/ops/torch.stft.md

infoflow 2023-09-04 21-31-04

@enkilee
Copy link
Contributor Author

enkilee commented Sep 5, 2023

收到,我把文档改下。

@enkilee
Copy link
Contributor Author

enkilee commented Sep 5, 2023

No.10 的文档改了:
PaddlePaddle/docs#6164

],
"kwargs_change": {
"input": "x",
"return_complex": "onesided"
Copy link
Collaborator

@zhwesky2010 zhwesky2010 Sep 7, 2023

Choose a reason for hiding this comment

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

这里把 return_complex 这个参数赋值给 onesided 这个参数,我觉得是不对的,这不是两个完全不同的参数吗?

如果认为 return_complex 已经废弃,且恒为True无其他影响,那就应该设置 "return_complex": "",表示直接去掉该参数。

目前这个写法 "return_complex": "onesided" 是出于什么考虑对应两个不同的参数呢?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

收到,已改

[-1.6092, 0.5419, -0.2993, 0.3195]])
n_fft = 4
win_length = 4
result = torch.stft(x, n_fft=n_fft, center=False, win_length=win_length, normalized=True, return_complex=True)
Copy link
Collaborator

Choose a reason for hiding this comment

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

增加一下测试case:return_complex=False

[ 0.4907, -1.3948, -1.0691, -0.3132],
[-1.6092, 0.5419, -0.2993, 0.3195]])
n_fft = 4
result = torch.stft(x, n_fft=n_fft, center=False, return_complex=True)
Copy link
Collaborator

Choose a reason for hiding this comment

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

这四种情况需要都测到:

  1. 全部指定关键字
  2. 全部不指定关键字
  3. 改变关键字顺序
  4. 默认参数全部省略

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contributor External developers HappyOpenSource 快乐开源活动issue与PR status: proposed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants