Merged
[fix] index-add 确定性分支增加 #5415
[fix] index-add 确定性分支增加 #5415
Merged
L1919_snowAdd on Jul 7
L1919_snowCollaborators
Jul 7

【合入来源】

如有社区issue,请关联issue链接
请勿携带内部流程信息(需求链接、问题单、内部issue等)

【修改方案】

Index_add_,开启确定性开关,和原生对齐,走index_put_ 计算image.png

【资料变更】

“不涉及”

【接口变更】

不涉及

【功能验证】

修改后,用例精度成功:
image.png 精度失败测为 用例跑失败,非精度失败,用例跑成功的,精度都没有问题

【CheckList】

PR提交人对以下CheckList自检项进行全量自检,自检通过或不涉及,均修改 [ ] 为 [x]

likedislike
Pull Request has been successfully merged by ascend-robot.
(Thank you for L1919_snow's contribution.)
LL1919_snowCollaborators
Jul 7 Created pull request, commit aac6c979
atomgit-bot
atomgit-bot
Jul 7 Comments:

变更摘要

该 PR 为 index_add 算子的 NPU OpApi 实现新增了确定性算法分支:当全局上下文启用了 deterministicAlgorithms() 模式时,通过 index_put_ 替代 aclnnIndexAdd 来执行计算,以保证结果的可复现性。同时移除了 Dimname 重载版本上的版本条件编译宏,使其在所有版本下均可用,并补充了必要的头文件引入。

主要改动

  • 新增确定性分支: 在 index_add_out 和 index_add 两个重载中增加了 at::globalContext().deterministicAlgorithms() 检查,当确定性模式开启时,将 dim 包装后构建 c10::List<c10::optional<at::Tensor>> 索引列表,通过 result.index_put_(indices, source * alpha, true) 完成累加,而非调用 aclnnIndexAdd。
  • 移除版本条件编译: 删除了 Dimname 参数重载 index_add 上的 #if !VERSION_BETWEEN(V2R13, VERSION_NEWEST) 宏守卫,使该重载在所有版本下编译可用。
  • 新增头文件: 添加了 #include <ATen/ATen.h>,为 at::globalContext() 和 at::maybe_wrap_dim 等调用提供声明支持。
  • 修正非确定路径的参数: 将 index_add(非 _out 版本)中 EXEC_NPU_CMD 宏的最后一个参数从 result.copy_(self) 改为 result,避免冗余拷贝。
  • 命名空间注释规范化: 将文件末尾的 } 改为 } // namespace op_api,明确命名空间闭合范围。
likedislike
Incorrect?
atomgit-bot
atomgit-bot
Jul 7 Comments:

代码审查

我已经完成了全面审查。以下是总结。


审查总结

审查文件: op_plugin/ops/opapi/IndexAddKernelNpuOpApi.cpp(已审查,发现 1 个问题)

优先级 数量
P0 0
P1 1
P2 0
P3 0

总体风险评估:中等风险。 此次变更主体逻辑(确定性分支增加、冗余 result.copy_(self) 修正)是正确的,但附带移除了 index_add(Dimname) 重载的版本守卫 #if !VERSION_BETWEEN(V2R13, VERSION_NEWEST),导致在 V2R13+ 构建中 dimname_to_position 符号未定义,会引发编译失败。该守卫移除与 PR 标题所述的确定性分支增加目的无关,且与代码库中所有其他 Dimname 重载的守卫策略不一致,应予以恢复。

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

⛔ 需要修改

likedislike
Incorrect?
ascend-robotascend-robotCollaborators
Jul 7 Added label: ascend-cla/yes
83 messages are collapsed here。 See more
ascend-robotascend-robotCollaborators
Jul 8 Added label: approved
culechan
culechanCollaborators
Jul 9 Comments:

/lgtm

likedislike
ascend-robotascend-robotCollaborators
Jul 9 Added label: lgtm
ascend-robotascend-robotCollaborators
Jul 9 Merged pull request
ascend-robot
ascend-robotCollaborators
Jul 9 Comments:

Pull Request 已合并或已关闭。

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

likedislike