Skip to content

Use double precision for the norm_p value in norm reduce GPU kernel#79497

Open
omoYang wants to merge 2 commits into
PaddlePaddle:developfrom
omoYang:p-norm-forward
Open

Use double precision for the norm_p value in norm reduce GPU kernel#79497
omoYang wants to merge 2 commits into
PaddlePaddle:developfrom
omoYang:p-norm-forward

Conversation

@omoYang

@omoYang omoYang commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

PR Category

Operator Mechanism

PR Types

Bug fixes

Description

paddle/phi/kernels/funcs/reduce_gpu_kernel.h

  • norm_p 参数类型从 float 改为 double,避免fp64下p值在传递过程中丢失精度
  • fp64的前向与torch对齐 不会带来额外性能开销

paddle/phi/kernels/gpu/p_norm_kernel.cu

  • porder参数由float改为double,避免fp64在_WIN32路径下计算时精度丢失porder精度

是否引起精度变化

Use double precision for porder in UnsignedPowFunctor to to keep consistent with ReduceGpuKernel in the non-Windows path
@paddle-bot

paddle-bot Bot commented Jul 17, 2026

Copy link
Copy Markdown

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

@risemeup1111 risemeup1111 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

本轮未发现需要阻塞合入的代码实现问题;下面有一个非行级的测试覆盖确认项。

P2 优先级:P2
非行级:本 PR 只修改了两个 GPU kernel 文件,没有新增或调整测试;现有 p_norm float64 基础用例默认使用 porder=2.0,这个值在 float 中可精确表示,无法覆盖本次修复的“double porder 在 GPU forward 传递过程中被截断成 float”场景。建议补充一个 GPU/float64、非整数或高精度 porder(例如 np.pi 或无法被 float 精确保留的 double 值)的回归用例,并用严格的 double 精度容忍度对齐 NumPy/Torch;如果已有 CI 用例覆盖该场景,也请在这里说明具体用例。

参考用例形态:

x_np = (np.random.random([2, 3, 4]) + 0.5).astype("float64")
porder = np.pi
expected = np.linalg.norm(x_np, ord=porder, axis=1)
out = paddle.linalg.vector_norm(paddle.to_tensor(x_np, place=paddle.CUDAPlace(0)), p=porder, axis=1)
np.testing.assert_allclose(out.numpy(), expected, rtol=1e-12, atol=1e-12)

处理要求:请针对该评论进行回复(同意并已修改请回复 Done,不同意请说明理由)。

Powered by Nyanpasu with gpt-5.5 xhigh, please check the suggestions carefully.

@omoYang

omoYang commented Jul 20, 2026

Copy link
Copy Markdown
Contributor Author

本轮未发现需要阻塞合入的代码实现问题;下面有一个非行级的测试覆盖确认项。

P2 优先级:P2 非行级:本 PR 只修改了两个 GPU kernel 文件,没有新增或调整测试;现有 p_norm float64 基础用例默认使用 porder=2.0,这个值在 float 中可精确表示,无法覆盖本次修复的“double porder 在 GPU forward 传递过程中被截断成 float”场景。建议补充一个 GPU/float64、非整数或高精度 porder(例如 np.pi 或无法被 float 精确保留的 double 值)的回归用例,并用严格的 double 精度容忍度对齐 NumPy/Torch;如果已有 CI 用例覆盖该场景,也请在这里说明具体用例。

参考用例形态:

x_np = (np.random.random([2, 3, 4]) + 0.5).astype("float64")
porder = np.pi
expected = np.linalg.norm(x_np, ord=porder, axis=1)
out = paddle.linalg.vector_norm(paddle.to_tensor(x_np, place=paddle.CUDAPlace(0)), p=porder, axis=1)
np.testing.assert_allclose(out.numpy(), expected, rtol=1e-12, atol=1e-12)

处理要求:请针对该评论进行回复(同意并已修改请回复 Done,不同意请说明理由)。

Powered by Nyanpasu with gpt-5.5 xhigh, please check the suggestions carefully.

Done

@omoYang omoYang closed this Jul 20, 2026
@omoYang omoYang reopened this Jul 20, 2026
@omoYang
omoYang requested a review from risemeup1111 July 20, 2026 03:31

@risemeup1111 risemeup1111 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

已复查新提交:作者已回复 Done 并补充了 float64 非整数 porder 的回归测试。测试方向是对的,但当前阈值仍不足以稳定捕获旧的 float 截断行为,具体建议已放在行级评论里。

除该测试覆盖敏感度问题外,暂未发现新的阻塞实现问题。

Powered by Nyanpasu with gpt-5.5 xhigh, please check the suggestions carefully.

Comment thread test/legacy_test/test_norm_all.py Outdated
Comment on lines +440 to +441
def test_check_grad(self):
self.check_grad(['X'], 'Out', user_defined_grads=self.gradient)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 优先级:P2

这里新增了 porder=2.3 的 float64 用例,但仍继承父类 test_check_output() 的默认 atol/rtol=1e-5double(2.3) 截断到 float32 后只差约 4.77e-08;按这个用例的长度 3 reduce 维度估算,旧实现的前向输出差在 1e-8 量级,低于默认阈值,因此旧的 porder float 截断问题仍可能通过该测试。请把这个回归用例的前向比较阈值收紧到 double 精度,确保旧实现会失败、新实现会通过。

Suggested change
def test_check_grad(self):
self.check_grad(['X'], 'Out', user_defined_grads=self.gradient)
def test_check_output(self):
self.check_output(atol=1e-12, rtol=1e-12, check_prim_pir=True)
def test_check_grad(self):
self.check_grad(['X'], 'Out', user_defined_grads=self.gradient)

处理要求:请针对该评论进行回复(同意并已修改请回复 Done,不同意请说明理由)。

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

已确认,当前 TestPnormOp7 的前向阈值已经按建议收紧,前面的测试覆盖问题可以按当前实现处理。

@risemeup1111 risemeup1111 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

已复查这次提交。先前关于测试覆盖阈值的意见已经在代码上落实,新的 TestPnormOp7 现在把 test_check_output 收紧到 double 精度,细节仍在之前的 inline 讨论里。

本轮没有新增行级问题;当前还需要作者在对应线程回复确认后,我再关闭那条意见。

Powered by Nyanpasu with gpt-5.5 xhigh, please check the suggestions carefully.

@risemeup1111 risemeup1111 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

已确认作者回复并完成对应修正,前面的测试覆盖意见已经落实。当前未发现需要阻塞合入的问题,细节在之前的 inline review comments 中。

Powered by Nyanpasu with gpt-5.5 xhigh, please check the suggestions carefully.

@wanghuancoder wanghuancoder left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@omoYang

omoYang commented Jul 20, 2026

Copy link
Copy Markdown
Contributor Author

/re-run all-failed

2 similar comments
@omoYang

omoYang commented Jul 20, 2026

Copy link
Copy Markdown
Contributor Author

/re-run all-failed

@omoYang

omoYang commented Jul 21, 2026

Copy link
Copy Markdown
Contributor Author

/re-run all-failed

@Xreki Xreki left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM for atol

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants