已合并
AddMaxPool3DGrad #799
huohuo_wangyan创建于 1月19日
AddMaxPool3DGrad #799
已合并
huohuo_wangyan创建于 1月19日
1月19日

描述

  1. 新增算子实现:通过两段式接口(获取工作空间与执行)实现了上述反向传播算子,包含严格的参数校验、张量格式转换及智能选择底层计算内核等逻辑。2) 代码重构与优化:对 AdaptiveMaxPool3DGrad 和 MaxPool3DGradWithArgmax 等算子的内核及分片(Tiling)实现进行了深度重构,通过引入公共函数库和模板基类(如 pool_3d_common)大幅消除了代码重复,统一了计算逻辑,并修复了输出维度计算、缓冲区初始化等潜在缺陷。3) 文档与构建完善:全面更新了相关算子的API文档,细化了产品支持列表,补充了结构化参数说明和完整调用示例;简化了多个项目的CMake构建逻辑,提升了构建的清晰度和可维护性。4) 测试增强:修正了测试代码中的路径与变量错误,新增了针对重叠模式等边界场景的单元测试,提升了测试的健壮性和覆盖率。

关联的Issue

测试

文档更新

类型标签

likedislike
Pull Request已成功合入, 合并人@CANN-robot
(感谢 huohuo_wangyan 的贡献)
CANN-robot
CANN-robot成员
1月19日 评论:

Thank your for your pull-request.

The full list of commands accepted by me can be found at here.

You can get sig-info at here

likedislike
CANN-robot
CANN-robot成员
1月19日 评论:

以下是根据您提交的修改文件推荐的Reviewer和Committer序列,需各模块评审通过后方可合入

