Skip to content

align complex mul with torch#79469

Merged
sneaxiy merged 1 commit into
PaddlePaddle:developfrom
greenhandF:mul
Jul 20, 2026
Merged

align complex mul with torch#79469
sneaxiy merged 1 commit into
PaddlePaddle:developfrom
greenhandF:mul

Conversation

@greenhandF

Copy link
Copy Markdown
Contributor

PR Category

Operator Mechanism

PR Types

Improvements

Description

align complex mul with torch by changing imaginary part calculation sequence

是否引起精度变化

@PaddlePaddle-bot PaddlePaddle-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🤖 Paddle-CI-Agent | pr_review | 2026-07-14 11:58:57

📋 Review 摘要

PR 概述:调整 CUDA/HIP 设备端复数乘法 imaginary part 的 FMA 顺序,以对齐 PyTorch GPU 舍入。
变更范围paddle/phi/common/complex.hphi::dtype::complex<T>::operator*
影响面 Tag[Operator Mechanism]

问题

级别 文件 概述
🟡 建议 paddle/phi/common/complex.h:235 核心复数乘法路径声称 bit-exact 对齐 PyTorch,但缺少能覆盖 FMA 舍入顺序的回归测试

📝 PR 规范检查

标题缺少官方 Tag;同时 PR 已填写“是否引起精度变化:是”,但 Description 未说明验证方式和完整影响范围。

标题建议(可直接复制):

[Operator Mechanism] Align complex mul with PyTorch GPU rounding
PR 描述建议(点击展开,可直接复制)
### PR Category
<!-- One of [ User Experience | Execute Infrastructure | Operator Mechanism | CINN | Custom Device | Performance Optimization | Distributed Strategy | Parameter Server | Communication Library | Auto Parallel | Inference | Environment Adaptation ] -->
Operator Mechanism

### PR Types
<!-- One of [ New features | Bug fixes | Improvements | Performance | BC Breaking | Deprecations | Docs | Devs | Not User Facing | Security | Others ] -->
Improvements

### Description
<!-- Describe what you’ve done -->
修改 CUDA/HIP 设备端 `phi::dtype::complex<T>::operator*` 的 imaginary part 计算顺序,使 GPU 复数乘法的舍入行为与 PyTorch 对齐。

精度变化来源:imaginary part 改为融合 `a.imag * b.real`,并将 `a.real * b.imag` 单独舍入后累加;real part 的计算顺序不变。该变化可能改变 `complex64`/`complex128` GPU 复数乘法结果的最后一位舍入。

影响范围:CUDA/HIP 设备端复数乘法,包括 `paddle.multiply`/`elementwise_mul` 和复数梯度中复用 `MultiplyFunctor<T>` 的路径;CPU 路径不变。

验证方式:当前 diff 未包含 GPU bit-exact 回归测试或验证记录;需要补充 `complex64`/`complex128` 输入,其 raw bits 能区分旧/新 FMA 顺序,并对比 Paddle GPU 输出与 PyTorch GPU 输出一致。

### 是否引起精度变化
<!-- one of the following [ 是 | 否 ]-->

总体评价

实现本身只调整复数乘法 imaginary part 的 FMA 归约顺序,公式等价且作用范围清晰;合入前建议补上能证明并长期守住 PyTorch GPU bit-exact 行为的回归测试,同时完善 PR 标题和精度变化说明。

// real = a.real*b.real - a.imag*b.imag = fma(a.real, b.real,
// -(a.imag*b.imag)) imag = a.imag*b.real + b.imag*a.real = fma(b.imag,
// a.real, a.imag*b.real)
// Match PyTorch's GPU rounding (verified bit-exact):

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 建议 这里把目标定义为 PyTorch GPU bit-exact,但当前 PR 只改了实现,没有新增能区分旧/新 FMA 顺序的回归测试。

