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 6th No.30】为 paddle.nn.functional.max_pool1d/max_pool2d /max_pool3d/paddle.signal.stft 进行功能增强 #62975

Merged
merged 10 commits into from
Jul 2, 2024

Conversation

AndPuQing
Copy link
Contributor

@AndPuQing AndPuQing commented Mar 24, 2024

PR Category

User Experience

PR Types

New features

Description

  1. 修复了在return_mask=True模式下,ceil_mode失效的问题,并添加了相应单测。

  2. 修复了 stft 在 as_real 后出现的数据不一致问题(develop分支无法复现)

image

另外,stft 是否需要根据输入数据 x 所在的设备来决定输出结果的设备,develop分支中,即使 x 在 cpu 上,输出tensor 依然在gpu上(因为window 创建时是根据default_device决定的)

Copy link

paddle-bot bot commented Mar 24, 2024

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

@paddle-bot paddle-bot bot added the contributor External developers label Mar 24, 2024
Copy link
Contributor

@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. max_pool1d/2d/3d重点需要检验下在ceil_mode为True/False下的shape是否一致?
  2. paddle.signal.sftftorch.sftf结果做了对比吗,详细对比两者下两者的结果
  3. 设备的问题:正常情况下,API的输入输出的设备是一致的,无需特殊指定配置,这里为何会出现不一致并且要手动指定?

@@ -362,6 +362,7 @@ def stft(
), f'expected a 1D window tensor of size equal to win_length({win_length}), but got window with shape {window.shape}.'
else:
window = paddle.ones(shape=(win_length,), dtype=x.dtype)
window = paddle.to_tensor(window, place=x.place)
Copy link
Contributor

Choose a reason for hiding this comment

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

这里windows的place为何是错的,一般情况下无需针对输出设备进行设置的

Copy link
Contributor Author

Choose a reason for hiding this comment

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

这里当用户默认设备是GPU,但是输入的X指定为CPU上,同时用户没有提供window输入,这里会创建一个在默认设备(GPU)上的tensor,最后造成输出时result在GPU,这是符合预期的吗?

@@ -362,6 +362,7 @@ def stft(
), f'expected a 1D window tensor of size equal to win_length({win_length}), but got window with shape {window.shape}.'
else:
window = paddle.ones(shape=(win_length,), dtype=x.dtype)
window = paddle.to_tensor(window, place=x.place)

Copy link
Contributor

Choose a reason for hiding this comment

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

正常情况下:API的输入与输出是在相同设备上的,无需指定,底层执行器会自动设定相应的place

test/legacy_test/test_pool1d_api.py Show resolved Hide resolved
test/legacy_test/test_pool1d_api.py Show resolved Hide resolved
test/legacy_test/test_pool2d_api.py Show resolved Hide resolved
test/legacy_test/test_pool3d_api.py Show resolved Hide resolved
@zhwesky2010
Copy link
Contributor

@AndPuQing 另外,该PR合入后,需要提交 API中文文档、API映射文档的修改。

同时需要将PaConvert中 _test 屏蔽的case打开,提交一个PR会自动触发测试。

Copy link

paddle-ci-bot bot commented Apr 4, 2024

Sorry to inform you that 02eb1dc's CIs have passed for more than 7 days. To prevent PR conflicts, you need to re-run all CIs manually.

@AndPuQing
Copy link
Contributor Author

Paddle version

commit: 02eb1dc336cf650ac3b5c5d2fdbf55f893f7fb02
cuda: 12.0
cudnn: 8.9.1
nccl: 21701
xpu: False
xpu_xccl: False
xpu_xhpc: False
cinn: False

Pytorch version

2.2.2+cu121
39901f229520a5256505ec24782f716ee7ddc843

input dtype = float32

input_shape n_fft hop_length normalized onesided Max absolute diff. Max relative diff.
(2,100) 50 15 False True 1.3126562e-06 9.709884e-06
(2,100) 100 15 False True 3.8146973e-06 9.33037e-06
(2,500) 50 15 False True 1.9073486e-06 9.538695e-06
(2,500) 100 15 False True 2.1324806e-06 8.827294e-05
(2,500) 200 15 False True 7.6293945e-06 8.2466126e-05
(2,500) 200 50 False True 3.8146973e-06 5.94125e-05
(2,500) 200 15 True True 4.7683716e-07 7.2988776e-05
(2,2000) 400 15 False True 1.5258789e-05 0.00011829