Module List Reviewers Committers
*/*/op_api/*.h lileizheng, liu-wei, wangyongguang, yu-xinjie62, chaotang233 tang-lei01, wangyongguang
repo-cann/ops-nn lileizheng, liu-wei, wangyongguang, yu-xinjie62, chaotang233 crystalhu, tangweiwei2, chenqi317, liubo75
likedislike
CANN-robotCANN-robot成员
1月19日 将crystalhu,chenqi317,liubo75,tangweiwei2,wangyongguang设为审查人
CANN-robotCANN-robot成员
1月19日 将chaotang233,FelixTang7,yu-xinjie62,zhajianqing123,liu-wei,lileizheng,zhangyuxiang0119,wangyongguang设为评审人
CANN-robotCANN-robot成员
1月19日 添加了label:cann-cla/yes
CANN-robot
CANN-robot成员
1月19日 评论:

CLA Signature Pass

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

likedislike
CANN-robot
CANN-robot成员
1月19日 评论:

🔵 source code change are detected, tasks labels is removed in this pull request!

likedislike
huohuo_wangyan
1月19日 评论:

compile

likedislike
CANN-robotCANN-robot成员
1月19日 添加了label:ci-pipeline-running
CANN-robot
CANN-robot成员
1月19日 评论:

流水线任务触发成功,任务链接 [94c1812370df460d85f3774b2339b691]

任务名称状态日志下载链接
codecheck ❌ FAILED >>>>>
anti_virus ✅ SUCCESS >>>>>
Check_Pr ✅ SUCCESS >>>>>
Compile_Ascend_X86 ✅ SUCCESS >>>>> >>>>>
Compile_Ascend_X86_mobile_station ✅ SUCCESS >>>>> >>>>>
Compile_Ascend_ARM ✅ SUCCESS >>>>> >>>>>
Compile_Ascend_single ✅ SUCCESS >>>>> >>>>>
API_Check 🔄 running >>>>>
UT_Test_ophost ⏳ waiting
UT_Test_opapi ⏳ waiting
UT_Test_kernel ⏳ waiting
Smoke_A900 ⏳ waiting >>>>>
likedislike
CANN-robot
CANN-robot成员
1月19日 评论:

问题/功能描述

本次PR为昇腾AI处理器算子库新增并完善了多个池化反向传播算子,主要包括 aclnnAdaptiveMaxPool2dBackwardaclnnAdaptiveMaxPool3dBackwardaclnnMaxPool3dWithArgmaxBackward,解决了相关神经网络层在训练时梯度无法正确回传的问题。同时,对已有算子的构建配置、内核实现、文档及测试进行了大规模重构、优化与维护,旨在提升代码复用性、可维护性、文档准确性及测试覆盖度。

修改方案描述

实现方案涵盖多个层面:1) 新增算子实现:通过两段式接口(获取工作空间与执行)实现了上述反向传播算子,包含严格的参数校验、张量格式转换及智能选择底层计算内核等逻辑。2) 代码重构与优化:对 AdaptiveMaxPool3DGradMaxPool3DGradWithArgmax 等算子的内核及分片(Tiling)实现进行了深度重构,通过引入公共函数库和模板基类(如 pool_3d_common)大幅消除了代码重复,统一了计算逻辑,并修复了输出维度计算、缓冲区初始化等潜在缺陷。3) 文档与构建完善:全面更新了相关算子的API文档,细化了产品支持列表,补充了结构化参数说明和完整调用示例;简化了多个项目的CMake构建逻辑,提升了构建的清晰度和可维护性。4) 测试增强:修正了测试代码中的路径与变量错误,新增了针对重叠模式等边界场景的单元测试,提升了测试的健壮性和覆盖率。

likedislike
CANN-robot
CANN-robot成员1月19日进行代码检视1
pooling/adaptive_max_pool3d_grad/op_host/adaptive_max_pool3d_grad_normal_tiling.cpp
@@ -196,6 +196,7 @@ ge::graphStatus AdaptiveMaxPool3DGradNormalTiling::DoOpTiling()
196196 bool res = SetNormalTilingParams();
197197 OP_CHECK_IF(!res, OP_LOGE(context_->GetNodeName(), "Normal cal tiling params failed."), return ge::GRAPH_FAILED);
198198 maxPoolGradParams.tilingType = TILING_TYPE_NORMAL;
199+ context_->SetScheduleMode(1);
CANN-robot
CANN-robot1月19日评论:

魔数与常量: 代码中直接使用了魔数1作为SetScheduleMode的参数,缺乏明确的含义说明。在代码检视的上下文中,虽然只关注第199行的改动,但结合前后代码分析,SetScheduleMode(1)中的参数1是一个硬编码的魔法数字,没有定义为有意义的常量。这降低了代码的可读性和可维护性,其他开发者难以理解参数1的具体含义。

问题类型: 魔数与常量
文件路径: pooling/adaptive_max_pool3d_grad/op_host/adaptive_max_pool3d_grad_normal_tiling.cpp
行号: 199
问题代码:

    context_->SetScheduleMode(1);

修改建议:

将魔法数字1定义为有意义的常量,例如:constexpr int SCHEDULE_MODE_NORMAL = 1; 并在调用时使用该常量:context_->SetScheduleMode(SCHEDULE_MODE_NORMAL);。同时建议添加注释说明该调度模式的具体含义。

此评论由代码审查工具自动生成

likedislike
CANN-robot
CANN-robot成员1月19日进行代码检视1
pooling/adaptive_max_pool3d_grad/op_kernel/adaptive_max_pool3d_grad_scatter_overlap.h
@@ -26,2 +25,3 @@
2625template <typename TX, typename TGrad, typename TArgmax, typename TY>
27-class AdaptiveMaxPool3DGradScatterOverlap : public AdaptiveMaxPool3DGradScatterBase<TX, TGrad, TArgmax, TY>
26+class AdaptiveMaxPool3DGradScatterOverlap :
27+ public MaxPool3DGradCommon::MaxPool3DGradScatterOverlapUnified<
CANN-robot
CANN-robot1月19日评论:

代码结构与可维护性: 头文件包含路径使用相对路径 '../pool_3d_common/arch32/max_pool3d_grad_scatter_overlap_unified.h',这种依赖关系不够清晰,且当文件目录结构发生变化时容易导致编译错误。相对路径增加了模块间的耦合度。

问题类型: 代码结构与可维护性
文件路径: pooling/adaptive_max_pool3d_grad/op_kernel/adaptive_max_pool3d_grad_scatter_overlap.h
行号: 19
问题代码:

#include "../pool_3d_common/arch32/max_pool3d_grad_scatter_overlap_unified.h"

修改建议:

1. 改为使用绝对路径或项目统一的包含路径策略。
2. 如果这是项目内的标准包含方式,建议添加注释说明这种包含关系的必要性。
3. 考虑将公共头文件放在统一的include目录下,使用 <> 包含。

此评论由代码审查工具自动生成

likedislike
CANN-robot
CANN-robot成员1月19日进行代码检视1
pooling/max_pool3d_grad_with_argmax/op_kernel/max_pool3d_grad_with_argmax_scatter.h
已过期
@@ -10,2 +10,3 @@
1010 
11-/* !
11+ 
12+/*!
CANN-robot
CANN-robot1月19日评论:

代码结构与可维护性: 文件头注释中的'/* !'格式不一致,通常应该是'/!'或'/ !'保持统一。虽然这是一个小问题,但代码风格的一致性对于大型项目很重要。

问题类型: 代码结构与可维护性
文件路径: pooling/max_pool3d_grad_with_argmax/op_kernel/max_pool3d_grad_with_argmax_scatter.h
行号: 12
问题代码:

/*!

修改建议:

统一文件头注释的格式,确保项目中所有文件使用相同的注释风格。建议检查项目编码规范,并遵循统一的格式。

此评论由代码审查工具自动生成

likedislike
CANN-robot
CANN-robot成员1月19日进行代码检视1
pooling/pool_3d_common/op_host/arch32/max_pool3d_grad_tiling_common.h
@@ -0,0 +196,4 @@
196+ params.totalRound = params.ncRound * params.doCnt *
197+ params.hoCnt * params.woCnt;
198+
199+ if (params.xDtypeSize != DTYPE_LEN_B32 && isOverLap) {
CANN-robot
CANN-robot1月19日评论:

硬编码数据类型大小: 在CalculateRoundParams函数中,硬编码了DTYPE_LEN_B32作为比较条件。这种硬编码的魔法数字降低了代码的可读性和可维护性。如果未来需要支持其他数据类型大小,需要修改这里的硬编码值。

问题类型: 硬编码数据类型大小
文件路径: pooling/pool_3d_common/op_host/arch32/max_pool3d_grad_tiling_common.h
行号: 199
问题代码:

if (params.xDtypeSize != DTYPE_LEN_B32 && isOverLap) {

修改建议:

将DTYPE_LEN_B32定义为有意义的常量名,或者使用枚举或配置方式管理数据类型大小。更好的做法是提供一个数据类型到大小的映射表。

此评论由代码审查工具自动生成

likedislike
CANN-robot
CANN-robot成员1月19日进行代码检视1
pooling/pool_3d_common/op_host/arch32/max_pool3d_grad_tiling_common.h
@@ -0,0 +227,4 @@
227+ tilingData.get_totalRound());
228+}
229+ 
230+} // namespace optiling
CANN-robot
CANN-robot1月19日评论:

文件结尾缺少换行符: 文件最后一行缺少换行符(No newline at end of file)。虽然这不是功能性问题,但不符合POSIX标准,某些工具(如diff、编译器)可能会产生警告。

问题类型: 文件结尾缺少换行符
文件路径: pooling/pool_3d_common/op_host/arch32/max_pool3d_grad_tiling_common.h
行号: 230
问题代码:

} // namespace optiling
#endif // OPS_BUILD_IN_OP_TILING_RUNTIME_MAX_POOL3D_GRAD_SCATTER_TILING_TEMPLATE_H

修改建议:

在文件末尾添加一个换行符。这是良好的编码习惯,可以避免一些工具的不必要警告。

此评论由代码审查工具自动生成

likedislike
CANN-robot
CANN-robot成员1月19日进行代码检视1
pooling/pool_3d_common/op_host/arch32/max_pool3d_grad_tiling_constants.h
@@ -0,0 +18,4 @@
18+namespace optiling {
19+ 
20+// 索引常量
21+constexpr uint32_t X_INDEX = 0;
CANN-robot
CANN-robot1月19日评论:

类型一致性: 常量定义中混合使用了uint32_t和size_t类型,缺乏一致性。X_INDEX、GRAD_INDEX、ARGMAX_INDEX使用uint32_t,而KSIZE_ATTR_INDEX等使用size_t。虽然这些常量都用于索引,但类型不一致可能在某些平台上导致隐式类型转换问题。

问题类型: 类型一致性
文件路径: pooling/pool_3d_common/op_host/arch32/max_pool3d_grad_tiling_constants.h
行号: 21
问题代码:

constexpr uint32_t X_INDEX = 0;
constexpr uint32_t GRAD_INDEX = 1;
constexpr uint32_t ARGMAX_INDEX = 2;
constexpr size_t KSIZE_ATTR_INDEX = 0U;

修改建议:

统一使用size_t类型定义所有索引常量,因为size_t专门用于表示对象大小和数组索引,更适合索引场景

此评论由代码审查工具自动生成

likedislike
CANN-robot
CANN-robot成员1月19日进行代码检视1
pooling/pool_3d_common/op_host/arch32/max_pool3d_grad_tiling_constants.h
@@ -0,0 +37,4 @@
37+constexpr uint32_t NUM_PER_REP_B16 = 128;
38+constexpr uint32_t NUM_PER_REP_B32 = 64;
39+constexpr uint32_t SELECT_RESERVED_UB_SIZE = 8192;
40+constexpr uint64_t MAX_INT32 = 2147483647;
CANN-robot
CANN-robot1月19日评论:

常量定义不精确: MAX_INT32常量定义为2147483647,但实际INT32_MAX应该是2147483647。虽然数值相同,但使用标准库中的INT32_MAX或std::numeric_limits<int32_t>::max()更清晰、更可移植。

问题类型: 常量定义不精确
文件路径: pooling/pool_3d_common/op_host/arch32/max_pool3d_grad_tiling_constants.h
行号: 40
问题代码:

constexpr uint64_t MAX_INT32 = 2147483647;

修改建议:

建议使用标准库定义:'constexpr int32_t MAX_INT32 = std::numeric_limits<int32_t>::max();' 或 '#include <cstdint>'后使用INT32_MAX

此评论由代码审查工具自动生成

likedislike
CANN-robot
CANN-robot成员1月19日进行代码检视1
pooling/pool_3d_common/op_kernel/arch32/max_pool3d_grad_common.h
@@ -0,0 +211,4 @@
211+ break;
212+ }
213+ self->block_.ncCntIndex = ncIndex;
214+ self->block_.ncShape =
CANN-robot
CANN-robot1月19日评论:

逻辑运算与副作用: 条件判断中使用(self->params_.ncCnt - 1UL),当ncCnt为0时,减法会下溢(因为是无符号整数),导致比较结果异常。

问题类型: 逻辑运算与副作用
文件路径: pooling/pool_3d_common/op_kernel/arch32/max_pool3d_grad_common.h
行号: 214
问题代码:

self->block_.ncShape =
    self->block_.ncCntIndex >= (self->params_.ncCnt - 1UL) ? 
    self->params_.ncTail : self->params_.baseNc;

修改建议:

1. 添加对ncCnt为0的特殊情况处理。2. 或者确保ncCnt至少为1。3. 修改条件判断逻辑,例如:self->block_.ncCntIndex + 1 >= self->params_.ncCnt

此评论由代码审查工具自动生成

likedislike
CANN-robot
CANN-robot成员1月19日进行代码检视2
pooling/pool_3d_common/op_kernel/arch32/max_pool3d_grad_scatter_base_template.h
已过期
@@ -0,0 +60,4 @@
60+ 
61+ params_.totalCnt = tiling->totalCnt; // 需要处理base矩阵个数
62+ params_.ncCntRound = tiling->ncRound; // 多核切nc,先分nc,向上取整,
63+ params_.preCoreNum = tiling->preCoreNum; // 每个核均分完后剩余nce由前preCoreNum个核进行填充
CANN-robot
CANN-robot1月19日评论:

代码结构与可维护性: 注释中'剩余nce'应为'剩余nc',可能是笔误。

问题类型: 代码结构与可维护性
文件路径: pooling/pool_3d_common/op_kernel/arch32/max_pool3d_grad_scatter_base_template.h
行号: 63
问题代码:

params_.preCoreNum = tiling->preCoreNum; // 每个核均分完后剩余nce由前preCoreNum个核进行填充

修改建议:

修正注释:// 每个核均分完后剩余nc由前preCoreNum个核进行填充

此评论由代码审查工具自动生成

likedislike
System
系统消息系统
1月19日 评论:

changed this line on 7b7176d1 view diff detail

CANN-robot
CANN-robot成员1月19日进行代码检视1
pooling/pool_3d_common/op_kernel/arch32/max_pool3d_grad_scatter_base_template.h
@@ -0,0 +65,4 @@
65+ params_.ncRealRound = 0;
66+ params_.ubSize = tiling->totalUBSize;
67+ uint64_t blockId = GetBlockIdx();
68+ if (params_.preCoreNum == 0 || blockId < params_.preCoreNum) { // 前preCoreNum个核
CANN-robot
CANN-robot1月19日评论:

代码可读性: 注释中的“填充数”可能指代不清晰,容易误解。结合上下文,应该是“每个核处理的 nc 轮数”或类似含义。

问题类型: 代码可读性
文件路径: pooling/pool_3d_common/op_kernel/arch32/max_pool3d_grad_scatter_base_template.h
行号: 68
问题代码:

// 由于轮数为向上取整,所以当前核填充数的起始位置为 填充数*核数

修改建议:

将注释改为更清晰的描述,例如:“// 由于 ncRound 是向上取整的,当前核处理的起始 nc 索引为 核ID * 每核轮数”

此评论由代码审查工具自动生成

likedislike
CANN-robot
CANN-robot成员1月19日进行代码检视2
pooling/pool_3d_common/op_kernel/arch32/max_pool3d_grad_scatter_base_template.h
已过期
@@ -0,0 +53,4 @@
53+ params_.hoTail = tiling->hoTail;
54+ params_.woTail = tiling->woTail;
55+ params_.ncCnt = tiling->ncCnt; // nc方向base矩阵个数
56+ params_.doCnt = tiling->doCnt; // h方向base矩阵个数
CANN-robot
CANN-robot1月19日评论:

注释错误: 第56行注释“h方向base矩阵个数”重复了两次,且第57行也注释为“h方向base矩阵个数”,这可能是笔误。根据变量名 params_.doCnt 和 params_.hoCnt,应该分别是 d 方向和 h 方向。

问题类型: 注释错误
文件路径: pooling/pool_3d_common/op_kernel/arch32/max_pool3d_grad_scatter_base_template.h
行号: 56
问题代码:

params_.doCnt = tiling->doCnt; // h方向base矩阵个数
params_.hoCnt = tiling->hoCnt; // h方向base矩阵个数

修改建议:

修正注释:params_.doCnt 应为“d方向base矩阵个数”,params_.hoCnt 应为“h方向base矩阵个数”。

此评论由代码审查工具自动生成

likedislike
System
系统消息系统
1月19日 评论:

changed this line on 7b7176d1 view diff detail

Hhuohuo_wangyan
1月19日 修改了pull request 的描述
huohuo_wangyan
1月19日 评论:

compile

likedislike
CANN-robot
CANN-robot成员
1月19日 评论:

流水线任务触发成功,任务链接 [5ffb16c1910d4397b41337d213069e5f]

任务名称状态日志下载链接
codecheck ❌ FAILED >>>>>
anti_virus ✅ SUCCESS >>>>>
Check_Pr ✅ SUCCESS >>>>>
Compile_Ascend_X86 ✅ SUCCESS >>>>> >>>>>
Compile_Ascend_X86_mobile_station ✅ SUCCESS >>>>> >>>>>
Compile_Ascend_ARM ✅ SUCCESS >>>>> >>>>>
Compile_Ascend_single ✅ SUCCESS >>>>> >>>>>
API_Check ✅ SUCCESS >>>>>
UT_Test_ophost ⚪ ABORTED
UT_Test_opapi ⚪ ABORTED
UT_Test_kernel ⚪ ABORTED
Smoke_A900 ⚪ ABORTED >>>>>

[2026-01-19 20:19:41]    CI执行失败

likedislike
CANN-robotCANN-robot成员
1月19日 添加了label:api-check-pass
CANN-robotCANN-robot成员
1月19日 删除了label:ci-pipeline-running
CANN-robotCANN-robot成员
1月19日 添加了label:ci-pipeline-failed
CANN-robot
CANN-robot成员
1月20日 评论:
CANN-robot
CANN-robot成员
1月20日 评论:
huohuo_wangyan
1月21日 评论:

compile

likedislike
CANN-robotCANN-robot成员
1月21日 删除了label:ci-pipeline-failed
CANN-robotCANN-robot成员
1月21日 添加了label:ci-pipeline-running
CANN-robotCANN-robot成员
1月21日 删除了label:api-check-pass
CANN-robot
CANN-robot成员
1月21日 评论:

流水线任务触发成功,任务链接 [7dca4150461b473c8e762505f4dea9af]

任务名称状态日志下载链接
codecheck ✅ SUCCESS >>>>>
anti_virus ✅ SUCCESS >>>>>
Check_Pr ✅ SUCCESS >>>>>
Compile_Ascend_X86 ✅ SUCCESS >>>>> >>>>>
Compile_Ascend_X86_mobile_station ✅ SUCCESS >>>>> >>>>>
Compile_Ascend_ARM ✅ SUCCESS >>>>> >>>>>
Compile_Ascend_single ✅ SUCCESS >>>>> >>>>>
API_Check ⚠️ WARNING >>>>>
UT_Test_ophost ✅ SUCCESS
UT_Test_opapi ✅ SUCCESS
UT_Test_kernel ✅ SUCCESS
Smoke_A900 ✅ SUCCESS >>>>> >>>>>

[2026-01-21 08:40:49]    CI执行结束

likedislike
CANN-robotCANN-robot成员
1月21日 添加了label:api-check-failed
此处折叠了5条事件消息 查看更多
CANN-robotCANN-robot成员
1月21日 添加了label:ci-pipeline-passed
wangyongguang成员
1月21日 评论:

/approve

likedislike
张喻翔
张喻翔
1月21日 评论:

/lgtm

likedislike
CANN-robotCANN-robot成员
1月21日 添加了label:lgtm
胡碧霞成员
1月21日 评论:

/approve

likedislike
CANN-robotCANN-robot成员
1月21日 添加了label:approved
CANN-robot
CANN-robot成员
1月21日 评论:

Review Guide

This Pull-Request Passes Review.
Committers who wrote a comment of /approve are: crystalhu, wangyongguang.
Reviewers who wrote a comment of /lgtm are: wangyongguang, crystalhu, zhangyuxiang0119.

likedislike
CANN-robotCANN-robot成员
1月21日 合入了pull request