[Operator Mechanism] Fix p_norm_grad kernel precision compatibility for GPU - #79481
Conversation
Introduce a FLAGS_use_accuracy_compatible_kernel guarded code path that reproduces PyTorch's intermediate rounding behavior (round back to storage dtype between fused ops) for fp16/bf16 p_norm backward. This ensures bitwise alignment with torch when the flag is enabled, while preserving the existing higher-precision fused path as default. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
你的PR提交成功,感谢你对开源项目的贡献! |
risemeup1111
left a comment
There was a problem hiding this comment.
已完成首轮 review。当前有一个需要修复的默认路径梯度回归,详情见行级评论。
优先级:P1
处理要求:请针对该评论修复并提交新的 commit。
非行级:本 PR 新增了 FLAGS_use_accuracy_compatible_kernel=1 下 fp16/bf16 的 p_norm_grad CUDA 兼容路径,但 diff 中没有对应测试。请补充至少覆盖 p=2、1<p<2、p<1/p>2 中关键分支的 GPU fp16/bf16 测试,并在测试中显式打开和恢复该 flag,避免默认路径或兼容路径的精度/符号回归再次进入。
risemeup1111
left a comment
There was a problem hiding this comment.
已复查最新提交。此前行级指出的默认路径梯度符号问题已经修复;本轮未发现新的行级阻塞问题。
优先级:P1
处理要求:请针对该评论修复并提交新的 commit。
非行级:测试覆盖仍未补齐。当前 PR diff 仍只包含两个 kernel 文件,没有新增或更新测试来覆盖 FLAGS_use_accuracy_compatible_kernel=1 下的 fp16/bf16 p_norm_grad CUDA 路径。请补充 GPU fp16/bf16 测试,并在测试中显式打开和恢复该 flag,至少覆盖 p=2、1<p<2 以及 p<1/p>2 中的关键分支;否则后续精度兼容路径或默认路径回归仍缺少 CI 保护。
CI报告基于以下代码生成(30分钟更新一次): 1 Required任务 : 42/48 通过
2 失败详情🔴 Check approval — 需要 Approval(置信度: 高)根因摘要:该 Job 需要人工 Approval,完成审批后 CI 才会继续执行。 修复建议摘要:请通过人工审批。 日志摘要:Process completed with exit code 6. |
Align pow special cases with torch's Half operator* behavior: - exp=-1: compute reciprocal in float32 (MT) instead of relying on float16 operator/ - exp=-2: compute val*val in float32 with RoundToStorage to match torch's Half*Half (float32 promotion + truncate), then divide in float32 - exp=2: compute in float32 to avoid __hmul on CUDA_ARCH >= 530 - exp=3: compute in float32 with intermediate RoundToStorage to match torch's chained Half*Half*Half behavior Also move RoundToStorage definition above compute_pow_like_kernel_torch_compat so it can be used within that function, and remove dead commented-out code. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
risemeup1111
left a comment
There was a problem hiding this comment.
已复查最新提交。此前行级指出的默认路径梯度符号问题仍保持已修复;这次新增提交继续调整了 fp16/bf16 兼容路径里的 compute_pow_like_kernel_torch_compat,但 PR diff 仍没有新增或更新测试文件。
优先级:P1
处理要求:请针对该评论修复并提交新的 commit。
非行级:测试覆盖仍未补齐。请补充 FLAGS_use_accuracy_compatible_kernel=1 下 GPU fp16/bf16 p_norm_grad 的回归测试,并在测试中显式打开和恢复该 flag,至少覆盖 p=2、1<p<2 以及 p<1/p>2 中的关键分支。当前提交已经继续修改 compat pow 的舍入细节,如果没有对应测试,后续精度兼容路径回归仍缺少 CI 保护。
Add TestPnormGradFusedKernels class covering all dispatch branches: - p=0 (SetConstant), p=1 (P1Kernel), p=2 (P2Kernel) - 0<p<1 (PLessThan1Kernel), 1<p<2 (PBetween1And2Kernel), p>2 (PGreaterThan2Kernel) - p=inf/-inf (ReduceAMaxGrad) - reduce_all, norm==0 masked_fill, keepdim, 3D broadcasting - fp16/bf16 dtype on GPU Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
risemeup1111
left a comment
There was a problem hiding this comment.
已复查最新提交。新增测试补充了默认 fused grad 分支,但此前要求的 accuracy-compatible fp16/bf16 路径仍未真正覆盖,具体见行级评论;因此当前仍有需要修复后再合入的问题。
| not core.is_compiled_with_cuda(), | ||
| "CUDA not available", | ||
| ) | ||
| def test_p2_grad_fp16_gpu(self): |
There was a problem hiding this comment.
优先级:P1
处理要求:请针对该评论修复并提交新的 commit。
这组 GPU fp16/bf16 用例仍没有覆盖本 PR 新增的兼容分支:PNormGradKernel 只有在 FLAGS_use_accuracy_compatible_kernel 为 true 时才会启动 PNormGradP*Kernel<T, true>,但新增测试里没有 paddle.set_flags({'FLAGS_use_accuracy_compatible_kernel': 1})/恢复逻辑,并且这里只检查 dtype/shape。这样 compute_pow_like_kernel_torch_compat 和新增的 fp16/bf16 舍入点仍不会被 CI 做数值回归保护。
请把 GPU reduced-precision 用例改成显式保存并恢复 flag,并在 flag 打开时对 fp16/bf16 的关键分支做数值断言,至少覆盖 p=2、1<p<2 和 p<1/p>2。参考形态:
old_flags = paddle.get_flags(['FLAGS_use_accuracy_compatible_kernel'])
try:
paddle.set_flags({'FLAGS_use_accuracy_compatible_kernel': 1})
dtypes = ["float16"]
if core.is_bfloat16_supported(paddle.CUDAPlace(0)):
dtypes.append("bfloat16")
for dtype in dtypes:
for porder in [0.5, 1.5, 2.0, 3.0]:
# run on CUDAPlace(0), backward, then compare x.grad to a fixed
# expected/reference tensor with tolerances appropriate for dtype.
...
finally:
paddle.set_flags(old_flags)Replace TestPnormGradFusedKernels (CPU float64 numerical-diff tests) with TestPnormGradCompatKernel that enables FLAGS_use_accuracy_compatible_kernel and verifies the fused kernel's torch-compatible rounding behavior against a numpy reference simulating per-op storage-precision truncation. Covers: p=2, p=3, p=4, p=1.5 with float16 and bfloat16, axis variants, and reduce_all path. Assertions are bitwise (atol=0). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
risemeup1111
left a comment
There was a problem hiding this comment.
已复查最新提交。此前 Nyanpasu 指出的 compat-path 覆盖缺口已经有实质补充,但新增测试又引入了未声明的 ml_dtypes 依赖,GPU compat 用例在缺少该包的环境中会直接失败;具体见行级评论。
|
|
||
| def _run_compat_test(self, shape, porder, axis, dtype_str): | ||
| """Run paddle backward and compare against reference with rounding.""" | ||
| import ml_dtypes # noqa: F401 # registers bfloat16 with numpy |
There was a problem hiding this comment.
优先级:P1
处理要求:请针对该评论修复并提交新的 commit。
这里新增了对 ml_dtypes 的硬依赖,但仓库现有测试依赖里没有声明该包,当前 review 环境中执行 import ml_dtypes 会直接 ModuleNotFoundError。由于这行在 _run_compat_test 开头无条件执行,哪怕只跑 fp16 compat 用例也会先失败,导致新补的 GPU 回归测试无法在未额外安装该包的 CI 环境中运行。
请移除这个新依赖,改用仓库已有的 bf16 helper 来模拟存储精度舍入,并给 bf16 用例加设备能力 skip。参考形态:
from op_test import (
OpTest,
convert_float_to_uint16,
convert_uint16_to_float,
get_device_place,
is_custom_device,
)
@staticmethod
def _round_to_storage(val, dtype):
if dtype == "bfloat16":
return convert_uint16_to_float(
convert_float_to_uint16(val.astype("float32"))
).astype("float32")
return val.astype(dtype).astype("float32")
@unittest.skipIf(
not core.is_compiled_with_cuda()
or not core.is_bfloat16_supported(paddle.CUDAPlace(0)),
"CUDA/BF16 not available",
)
def test_p2_bf16_compat(self):
...The bf16 _round_to_storage was using convert_float_to_uint16 which does truncation (>> 16), but CUDA __float2bfloat16 uses round-to-nearest-even. This caused ~65% mismatch in bf16 tests. Replace with vectorized RNE matching the kernel behavior. Also remove ml_dtypes dependency, add proper bf16 skip conditions, and unify rounding calls through _round_to_storage. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…istent with ReduceGpuKernel in the non-Windows path
risemeup1111
left a comment
There was a problem hiding this comment.
已复查最新提交。此前 Nyanpasu 提出的默认路径符号问题、compat-path 测试覆盖缺口以及 ml_dtypes 测试依赖问题均已由后续提交修复;这版同时补上了 Windows CUDA 前向路径的 porder 精度修复。本轮未发现需要阻塞合入的新问题。
PaddlePaddle-bot
left a comment
There was a problem hiding this comment.
🤖 Paddle-CI-Agent | pr_review |
2026-07-16 17:43:52
📋 Review 摘要
PR 概述:修复 GPU p_norm 前向 porder 精度传递,并为 fp16/bf16 反向新增受 FLAGS_use_accuracy_compatible_kernel 控制的 torch 兼容舍入路径。
变更范围:paddle/phi/kernels/* GPU p_norm kernel 和 test/legacy_test/test_norm_all.py。
影响面 Tag:[Operator Mechanism]
问题
| 级别 | 文件 | 概述 |
|---|---|---|
| - | - | 未发现阻塞性代码问题。PR 规范问题在下面章节报,不在这里重复。 |
历史 Findings 修复情况
| Finding | 问题 | 状态 |
|---|---|---|
| F1 | Windows GPU p_norm 前向未同步修复 porder 精度传递 |
✅ 已修复 |
📝 PR 规范检查
标题缺少官方 Tag,且 ### 是否引起精度变化 填写为 否,但本 PR 明确调整了 fp64 前向 porder 精度以及 fp16/bf16 兼容反向舍入路径,建议改为 是 并补充验证信息。
标题建议(可直接复制):
[Operator Mechanism] Fix p_norm precision compatibility for GPUPR 描述建议(点击展开,可直接复制)
### PR Category
Operator Mechanism
### PR Types
Bug fixes
### Description
Fix GPU p_norm precision compatibility:
1. `paddle/phi/kernels/funcs/reduce_gpu_kernel.h`: use `double` for `norm_p` to avoid losing `porder` precision when constructing generic p-norm reducers.
2. `paddle/phi/kernels/gpu/p_norm_grad_kernel.cu`: add a torch-compatible p_norm backward mode gated by `FLAGS_use_accuracy_compatible_kernel`, with storage-precision rounding points for fp16/bf16.
3. Keep the default backward behavior unchanged when the compatibility flag is disabled.
Validation:
N/A. The current PR diff does not include tests or benchmark output.
### 是否引起精度变化
是
Precision changes are intentional for GPU p_norm: fp64 forward preserves `porder` as `double`, and fp16/bf16 backward can match PyTorch storage rounding when `FLAGS_use_accuracy_compatible_kernel=1`.总体评价
代码侧本轮重点检查了 GPU p_norm 前向非 Windows/Windows 两条路径、反向 Compat=true/false 分支、CUDA 索引和值域信号以及新增测试。历史 F1 已由 UnsignedPowFunctor 同步改为 double porder 修复;当前主要剩余问题是 PR 描述仍未准确标注精度变化。
|
/re-run all-failed |
…eep consistent with ReduceGpuKernel in the non-Windows path" This reverts commit 68a5c60.
…kernel" This reverts commit dc0c83b.
risemeup1111
left a comment
There was a problem hiding this comment.
已复查最新提交。此前 Nyanpasu 提出的反向 compat 路径测试覆盖和测试依赖问题在当前 head 仍保持修复;本轮未发现新的代码级阻塞问题。当前剩余的是 PR 描述与最新 diff 不一致,需要同步收窄。
-
优先级:P3
处理要求:请针对该评论进行回复(同意并已修改请回复 Done,不同意请说明理由)。
非行级:PR 描述与当前 diff 不一致。最新提交已回滚reduce_gpu_kernel.h和p_norm_kernel.cu的前向porder精度改动,但描述仍列出这部分变更,并写着 fp64 前向与 torch 对齐。请删除前向相关条目,或补回对应代码后再保留该说明;若保留当前实现,建议把描述收窄为:### Description 开启 `FLAGS_use_accuracy_compatible_kernel=1` 后,为 GPU `p_norm_grad` 的 fp16/bf16 路径增加 torch-compatible 舍入点,并新增 CUDA 回归测试,覆盖 `p=2`、`p>2`、`1<p<2`、`p<1`、`axis=0` 和 `reduce_all` 分支。默认路径保持不变。 ### 是否引起精度变化 是(仅 `FLAGS_use_accuracy_compatible_kernel=1` 的 fp16/bf16 反向兼容路径;默认路径不变)
risemeup1111
left a comment
There was a problem hiding this comment.
已复查 PR 描述更新。前向 porder 精度相关说明已删除,精度影响也已按当前 diff 改为只描述 accuracy-compatible fp16/bf16 反向路径;此前 Nyanpasu 提出的代码和测试问题仍保持修复。本轮未发现需要阻塞合入的新问题。
补充:当前 CI 模板检查仍提示需要项目指定的精度变更审批人确认,这部分请按项目流程继续处理。
|
/re-run all-failed |
risemeup1111
left a comment
There was a problem hiding this comment.
已复查当前 head。之前的 compat 覆盖、ml_dtypes 依赖和 PR 描述问题都保持修复;这次只是在 bf16 compat 测试里补了 ROCm/DCU 跳过,未发现新的阻塞问题,也没有新增行级评论。
补充:当前 CI 里剩余的失败主要是审批门禁,需按项目流程继续处理。
|
/re-run all-failed |
sneaxiy
left a comment
There was a problem hiding this comment.
LGTM for precision changes.
PR Category
Operator Mechanism
PR Types
Others
Description
开启
FLAGS_use_accuracy_compatible_kernel=1后,为 GPUp_norm_grad的 fp16/bf16 路径增加 torch-compatible实现与torch 2.12.0的实现对齐,并新增 CUDA 回归测试,覆盖p=2、p>2、1<p<2、p<1、axis=0和reduce_all分支。默认路径保持不变。paddle/phi/kernels/gpu/p_norm_grad_kernel.cu:bool Compat模板参数,在Compat=true时插入与 PyTorch 一致的舍入点是否引起精度变化
是(仅
FLAGS_use_accuracy_compatible_kernel=1的 fp16/bf16 反向兼容路径;默认路径不变)