input dtype = float64

input_shape n_fft hop_length normalized onesided Max absolute diff. Max relative diff.
(2,100) 50 15 False True 3.55271368e-15 5.96877696e-15
(2,100) 100 15 False True 7.10542736e-15 4.95725428e-14
(2,500) 50 15 False True 3.55271368e-15 2.49609664e-14
(2,500) 100 15 False True 7.10542736e-15 4.42488659e-14
(2,500) 200 15 False True 1.42108547e-14 1.87328036e-13
(2,500) 200 50 False True 9.28696749e-15 1.8887612e-13
(2,500) 200 15 True True 1.33226763e-15 5.8703342e-14
(2,2000) 400 15 False True 1.77635684e-15 2.74193926e-13

Test Code

import paddle.cuda_env
import paddle.version
import torch
import paddle
import numpy as np
import torch.version

paddle.version.show()
print(torch.version.__version__)
print(torch.version.git_version)
n_fft = 400
hop_length = 15
normalized = False
onesided = True
inputs_np = np.random.rand(2, 2000).astype('float32')
window_np = np.hamming(n_fft).astype('float32')

# Paddle
x = paddle.to_tensor(inputs_np)
window = paddle.to_tensor(window_np)
res = paddle.signal.stft(
    x=x,
    n_fft=n_fft,
    hop_length=hop_length,
    window=window,
    normalized=normalized,
    onesided=onesided,
)

res_a = paddle.as_real(res)

# PyTorch
x = torch.tensor(inputs_np)
window = torch.tensor(window_np)
res0 = torch.stft(
    input=x,
    n_fft=n_fft,
    hop_length=hop_length,
    window=window,
    normalized=normalized,
    onesided=onesided,
    return_complex=True,
)

res_b = torch.view_as_real(res0)

# Compare results
np.testing.assert_allclose(
    res.numpy(), res0.numpy(), rtol=1e-20, atol=1e-20, verbose=True
)

@zhwesky2010
Copy link
Contributor

zhwesky2010 commented Apr 8, 2024

@AndPuQing

  1. 第1个问题修复没问题
  2. 第2个问题验证无误,无需修改了
  3. 第3个问题,输出out与输入x是应该要保持place一致的,这个原因是paddle.to_tensor 默认会使用全局place,因此导致中间结果window的place与输入x不一致;这个问题需要修一下,另外这个文件中的其他window也有这个问题,统一修改下吧

@luotao1
Copy link
Contributor

luotao1 commented Apr 8, 2024

第1个问题修复没问题

也可以单独提PR先把问题1给合入了 @AndPuQing

@AndPuQing
Copy link
Contributor Author

@AndPuQing 另外,该PR合入后,需要提交 API中文文档、API映射文档的修改。

同时需要将PaConvert中 _test 屏蔽的case打开,提交一个PR会自动触发测试。

  1. 不知道这个文档指的什么文档呢?因为ceil_mode原本就有这个api.
  2. 问题3 我另外单独提PR

Copy link

paddle-ci-bot bot commented Apr 14, 2024

Sorry to inform you that c1bec1c's CIs have passed for more than 7 days. To prevent PR conflicts, you need to re-run all CIs manually.

@zhwesky2010
Copy link
Contributor

zhwesky2010 commented Apr 17, 2024

@AndPuQing 另外,该PR合入后,需要提交 API中文文档、API映射文档的修改。
同时需要将PaConvert中 _test 屏蔽的case打开,提交一个PR会自动触发测试。

  1. 不知道这个文档指的什么文档呢?因为ceil_mode原本就有这个api.
  2. 问题3 我另外单独提PR

@AndPuQing

@luotao1 luotao1 added the API label Apr 24, 2024
zhwesky2010
zhwesky2010 previously approved these changes Jun 27, 2024
Copy link
Contributor

@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.

paddle/phi/ops/yaml/op_version.yaml 里需要修改下

action:
add_attr

Copy link
Contributor

@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

@luotao1 luotao1 merged commit bdb15e3 into PaddlePaddle:develop Jul 2, 2024
31 of 32 checks passed
@AndPuQing AndPuQing deleted the fix-maxpool branch July 12, 2024 09:06
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.

5 participants