complex<T>::operator* 会通过 MultiplyFunctor<T> 复用到 multiply/multiply_raw 以及复数梯度路径;现有 test_elementwise_mul_op.py 主要用普通复数样例和 NumPy 容差比较,无法守住这一位级舍入约束。建议补一组 CUDA/HIP 复数乘法回归用例,固定能触发旧/新 imaginary FMA 顺序差异的 complex64/complex128 输入,并用 raw bits(或 view 成整数后比较)验证与 PyTorch GPU 输出一致。

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

已完成首轮代码审查。这里有一个需要先补齐的阻塞项,详细说明已放在行级评论里:这次改动声明要对齐 GPU 复数乘法的 bit-exact 舍入顺序,需要相应的回归测试把该行为固定下来。

另外,目前精度变更和 PHI 目录相关的审批门禁仍未满足,合入前也需要按项目流程处理。

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

Comment on lines +235 to +242
// Match PyTorch's GPU rounding (verified bit-exact):
// real: fuse a.real*b.real, round a.imag*b.imag
// imag: fuse a.imag*b.real, round a.real*b.imag
return complex<T>(__fma_rn(a.real, b.real, -__dmul_rn(a.imag, b.imag)),
__fma_rn(b.imag, a.real, __dmul_rn(a.imag, b.real)));
__fma_rn(a.imag, b.real, __dmul_rn(a.real, b.imag)));
} else {
return complex<T>(__fmaf_rn(a.real, b.real, -__fmul_rn(a.imag, b.imag)),
__fmaf_rn(b.imag, a.real, __fmul_rn(a.imag, b.real)));
__fmaf_rn(a.imag, b.real, __fmul_rn(a.real, b.imag)));

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.

P1 优先级:P1
处理要求:请针对该评论修复并提交新的 commit。

这次改动的目标是固定 GPU 复数乘法的 bit-exact PyTorch 舍入顺序,但 PR 只改了 complex.h,没有新增能区分新旧 FMA 顺序的回归用例。现有 test/legacy_test/test_elementwise_mul_op.py 主要按 NumPy 结果和容差检查复数乘法,旧的 imag 计算顺序也很容易通过,无法防止后续把这里的兼容性目标回退掉。

请新增 CUDA 端的固定输入回归测试,覆盖 complex64complex128,并直接比较输出 bit pattern,或与 PyTorch 预生成的 fixture 做 array_equal。测试需要包含至少一组新旧 imag FMA 顺序产生不同最低位的输入;如果这条 HIP/DCU 路径也要声明 bit-exact,请同步补覆盖或在测试说明中明确该平台的验证方式。实现形态可以类似:

# pseudocode
x = np.array([...], dtype=np.complex64)
y = np.array([...], dtype=np.complex64)
expected = np.array([...], dtype=np.complex64)  # generated from the target PyTorch GPU result
out = paddle.multiply(
    paddle.to_tensor(x, place=paddle.CUDAPlace(0)),
    paddle.to_tensor(y, place=paddle.CUDAPlace(0)),
).numpy()
np.testing.assert_array_equal(out.view(np.uint32), expected.view(np.uint32))

@greenhandF

Copy link
Copy Markdown
Contributor Author

/re-run all-failed

@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

@paddle-bot

paddle-bot Bot commented Jul 14, 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.

@paddle-bot paddle-bot Bot added the contributor External developers label Jul 14, 2026
@greenhandF

Copy link
Copy Markdown
Contributor Author

/re-run all-failed

@PaddlePaddle PaddlePaddle locked and limited conversation to collaborators Jul 18, 2026
@PaddlePaddle PaddlePaddle unlocked this conversation Jul 18, 2026
@sneaxiy sneaxiy closed this Jul 18, 2026
@sneaxiy sneaxiy reopened this Jul 18, 2026
@sneaxiy
sneaxiy merged commit dfa69d5 into PaddlePaddle:develop Jul 20, 2026
124 of 129 checks passed
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.

5 participants