Merged
docs: StatelessNormal README明确mean/stdev张量仅第0项参与计算,与算子实际实现对齐 #6916
docs: StatelessNormal README明确mean/stdev张量仅第0项参与计算,与算子实际实现对齐 #6916
Merged
aiteerAdd on 10 days ago
aiteer
aiteer
10 days ago

描述

README 中 mean/stdev 的参数描述及“同形”约束确实与当前实现契约存在偏差:按 stateless_normal_proto.h 中的原型注释及 NormalLauncher 实现,Tensor 入参实际仅使用第 0 项,所有输出元素共用同一组均值和标准差。本pr明确mean/stdev张量仅第0项参与计算,与算子实际实现对齐。

关联的Issue

#4232

测试

文档更新

更新了StatelessNormal README.md文件

类型标签

likedislike
Pull Request has been successfully merged by CANN-robot.
(Thank you for aiteer's contribution.)
aiteeraiteer
10 days ago Created pull request, commit 93802732
aiteeraiteer
10 days ago Associated issue: [Documentation|文档反馈]: StatelessNormal 对同形 mean/stdev 张量的描述未说明实际上只使用首项
atomgit-bot
atomgit-bot
10 days ago Comments:

变更摘要

本 PR 属于纯文档更新,目标是让 random/stateless_normal/README.md 中 mean、stdev 两个输入参数的说明与算子实际实现约定一致。根据 stateless_normal_proto.h 的原型注释和 NormalLauncher 实现,mean、stdev 是张量时,实际只使用第 0 项,所有输出元素共用同一组均值和标准差。因此原 README 中“支持标量或与输出同形状的张量”和“shape 必须与输出 y 的 shape 一致”的表述被替换为“仅第 0 项参与计算,其余元素不参与计算”。

主要改动

  • 计算公式说明补充张量语义:在 y = BoxMuller(Philox(seed, offset)) \times stdev + mean 公式后新增说明,明确 mean、stdev 可为标量或张量;为张量时仅第 0 项参与计算,所有输出元素共用这组均值和标准差。
  • mean 参数描述修正:将“支持标量或与输出同形状的张量”改为“支持标量或张量。当为张量时,仅第 0 项参与计算,所有输出元素共用该均值”。
  • stdev 参数描述修正:将“支持标量或与输出同形状的张量”改为“支持标量或张量。当为张量时,仅第 0 项参与计算,所有输出元素共用该标准差”。
  • 约束条件列表更新:删除“mean 和 stdev 为 tensor 时 shape 必须与输出 y 的 shape 一致(不支持广播)”这一条,替换为仅第 0 项参与计算、其余元素不参与计算的说明,与参数表表述保持一致。

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

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

代码审查

审查总结

变更文件逐一确认:

  • random/stateless_normal/README.md:已审查(新增第 24 行公式后的语义说明、第 68/75 行 mean/stdev 参数描述、第 92 行约束条目替换)。发现 1 处问题——第 92 行把「shape 必须与输出 y 一致(不支持广播)」整条删除,仅保留「仅第0项参与计算」,可能导致实际仍生效的 shape 校验失去文档覆盖,形成文档契约与实现不一致。其余新增/修改文案与 Issue #4232 所述 proto 注释语义一致,未发现其他问题。

问题统计:P0=0,P1=0,P2=1,P3=0。

整体风险判断:本次为纯文档改动,不触及运行行为;唯一风险点是删除了原文档中的 shape 约束描述,若该约束在 op_host/infershape 中仍保留,会误导使用者传入异形 mean/stdev 张量并在 shape 推导阶段失败。建议作者核对实现后决定是「保留约束」还是「明确声明形状无要求」。

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

💬 仅评论


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

likedislike
Incorrect?
CANN-robotCANN-robotCollaborators
10 days ago Added label: cann-cla/yes
CANN-robot
CANN-robotCollaborators
10 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
*/*/README.md ✅ 陈娇, xuejinghui (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

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

likedislike
CANN-robotCANN-robotCollaborators
10 days ago Set 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,clinglai0517,caiwenwen,yanglu-1 as approver
CANN-robotCANN-robotCollaborators
10 days ago Set gitee-duhuiping,gitcode-chenjiao,fengying555682,caiwenwen,yanglu-1 as approver
atomgit-bot
atomgit-bot10 days agoStart code review2
random/stateless_normal/README.md
@@ -88,3 +90,3 @@
8890- offset必须为4的倍数。
8991- mean和stdev的数据类型与输出y锁步一致。
90-- mean和stdev为tensor时,shape必须与输出y的shape一致(不支持广播)。
92+- mean和stdev为tensor时,仅第0项参与计算,所有输出元素共用这组均值和标准差,其余元素不参与计算。
atomgit-bot
atomgit-bot10 days agoComments:

🟡 Medium Priority

变更行(README.md 第 92 行)把原来的 - mean和stdev为tensor时,shape必须与输出y的shape一致(不支持广播)。 整条删除,只替换为「仅第0项参与计算」。

影响与证据链:

  1. 原始约束同时包含两个信息:(a) shape 必须与 y 一致;(b) 不支持广播。本次替换把 (a)(b) 全部抹掉,新文本对 mean/stdev 张量允许的 shape 不作任何说明,读者会自然推断「形状任意,只要读第 0 项即可」。
  2. PR 背景与关联 Issue #4232 只说 README「暗示逐元素参数」与 proto 注释 Only the 0th element is used for calculation if a tensor is input、NormalLauncher 读取 meanGlobal_(0)/stdGlobal_(0) 不一致;该 Issue 的「存在的问题」部分只针对逐元素语义,并未指出 shape 相等约束是错的 —— 说明 shape 一致这条约束很可能仍在 op_host/infershape、tiling 或 op_api 校验中生效(本次改动没有改任何实现代码,校验逻辑不变)。
  3. 若该校验仍生效,失败模式为:用户按新文档传入与输出 y 不同形的 mean/stdev 张量(例如 y 为 [1024]、mean 为 [1]),在 infershape/tiling 阶段报 shape 不匹配错误,产生「文档说形状无限制、实际直接失败」的生产误用。属文档契约与实际实现不一致的回归。

说明:受审查工具范围限制,无法读取 op_graph/op_host 源码确认该校验是否仍存在,因此置信度下调;若实现确实已放开 shape 校验,则该删除是正确的,可忽略本条。

建议:先核对 op_host infershape / tiling / op_api 中 mean、stdev 的 shape 校验是否仍要求与输出 y 同形:若仍要求,则保留原约束并与新增语义合并成一句,例如「mean和stdev为tensor时,shape需与输出y一致(不支持广播),且仅第0项参与计算,所有输出元素共用该组均值和标准差」;若实现已放开 shape 校验,则维持当前写法,并明确说明张量 shape 无特殊要求、仅取第 0 项。

likedislike
Incorrect?
aiteer
aiteer
10 days ago Comments:

经核实该约束在实现中从未存在(tiling 侧 mean/stdev 仅校验 dtype、infershape 仅由 shape 输入推导 y、kernel 仅读第 0 项),且“将形状约束与 host 实际校验统一”正是 issue #4232 的调整方向,故删除该条为修复动作本身,本条可关闭。

aiteer
aiteer
10 days ago Comments:

compile

likedislike
aiteeraiteer
10 days ago Pre merge succeeded (commit_id: 6343d445f6517cfef64f34b639df10fabbc8e9a8)
CANN-robot
CANN-robotCollaborators
10 days ago Comments:
🚀 CI 流水线已启动
📋 执行详情: 点击查看流水线
likedislike
CANN-robotCANN-robotCollaborators
10 days ago Added label: ci-pipeline-running
CANN-robotCANN-robotCollaborators
10 days ago Removed label: ci-pipeline-running
CANN-robotCANN-robotCollaborators
10 days ago Added label: ci-pipeline-passed
chenyifengCollaborators
10 days ago Comments:

您好,PR已满足合入指标,请尽快联系Committer进行合入。如暂时不需要合入,请在PR标题前加[WIP]

likedislike
chenjiaoCollaborators
10 days ago Comments:

/approve

likedislike
CANN-robotCANN-robotCollaborators
10 days ago Added label: approved
xuejinghuiCollaborators
10 days ago Comments:

/lgtm

likedislike
CANN-robotCANN-robotCollaborators
10 days ago Added label: lgtm
CANN-robot
CANN-robotCollaborators
10 days ago Comments:

The MR can not be merged, because of CodeReview discussion not resolved

If you want to solve this problem, you can click here to do it in the FAQs.

likedislike
XxuejinghuiCollaborators
10 days ago Resolved final issue
CANN-robotCANN-robotCollaborators
10 days ago Closed associated issue
CANN-robotCANN-robotCollaborators
10 days ago Merged pull request