Merged
fix docs about compact=1 #7261
ChenYongji123Add on 9 days ago
fix docs about compact=1 #7261
Merged
ChenYongji123Add on 9 days ago
ChenYongji123
9 days ago

PR 应该避免没有描述信息,描述信息的最重要的作用是描述修改意图,应该清晰明了。例如:标题中包含了 fix 标签,如果没有描述信息,就需要Review人员猜测PR具体修复了哪些内容,或者怀疑修改的必要性。如果PR和特定的Issue相关,可以单独包含一行 Related Issue: 放到描述信息最后。

下面是一个典型的PR描述信息:

The origin issue template brings too many noise, it's not suitable for git message. eg: comments
are not removed, labels are redundant as the PR tile already contains the tag.

The guide gives a clear suggestion on how to write the PR body.

Related Issue: #1234,#5678

更多描述信息见:Commit Message 规范

likedislike
Pull Request has been successfully merged by CANN-robot.
(Thank you for ChenYongji123's contribution.)
CChenYongji123
9 days ago Created pull request, commit 2db5f46f
atomgit-bot
atomgit-bot
9 days ago Comments:

变更摘要

本 PR 为文档修正,统一说明 pypto_pro.language.TileType 的 compact 参数默认值为 1(普通紧凑模式)。原先文档把默认值写成 None,并把 0 或 None 一并描述为“不启用紧凑模式”;本次修改后,明确省略参数时取 1、显式传 None 时底层默认同样是普通紧凑模式、关闭紧凑模式必须显式设置 compact=0。相关 API 参考、编程指南示例与说明文字已同步更新。

主要改动

  • TileType 接口签名默认值: docs/zh/api/pro_api/SIMD-API/basic_data_structures/TileType.md 中构造签名由 compact: Optional[int] = None 改为 compact: Optional[int] = 1,并重写参数说明以区分默认值 1、None 与显式 0 的语义。
  • 取值范围表述调整: 参数枚举由“0或None:不启用紧凑模式”改为“0:不启用紧凑模式”,并把“1”标注为默认取值。
  • 示例与注释改为显式 compact=0: 同一文档中“不配置 compact”的说明改为“显式配置 compact=0”,make_tile_group 示例里的 L0A、L0C Tile 补充 compact=0,相关注释同步更新为“显式设置 compact=0”。
  • 尾块处理说明补充: move.md 在尾块场景注意事项中补充 compact 默认值为 1、仍需正确设置 valid_shape,以及显式 compact=0 时须确认搬运路径满足非紧凑布局要求;cube_computation.md 和 cube_vector_fusion.md 将 compact=1 标注为默认模式(示例中显式写出)。
  • 编程指南一致性更新: tile_creation_and_operations.md 的 TileType 示例与参数表改为 compact=1 并注明关闭紧凑模式需设为 0;tile_computation.md 把“通常不需要配置 compact”改为“沿用默认值 1”,场景表中 compact 列统一改为“通常沿用默认值 1”。
likedislike
Incorrect?
atomgit-bot
atomgit-bot
9 days ago Comments:

代码审查

审查结论

本 PR 为纯文档修改(compact 默认值由 None 改为 1),整体方向与标题一致,未涉及命令、权限、依赖或测试流程,无安全面风险;主要问题集中在“默认值切换后遗留的表述冲突”,需要作者与实现侧确认一次。

逐文件结果

文件 结论
docs/zh/api/pro_api/SIMD-API/basic_data_structures/TileType.md 发现 2 个问题(见下)
docs/zh/api/pro_api/SIMD-API/memory_data_movement/move.md:73 无问题(新增句与 TileType.md 新默认值表述一致)
docs/zh/guide/programming_guide/pro/development/cube_computation.md:418 无问题(该行与示例中显式 compact=1 一致;但同文件 147 行图2 段落与 TileType.md:48 冲突,作为证据在问题1中引用)
docs/zh/guide/programming_guide/pro/development/cube_vector_fusion.md:272 无问题(与示例显式 compact=1 一致)
docs/zh/guide/programming_guide/pro/development/tile_creation_and_operations.md:29/42/271 无问题(原型示意与参数表一致)
docs/zh/guide/programming_guide/pro/development/vector_computation/tile_computation.md:267/413-417 无问题(表格“通常沿用默认值1”与新默认值自洽)

问题清单(按优先级)

  • P2(1 个) TileType.md:48 —— “L1 Buffer 上配置与否对结果无影响”与 cube_computation.md:147 “compact=1 会按 valid shape 紧凑解释片上布局、不使用图2 的标准分形边界”相互矛盾;默认值改为 1 后该冲突被放大,读者按图2 计算 L1 分形地址偏移可能得到错误布局。confidence 0.4(需实现侧裁定哪一侧正确)。
  • P3(1 个) TileType.md:302-315 —— 示例中 x_wide、y_l0b 未像同段 x_l0a/c_l0c 那样显式 compact=0,仍依赖旧默认值;且这两个 Tile 的 valid_shape 保持动态且从未 set_validshape,与“全量物理搬运保证拼接连续”的说明存在不一致风险。confidence 0.45。

总体风险判断:低。改动无安全与流程风险,但默认值语义翻转后存在两处表述不一致,建议合入前先厘清 L1 Buffer 是否受 compact 影响,并补齐示例中遗漏的显式参数。

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

💬 仅评论

likedislike
Incorrect?
CANN-robotCANN-robotCollaborators
9 days ago Added label: cann-cla/yes
CANN-robot
CANN-robotCollaborators
9 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 ✅ lyz0328, gaoxiang618 (2/2) ✅ lyz0328 (1/1)

💡 Tip:

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

CLA Signature Pass

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

likedislike
CANN-robotCANN-robotCollaborators
9 days ago Set xinweiliu,lyz0328,cam_bxh,dai-zhentao,lululu40888,shi-dingkun,gguuaanngg,codehjp423,gaoxiang618,wangzheng97,DCGDDD,syfeng,jason_yuan_ye,foundea,ym-HW,hw_zhangzhiqi,yinjie27_,congqiannan,zhangyuebing,tsungl4,gaoxingwang,zhiweiyuan,caodongdong,make_pto,zhoubotcam,linjiashu,wangbing110,WenboCodes as approver
CANN-robotCANN-robotCollaborators
9 days ago Set lyz0328,foundea,yinjie27_,linjiashu as approver
atomgit-bot
atomgit-bot9 days agoStart code review2
docs/zh/api/pro_api/SIMD-API/basic_data_structures/TileType.md
Outdated
@@ -46,3 +46,3 @@
4646| fractal | 输入 | 可选,分形大小,整数或None(默认)。L0C Buffer中的FP32/INT32 Tile在未指定时自动设为1024;L1 Buffer中layout为ZZ或NN的DT_FP8E8M0 Tile及L0A_MX Buffer/L0B_MX Buffer中的Tile默认且仅支持fractal=32;其他取值要求由具体Tile API决定。 |
4747| pad | 输入 | 可选,填充模式,支持[pypto_pro.language.TilePad](TilePad.md)类型或int类型。当为int类型时取值范围为[0, 3]:<br>- 0:不填充。<br>- 1:补零。<br>- 2:补最大值。<br>- 3:补最小值。<br>非法值报ValueError,非法类型报TypeError。 |
48-| compact | 输入 | 可选,Tile缓冲区的紧凑布局模式,描述Tile在搬运、重排和矩阵计算路径中的布局解释方式,不改变数据类型,也不代替valid_shape对实际有效区域的描述。取值如下:<br>- **0或None**:不启用紧凑模式。此时L1 Buffer→L0A Buffer/L0B Buffer的[move](../memory_data_movement/move.md)完全按物理shape搬运,set_validshape在该搬运路径上不生效。<br>- **1**:使用普通紧凑模式。数据在valid_shape向上对齐到分形粒度的有效空间内连续排布,Tile声明的其余空间空闲在尾部,详见[普通紧凑模式的数据排布](#普通紧凑模式的数据排布)。通常用于尾块场景,与set_validshape搭配使用。在L1 Buffer上配置与否对结果无影响。与phase搭配使用时,如果存在尾块,L0C Buffer必须配置compact=1,否则可能会卡死。compact不会填充无效区域,需要填充时通过pad参数或fillpad补齐。compact不改变缓冲区的分配大小,只改变数据在缓冲区内的排布,因此把多个Tile拼成一个更宽的操作数时需特别注意地址偏移,详见[多Tile拼接的尾块处理](#多tile拼接的尾块处理)。<br>- **2**:使用RowPlusOne紧凑模式。仅在UB Tile中配置,用于避免以该Tile为源作搬运时的bank冲突,详见[RowPlusOne紧凑模式的数据排布](#rowplusone紧凑模式的数据排布)。NZ格式下每个分形列多预留一行物理空间,仅作占位不参与计算,因此申请Tile的物理shape时须包含多出来的这一行,数据使用时通过set_validshape配置实际的有效行数;ZN格式同理,多预留的是一列。 |
48+| compact | 输入 | 可选,默认值为1,即普通紧凑模式。省略参数时使用该默认值;显式传入None时,创建IR会采用底层默认模式,当前同样为普通紧凑模式。关闭紧凑模式须显式设置compact=0。Tile缓冲区的紧凑布局模式描述Tile在搬运、重排和矩阵计算路径中的布局解释方式,不改变数据类型,也不代替valid_shape对实际有效区域的描述。取值如下:<br>- **0**:不启用紧凑模式。此时L1 Buffer→L0A Buffer/L0B Buffer的[move](../memory_data_movement/move.md)完全按物理shape搬运,set_validshape在该搬运路径上不生效。<br>- **1(默认)**:使用普通紧凑模式。数据在valid_shape向上对齐到分形粒度的有效空间内连续排布,Tile声明的其余空间空闲在尾部,详见[普通紧凑模式的数据排布](#普通紧凑模式的数据排布)。通常用于尾块场景,与set_validshape搭配使用。在L1 Buffer上配置与否对结果无影响。与phase搭配使用时,如果存在尾块,L0C Buffer必须配置compact=1,否则可能会卡死。compact不会填充无效区域,需要填充时通过pad参数或fillpad补齐。compact不改变缓冲区的分配大小,只改变数据在缓冲区内的排布,因此把多个Tile拼成一个更宽的操作数时需特别注意地址偏移,详见[多Tile拼接的尾块处理](#多tile拼接的尾块处理)。<br>- **2**:使用RowPlusOne紧凑模式。仅在UB Tile中配置,用于避免以该Tile为源作搬运时的bank冲突,详见[RowPlusOne紧凑模式的数据排布](#rowplusone紧凑模式的数据排布)。NZ格式下每个分形列多预留一行物理空间,仅作占位不参与计算,因此申请Tile的物理shape时须包含多出来的这一行,数据使用时通过set_validshape配置实际的有效行数;ZN格式同理,多预留的是一列。 |
atomgit-bot
atomgit-bot9 days agoComments:

🟡 Medium Priority

改动行 TileType.md:48(compact 参数说明整行重写,默认值由 None 改为 1)仍保留原句“在L1 Buffer上配置与否对结果无影响”,但 cube_computation.md:147 明确写“(图2)展示compact=0时的标准分形布局…compact=1会按valid shape紧凑解释片上布局,不使用图2所示的完整标准分形边界”,而图2描述的正是 GM→L1 Buffer 的 ND→NZ 分形转换。两处对“L1 Buffer 中的 Tile 是否受 compact 影响”给出相反结论。

失效链路:本次把默认值改为 1 后,未显式指定 compact 的 L1 Tile 即处于 compact=1 状态。读者若按图2(48×64 标准分形边界)计算 L1 上的分形地址偏移(典型场景:多 Tile 拼接、转置搬入、按物理地址划分 L1 子块),而实际按 valid_shape 紧凑排布,则地址偏移计算错误,搬运后得到错误分形数据,最终 matmul 结果错误。

该矛盾在旧默认值(不启用紧凑)下影响有限,默认值切换为 1 后被放大,属于本次改动引入的新不一致。需要作者确认实现侧到底哪种说法成立:若不生效,应删掉 TileType.md 该句;若生效,应在 cube_computation.md 图2段落说明默认值已为 1、图2 仅对应显式 compact=0 的场景。

建议:与实现侧确认 L1 Buffer 的 Tile 是否受 compact 影响,然后统一两处表述:若不生效则删除 TileType.md:48 中的“在L1 Buffer上配置与否对结果无影响”歧义句并显式说明;若生效则修改 cube_computation.md:147 图2段落,指明默认 compact=1 时 L1 布局按 valid_shape 紧凑解释、图2 仅对应显式 compact=0。

likedislike
Incorrect?
System
System MessageSystem
9 days ago Comments:

changed this line on ce1ec0ee view diff detail

CChenYongji123
9 days ago Pre merge succeeded (commit_id: 0973232aaeb35d1fa276f445905382880232136a)
8 event messages are collapsed here。 See more
CANN-robotCANN-robotCollaborators
9 days ago Added label: cann-cla/yes
ChenYongji123
9 days ago Comments:

compile

likedislike
CChenYongji123
9 days ago Pre merge succeeded (commit_id: 9ec5ce9483f8fff5576595420c64c41b6fecb436)
CANN-robot
CANN-robotCollaborators
9 days ago Comments:
🚀 CI 流水线已启动
📋 执行详情: 点击查看流水线
likedislike
CANN-robotCANN-robotCollaborators
9 days ago Added label: ci-pipeline-running
CANN-robotCANN-robotCollaborators
8 days ago Removed label: ci-pipeline-running
CANN-robotCANN-robotCollaborators
8 days ago Added label: ci-pipeline-passed
lyz0328
lyz0328Collaborators
8 days ago Comments:

/approve

likedislike
CANN-robotCANN-robotCollaborators
8 days ago Added label: approved
gaoxiang618Collaborators
8 days ago Comments:

/approve
/lgtm

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

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

likedislike
CANN-robotCANN-robotCollaborators
8 days ago Added label: lgtm
CANN-robot
CANN-robotCollaborators
8 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
Ggaoxiang618Collaborators
8 days ago Resolved final issue
CANN-robotCANN-robotCollaborators
8 days ago Merged pull request