Merged
fix(cosh): 按人工检视意见修复完善约束文档 #7044
focusforceAdd on 7 days ago
fix(cosh): 按人工检视意见修复完善约束文档 #7044
Merged
focusforceAdd on 7 days ago
focusforceCollaborators
7 days ago

描述

完善约束中不同类型的不同实现路径,并声明溢出饱和语义

关联的Issue

#4118

测试

文档更新

类型标签

likedislike
Pull Request has been successfully merged by CANN-robot.
(Thank you for focusforce's contribution.)
FfocusforceCollaborators
7 days ago Created pull request, commit f73498da
FfocusforceCollaborators
7 days ago Associated issue: [Documentation|文档反馈]: aclnnInplaceCosh 的 A2 面 dtype 支持表漏列 BFLOAT16/COMPLEX64/COMPLEX128(910B 实测三种均可用)
atomgit-bot
atomgit-bot
7 days ago Comments:

变更摘要

本 PR 属于文档完善,对应 issue #4118 的人工检视意见,针对 aclnnCosh 与 aclnnInplaceCosh 接口文档补充了两项约束说明:不同输入数据类型的执行路径差异,以及计算结果溢出时的溢出语义(输出 inf 而非饱和截断)。改动同步落在中英文两份文档 math/cosh/docs/aclnnCosh&aclnnInplaceCosh.md 与 aclnnCosh&aclnnInplaceCosh_en.md 中,不涉及接口签名或实现代码变更。

主要改动

  • 补充执行路径约束:明确 FLOAT、FLOAT16、BFLOAT16 输入经 AI Core 计算,DOUBLE、COMPLEX64、COMPLEX128 输入经 AI CPU 计算,从而使调用方了解不同数据类型的计算后端差异。
  • 说明低精度整型/布尔类型的转换路径:INT8、INT16、INT32、INT64、UINT8、BOOL 输入会先转换为 FLOAT 参与计算,计算结果再转换为 out 的数据类型。
  • 声明溢出饱和语义:输入与输出数据类型相同时,若 cosh(self) 结果超出该类型最大可表示范围,输出为 inf,不会饱和截断为该类型最大值。
  • 给出各类型溢出阈值示例:以 FLOAT(约 89.4)、FLOAT16(约 11.8)、BFLOAT16(约 89.4)、DOUBLE(约 710.5)的输入绝对值阈值说明何时输出 inf。
  • 中英文文档保持一致:上述两项约束在英文版文档中以对应表述同步补充,保证双语接口说明一致。

🧩 本次 Review 已使用自定义 Skill:

likedislike
Incorrect?
atomgit-bot
atomgit-bot
7 days ago Comments:

代码审查

检视结论

逐文件确认

  • math/cosh/docs/aclnnCosh&aclnnInplaceCosh.md:已检视(第 366-370 行新增两条约束)。执行路径与 op_api/cosh.cpp:26-38(AI Core 列表 FLOAT/FLOAT16/BF16)、op_kernel_aicpu/cosh_aicpu_def.cpp:19-20(FLOAT16/FLOAT/DOUBLE/COMPLEX64/COMPLEX128)、op_api/aclnn_cosh.cpp:47-49,127-137(整型 Cast 到 FLOAT 后按 out dtype 回转)逐项一致;溢出阈值的数值(FLOAT/BFLOAT16 ≈89.4、FLOAT16 ≈11.8、DOUBLE ≈710.5 与 max 值 acosh 计算及 op_kernel/arch35/cosh.h:27,81,234-243 的 +inf 兜底一致),no issues。
  • math/cosh/docs/aclnnCosh&aclnnInplaceCosh_en.md:已检视(第 373-374 行新增两条约束),发现 1 处问题(见下)。

Findings

优先级 位置 问题
P2 (0.6) aclnnCosh&aclnnInplaceCosh_en.md:373-374 新增“执行路径/溢出行为”声明(含 BFLOAT16、COMPLEX64、COMPLEX128、INT8..BOOL)与本文件上方参数表(第 107/117/261 行仍只有 FLOAT/FLOAT16/DOUBLE,第 144/288 行平台标记仅 npu="A3")自相矛盾,且与已修好的中文版(npu="950,A3,910b"、表内含 complex/整型)不一致,英文读者仍会误判 Atlas A2 的 dtype 支持面。

总体风险评估

改动为纯文档补充,无运行时代码、无构建或测试变更;唯一风险是英文版未随本 PR 与中文版对齐,造成中英文资料与文档内部声明冲突。建议合并前补齐英文文档的 dtype 表与平台标记。

类型 数量
🔴 阻塞 0
🟡 建议 1

💬 仅评论


🧩 本次 Review 已使用自定义 Skill:

likedislike
Incorrect?
CANN-robotCANN-robotCollaborators
7 days ago Added label: stat/needs-squash
CANN-robotCANN-robotCollaborators
7 days ago Added label: cann-cla/yes
CANN-robot
CANN-robotCollaborators
7 days ago Comments:

CLA Signature Pass

focusforce, thanks for your pull request. All authors of the commits have signed the CLA. 👍

likedislike
CANN-robot
CANN-robotCollaborators
7 days ago Comments:

Thanks for your pull-request.
The full list of commands accepted by me can be found at here.
You can get sig-info at here.
You can self-configure the PR merge rules for this repository. For more details, please refer to Here.
For more, you also can visit HICANN.


PR Approval Progress

✅ Congratulations! All modules have met the lgtm and approve requirements.

Module Approval Details

module lgtm status approve status
*/*/docs/acl*.md ✅ 冯英, zhanw_coding (2/2) ✅ 冯英 (1/1)

💡 Tip:

  • Committer can comment /approve or /lgtm
  • Commenting /approve implies both code review (lgtm) and intent to merge (approve)

CLA Signature Pass

focusforce, thanks for your pull request. All authors of the commits have signed the CLA. 👍

likedislike
CANN-robotCANN-robotCollaborators
7 days ago Set FelixTang7,yue-ma,zhou-qilong,gitee-duhuiping,wangrui_,rxtfeng,loov1,zhangzijie,jia0liang,zl_hw,songkai111,gitcode-chenjiao,chenxingyu18,pingchuantang,llimwang,gubaocheng,andong_hw,fengying555682,xuejinghui,onanfield,zhanw_coding,jiangjiawei,clinglai0517,caiwenwen,yanglu-1 as approver
CANN-robotCANN-robotCollaborators
7 days ago Set gitee-duhuiping,gitcode-chenjiao,fengying555682,caiwenwen,yanglu-1 as approver
focusforceCollaborators
7 days ago Comments:

/compile

likedislike
FfocusforceCollaborators
7 days ago Pre merge succeeded (commit_id: 579168c7c8bdf671c56d793e938b719a006c0d15)
atomgit-bot
atomgit-bot7 days agoStart code review1
math/cosh/docs/aclnnCosh&aclnnInplaceCosh_en.md
@@ -371,2 +371,4 @@
371371- Deterministic computation:
372372 - aclnnCosh and aclnnInplaceCosh default to a deterministic implementation.
373+- Execution path: FLOAT, FLOAT16, and BFLOAT16 inputs are computed on AI Core; DOUBLE, COMPLEX64, and COMPLEX128 inputs are computed on AI CPU; INT8, INT16, INT32, INT64, UINT8, and BOOL inputs are first converted to FLOAT for computation, and the computation results are then converted to the data type of out.
374+- Overflow behavior: When the input and output data types are the same, if the computation result of cosh(self) exceeds the maximum representable range of the data type, the output is inf, instead of being saturated to the maximum value of the data type. For example, the output is inf when the absolute value of the input exceeds approximately 89.4 for FLOAT, approximately 11.8 for FLOAT16, approximately 89.4 for BFLOAT16, or approximately 710.5 for DOUBLE.
atomgit-bot
atomgit-bot7 days agoComments:

🟡 Medium Priority

新增行(第 373-374 行)声明「FLOAT、FLOAT16、BFLOAT16 走 AI Core;DOUBLE、COMPLEX64、COMPLEX128 走 AI CPU;INT8/INT16/INT32/INT64/UINT8/BOOL 先转 FLOAT 计算」,与代码一致(math/cosh/op_api/cosh.cpp:26-38 AICORE 列表含 BF16;op_kernel_aicpu/cosh_aicpu_def.cpp:19-20 含 DOUBLE/COMPLEX64/COMPLEX128;aclnn_cosh.cpp:47-49、127-137 整型先 Cast 到 FLOAT 再转 out dtype)。但同文件上方的参数表并未同步:
而中文版第 105/115/257 行表内已列出 INT8..BOOL、COMPLEX64、COMPLEX128,且第 142 行、284 行平台标记为 npu="950,A3,910b"(含 Atlas A2)。结果:新增行让英文文档内部出现「约束说明支持 complex/bf16/整型,但参数表不支持」的直接矛盾,且与中文版不一致;按英文文档在 Atlas A2 上选型的用户仍会误判 bf16/complex 不可用(这正是关联 Issue #4118 所报的资料低报问题),或误以为整型入参在非 A3 平台可用。修复方向:把英文版两处参数表及平台标记与中文版对齐(self/out/selfRef 表补 COMPLEX64、COMPLEX128、INT8..BOOL,补充段标记改为 npu="950,A3,910b" 并声明 A2 额外支持 BFLOAT16)。

建议:同步修改英文文档参数表与平台标记:第 107/117 行补齐 COMPLEX64、COMPLEX128、INT8/INT16/INT32/INT64/UINT8/BOOL(与中文第 105/115 行一致),第 261 行 selfRef 补齐 COMPLEX64、COMPLEX128(与中文第 257 行一致),第 144、288 行的 npu="A3" 改为 npu="950,A3,910b" 并按中文第 143、285 行声明「self/out/selfRef 额外支持 BFLOAT16」,避免新增的执行路径说明与参数表冲突。

likedislike
Incorrect?
CANN-robot
CANN-robotCollaborators
7 days ago Comments:
🚀 CI 流水线已启动
📋 执行详情: 点击查看流水线
likedislike
CANN-robotCANN-robotCollaborators
7 days ago Added label: ci-pipeline-running
12 event messages are collapsed here。 See more
FfocusforceCollaborators
7 days ago Resolved final issue
zhanw_coding
zhanw_codingCollaborators
7 days ago Comments:

/lgtm
/approve

likedislike
fengying555682Collaborators
7 days ago Comments:

/lgtm
/approve

likedislike
CANN-robot
CANN-robotCollaborators
7 days ago Comments:

The following users do not have permission to comment /lgtm or /approve on any module in this PR:
/approve: zhanw_coding

likedislike
CANN-robotCANN-robotCollaborators
7 days ago Added label: lgtmapproved
CANN-robotCANN-robotCollaborators
7 days ago Closed associated issue
CANN-robotCANN-robotCollaborators
7 days ago Merged pull request