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 7th No.36】为 Paddle 代码转换工具新增 API 转换规则(第 3 组) #479

Merged
merged 59 commits into from
Oct 16, 2024

Conversation

enkilee
Copy link
Contributor

@enkilee enkilee commented Sep 19, 2024

PR Docs

Docs:

PR APIs

torch.signal.windows.blackman
torch.signal.windows.cosine
torch.signal.windows.exponential
torch.signal.windows.gaussian
torch.signal.windows.general_cosine
torch.signal.windows.general_hamming
torch.signal.windows.hamming
torch.signal.windows.hann

Copy link

paddle-bot bot commented Sep 19, 2024

Thanks for your contribution!

@zhwesky2010
Copy link
Collaborator

zhwesky2010 commented Sep 19, 2024

单测未通过,请保证CI通过,CI不通过不予合入:

2024-09-19 15:54:54 FAILED tests/test_signal_windows_gaussian.py::test_case_1 - AttributeError: m...
2024-09-19 15:54:54 FAILED tests/test_signal_windows_gaussian.py::test_case_2 - AttributeError: m...
2024-09-19 15:54:54 FAILED tests/test_signal_windows_gaussian.py::test_case_3 - AttributeError: m...
2024-09-19 15:54:54 FAILED tests/test_signal_windows_gaussian.py::test_case_4 - KeyError: 'out'
2024-09-19 15:54:54 FAILED tests/test_signal_windows_gaussian.py::test_case_5 - AttributeError: m...
2024-09-19 15:54:54 FAILED tests/test_signal_windows_gaussian.py::test_case_6 - AttributeError: m...
2024-09-19 15:54:54 FAILED tests/test_signal_windows_gaussian.py::test_case_7 - AttributeError: m...
2024-09-19 15:54:54 FAILED tests/test_signal_windows_general_cosine.py::test_case_1 - AttributeEr...
2024-09-19 15:54:54 FAILED tests/test_signal_windows_general_cosine.py::test_case_2 - AttributeEr...
2024-09-19 15:54:54 FAILED tests/test_signal_windows_general_cosine.py::test_case_3 - AttributeEr...
2024-09-19 15:54:54 FAILED tests/test_signal_windows_general_cosine.py::test_case_4 - KeyError: '...
2024-09-19 15:54:54 FAILED tests/test_signal_windows_general_cosine.py::test_case_5 - AttributeEr...
2024-09-19 15:54:54 FAILED tests/test_signal_windows_general_cosine.py::test_case_6 - AttributeEr...
2024-09-19 15:54:54 FAILED tests/test_signal_windows_general_cosine.py::test_case_7 - AttributeEr...
2024-09-19 15:54:54 FAILED tests/test_signal_windows_general_hamming.py::test_case_1 - AttributeE...
2024-09-19 15:54:54 FAILED tests/test_signal_windows_general_hamming.py::test_case_2 - AttributeE...
2024-09-19 15:54:54 FAILED tests/test_signal_windows_general_hamming.py::test_case_3 - AttributeE...
2024-09-19 15:54:54 FAILED tests/test_signal_windows_general_hamming.py::test_case_4 - KeyError: ...
2024-09-19 15:54:54 FAILED tests/test_signal_windows_general_hamming.py::test_case_5 - AttributeE...
2024-09-19 15:54:54 FAILED tests/test_signal_windows_general_hamming.py::test_case_6 - AttributeE...
2024-09-19 15:54:54 FAILED tests/test_signal_windows_general_hamming.py::test_case_7 - AttributeE...
2024-09-19 15:54:54 FAILED tests/test_signal_windows_hamming.py::test_case_1 - AttributeError: mo...
2024-09-19 15:54:54 FAILED tests/test_signal_windows_hamming.py::test_case_2 - AttributeError: mo...
2024-09-19 15:54:54 FAILED tests/test_signal_windows_hamming.py::test_case_3 - AttributeError: mo...
2024-09-19 15:54:54 FAILED tests/test_signal_windows_hamming.py::test_case_4 - KeyError: 'out'
2024-09-19 15:54:54 FAILED tests/test_signal_windows_hamming.py::test_case_5 - AttributeError: mo...
2024-09-19 15:54:54 FAILED tests/test_signal_windows_hamming.py::test_case_6 - AttributeError: mo...
2024-09-19 15:54:54 FAILED tests/test_signal_windows_hann.py::test_case_1 - AttributeError: modul...
2024-09-19 15:54:54 FAILED tests/test_signal_windows_hann.py::test_case_2 - AttributeError: modul...
2024-09-19 15:54:54 FAILED tests/test_signal_windows_hann.py::test_case_3 - AttributeError: modul...
2024-09-19 15:54:54 FAILED tests/test_signal_windows_hann.py::test_case_4 - KeyError: 'out'
2024-09-19 15:54:54 FAILED tests/test_signal_windows_hann.py::test_case_5 - AttributeError: modul...
2024-09-19 15:54:54 FAILED tests/test_signal_windows_hann.py::test_case_6 - AttributeError: modul...
2024-09-19 15:54:54 =========== 52 failed, 8104 passed, 90 skipped in 188.19s (0:03:08) ============

@enkilee
Copy link
Contributor Author

enkilee commented Sep 19, 2024

能否修复下CI看错误,谢谢。

@paddle-bot paddle-bot bot added the contributor External developers label Sep 19, 2024
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.

修改一下comment

paconvert/api_matcher.py Outdated Show resolved Hide resolved
paconvert/api_matcher.py Outdated Show resolved Hide resolved
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.

  1. 所有单测都只测了sym=False,对bool参数正反都测下。包括requires_grad。其他参数的取值你也考虑多样性问题
  2. 所有测试的check_value都关了,这个是因为有随机性问题吗?如果没有随机性等特殊问题,这个正常情况是需要打开的

@enkilee
Copy link
Contributor Author

enkilee commented Oct 10, 2024

  1. 所有单测都只测了sym=False,对bool参数正反都测下。包括requires_grad。其他参数的取值你也考虑多样性问题
  2. 所有测试的check_value都关了,这个是因为有随机性问题吗?如果没有随机性等特殊问题,这个正常情况是需要打开的

1、收到,增加例子。
2、不是随机性,而是pytorch和paddle的结果不一致。

torch.signal.windows.blackman(5, dtype=torch.float32)
paddle.audio.functional.get_window("blackman",5, dtype='float32')
pytorch: tensor([-1.4901e-08,  3.4000e-01,  1.0000e+00,  3.4000e-01, -1.4901e-08])
paddle: Tensor(shape=[5], dtype=float32, place=Place(cpu), stop_gradient=True,
       [-0.00000001,  0.20077014,  0.84922987,  0.84922987,  0.20077014])

@zhwesky2010
Copy link
Collaborator

zhwesky2010 commented Oct 11, 2024

  1. 所有单测都只测了sym=False,对bool参数正反都测下。包括requires_grad。其他参数的取值你也考虑多样性问题
  2. 所有测试的check_value都关了,这个是因为有随机性问题吗?如果没有随机性等特殊问题,这个正常情况是需要打开的

1、收到,增加例子。 2、不是随机性,而是pytorch和paddle的结果不一致。

torch.signal.windows.blackman(5, dtype=torch.float32)
paddle.audio.functional.get_window("blackman",5, dtype='float32')
pytorch: tensor([-1.4901e-08,  3.4000e-01,  1.0000e+00,  3.4000e-01, -1.4901e-08])
paddle: Tensor(shape=[5], dtype=float32, place=Place(cpu), stop_gradient=True,
       [-0.00000001,  0.20077014,  0.84922987,  0.84922987,  0.20077014])

那这个不太合理,除非API的实现被论证有Bug,这个需要给一个明确的结论出来,是API有Bug还是转换有Bug。需要逐个排查并且打开check_value。

另外你上面的对应是有问题的,应该是这样:

torch.signal.windows.blackman(5)
paddle.audio.functional.get_window('blackman', 5, fftbins = False, dtype='float32')
tensor([-1.4901e-08,  3.4000e-01,  1.0000e+00,  3.4000e-01, -1.4901e-08])
Tensor(shape=[5], dtype=float32, place=Place(cpu), stop_gradient=True,
       [-0.00000001,  0.33999997,  0.99999994,  0.33999997, -0.00000001])

两者是能对应上的,同时说明 已合入的映射文档也可能是存在Bug的,需要一并修复

@enkilee
Copy link
Contributor Author

enkilee commented Oct 11, 2024

  1. 所有单测都只测了sym=False,对bool参数正反都测下。包括requires_grad。其他参数的取值你也考虑多样性问题
  2. 所有测试的check_value都关了,这个是因为有随机性问题吗?如果没有随机性等特殊问题,这个正常情况是需要打开的

1、收到,增加例子。 2、不是随机性,而是pytorch和paddle的结果不一致。

torch.signal.windows.blackman(5, dtype=torch.float32)
paddle.audio.functional.get_window("blackman",5, dtype='float32')
pytorch: tensor([-1.4901e-08,  3.4000e-01,  1.0000e+00,  3.4000e-01, -1.4901e-08])
paddle: Tensor(shape=[5], dtype=float32, place=Place(cpu), stop_gradient=True,
       [-0.00000001,  0.20077014,  0.84922987,  0.84922987,  0.20077014])

那这个不太合理,除非API的实现被论证有Bug,这个需要给一个明确的结论出来,是API有Bug还是转换有Bug。需要逐个排查并且打开check_value。

另外你上面的对应是有问题的,应该是这样:

torch.signal.windows.blackman(5)
paddle.audio.functional.get_window('blackman', 5, fftbins = False, dtype='float32')
tensor([-1.4901e-08,  3.4000e-01,  1.0000e+00,  3.4000e-01, -1.4901e-08])
Tensor(shape=[5], dtype=float32, place=Place(cpu), stop_gradient=True,
       [-0.00000001,  0.33999997,  0.99999994,  0.33999997, -0.00000001])

两者是能对应上的,同时说明 已合入的映射文档也可能是存在Bug的,需要一并修复

之前看代码,paddle的general_cosine和pytorch的运算就不同。
blackman就是调用的general_cosine

paddle:

    M, needs_trunc = _extend(M, sym)
    fac = paddle.linspace(-math.pi, math.pi, M, dtype=dtype)
    w = paddle.zeros((M,), dtype=dtype)
    for k in range(len(a)):
        w += a[k] * paddle.cos(k * fac)
    return _truncate(w, needs_trunc)

pytorch:

    constant = 2 * torch.pi / (M if not sym else M - 1)

    k = torch.linspace(start=0,
                       end=(M - 1) * constant,
                       steps=M,
                       dtype=dtype,
                       layout=layout,
                       device=device,
                       requires_grad=requires_grad)

    a_i = torch.tensor([(-1) ** i * w for i, w in enumerate(a)], device=device, dtype=dtype, requires_grad=requires_grad)
    i = torch.arange(a_i.shape[0], dtype=a_i.dtype, device=a_i.device, requires_grad=a_i.requires_grad)
    return (a_i.unsqueeze(-1) * torch.cos(i.unsqueeze(-1) * k)).sum(0)

两者调用linspace的参数不同,结果也不同。

@enkilee
Copy link
Contributor Author

enkilee commented Oct 11, 2024

blackman和general_cosine好像没法改,其他的我来改。

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.

所有的check_value为啥还全部都是关着的呢?全部精度测试过不了?还是全部都有Bug呢?

关闭check_value时写一个明确的理由reason

infoflow 2024-10-12 15-17-31

tests/test_signal_windows_blackman.py Outdated Show resolved Hide resolved
@enkilee
Copy link
Contributor Author

enkilee commented Oct 12, 2024

@zhwesky2010 改好了,不好意思,粗心大意,给您添麻烦了。2个PR都改了。DOC的pr也改了。
doc pr: PaddlePaddle/docs#6906

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.

有一些代码会引入问题,需要增强传入一个变量的测试,参考 [开发技巧] 的第2条:

infoflow 2024-10-12 20-19-10

https://github.com/PaddlePaddle/PaConvert/blob/master/docs/CONTRIBUTING.md#%E5%BC%80%E5%8F%91%E6%8A%80%E5%B7%A7

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
paconvert/api_matcher.py Outdated Show resolved Hide resolved
paconvert/api_matcher.py Outdated Show resolved Hide resolved
paconvert/api_matcher.py Show resolved Hide resolved
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.

重复的单测不用复制那么多,每个单测都有很多完全重复的case

tests/test_signal_windows_exponential.py Show resolved Hide resolved
tests/test_signal_windows_exponential.py Show resolved Hide resolved
tests/test_signal_windows_exponential.py Outdated Show resolved Hide resolved
paconvert/api_matcher.py Show resolved Hide resolved
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
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.

LGTM

@zhwesky2010 zhwesky2010 merged commit 808ab1c into PaddlePaddle:master Oct 16, 2024
7 checks passed
@luotao1 luotao1 changed the title 【Hackathon 7th No.36】为 Paddle 代码转换工具新增 API 转换规则(第 3 组)-part 【Hackathon 7th No.36】为 Paddle 代码转换工具新增 API 转换规则(第 3 组) Oct 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contributor External developers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants