已合并
[feature] add torchnpugen packages #30026
梁松伟创建于 1月26日
[feature] add torchnpugen packages #30026
已合并
梁松伟创建于 1月26日
梁松伟
梁松伟成员
1月26日

What type of PR is this?

Uncomment only one /kind <> line, hit enter to put that in a new line, and remove leading whitespaces from that line:

/kind bug
/kind task
/kind feature

What does this PR do / why do we need it:

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewers:

likedislike
Pull Request已成功合入, 合并人@ascend-robot
(感谢 梁松伟 的贡献)
梁松伟梁松伟成员
1月26日 创建了 pull request,commit 90779483
ascend-robot
ascend-robot成员
1月26日 评论:

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

likedislike
ascend-robot
ascend-robot成员
1月26日 评论:

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

Module List Reviewers Committers
repo-Ascend/pytorch yanpengquan, zqwenn, shaoyf, huangyunlong2022, rain-666 yangkaixin, rain-666, wasd1111222, wangqiang160, zichun_ye
likedislike
梁松伟
梁松伟成员
1月26日 评论:

compile

likedislike
ascend-robotascend-robot成员
1月26日 添加了label:ascend-cla/yes
ascend-robot
ascend-robot成员
1月26日 评论:

CLA Signature Pass

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

likedislike
ascend-robotascend-robot成员
1月26日 添加了label:ci-pipeline-running
AtlasAccount
AtlasAccount成员
1月26日 评论:

问题/功能描述

本次PR主要进行了代码生成工具模块(codegen)的重构与重命名,将其统一更名为torchnpugen。此修改旨在解决模块命名冲突,提升代码结构的清晰度和一致性,确保构建脚本和代码生成脚本能够正确引用更新后的模块。此外,PR还扩展了代码生成器的功能,新增了对算子功能化(Functionalization)的支持,并修复了若干因模块路径变更导致的导入错误,从而保证整个构建流程和代码生成工具链的正常运行。

修改方案描述

修改方案涉及多个脚本和模块,核心内容包括:1)将项目中所有对codegen模块的导入路径和命令行调用统一更新为torchnpugen;2)在自动微分代码生成工具中,将相对路径导入重构为基于新包结构的绝对路径导入;3)新增功能化代码生成模块,为自定义算子自动生成功能化内核,使其能正确集成到PyTorch的功能化系统中;4)修复了特定函数中错误的模块导入路径。这些修改属于代码重构和功能扩展,不涉及核心业务逻辑的变动,确保了项目在重构后的组织结构和工具链功能完整性。

likedislike
AtlasAccount
AtlasAccount成员1月26日进行代码检视1
torchnpugen/__init__.py
@@ -7,1 +1,0 @@
7- 
8-def _write_if_changed_security(self, filename: str, contents: str) -> None:
AtlasAccount
AtlasAccount1月26日评论:

函数设计: _write_if_changed_security函数被定义为实例方法(带有self参数),但实际使用中通过monkey patch方式赋值给torchgen.gen.FileManager._write_if_changed。这意味着该函数将被作为实例方法调用,但函数内部并未使用self参数,这可能导致混淆。从上下文看,原始FileManager._write_if_changed可能是一个实例方法,但当前实现中self参数未被使用,这可能表明函数设计意图不清晰。

问题类型: 函数设计
文件路径: torchnpugen/__init__.py
行号: 8
问题代码:

def _write_if_changed_security(self, filename: str, contents: str) -> None:

修改建议:

如果函数确实不需要访问实例属性,建议移除self参数,将其定义为普通函数。或者,如果需要在函数内部访问FileManager实例的属性,则应在函数体中合理使用self参数。考虑到这是一个monkey patch,应该保持与原方法相同的签名,但需要明确说明self参数的使用意图。

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

likedislike
AtlasAccount
AtlasAccount成员1月26日进行代码检视1
torchnpugen/__init__.py
@@ -8,1 +1,0 @@
8-def _write_if_changed_security(self, filename: str, contents: str) -> None:
9- old_contents: Optional[str]
AtlasAccount
AtlasAccount1月26日评论:

变量和数据类型问题: 变量old_contents被声明为Optional[str]类型,但在try-except块中,当文件读取失败时(IOError)被赋值为None。然而,在Python 3中,建议使用更具体的异常类型如FileNotFoundError或PermissionError,而不是通用的IOError。此外,类型注解Optional[str]是合适的,但实际使用中需要考虑文件读取失败的其他情况。

问题类型: 变量和数据类型问题
文件路径: torchnpugen/__init__.py
行号: 9
问题代码:

    old_contents: Optional[str]
    filepath = os.path.realpath(filename)
    try:
        with open(filepath, 'r') as f:
            old_contents = f.read()
    except IOError:
        old_contents = None

修改建议:

建议将except IOError:改为except OSError:以捕获更广泛的操作系统相关错误,或者根据具体需求捕获FileNotFoundError、PermissionError等更具体的异常。同时,确保类型注解与异常处理逻辑一致。

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

likedislike
AtlasAccount
AtlasAccount成员1月26日进行代码检视1
torchnpugen/__init__.py
@@ -26,1 +1,0 @@
26- 
27-apply_codegen_patches()
AtlasAccount
AtlasAccount1月26日评论:

代码逻辑和结构: 模块导入时立即调用apply_codegen_patches()函数,这会导致在导入torchnpugen模块时自动应用补丁。这种隐式行为可能使代码的行为难以预测,特别是当模块被多次导入或在不同环境中使用时。此外,全局状态的修改应该在可控的条件下进行,而不是在导入时自动执行。

问题类型: 代码逻辑和结构
文件路径: torchnpugen/__init__.py
行号: 27
问题代码:

apply_codegen_patches()

修改建议:

建议将apply_codegen_patches()调用移至一个显式的初始化函数中,或者通过环境变量、配置选项来控制是否应用补丁。这样可以让用户更清楚地了解代码的行为,并避免不必要的副作用。例如,可以提供一个initialize()函数,用户需要显式调用它来应用补丁。

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

likedislike
AtlasAccount
AtlasAccount成员1月26日进行代码检视1
torchnpugen/__init__.py
@@ -16,1 +1,0 @@
16- if contents != old_contents:
17- PathManager.remove_path_safety(filepath)
AtlasAccount
AtlasAccount1月26日评论:

错误处理与异常管理: 在调用PathManager.remove_path_safety(filepath)时,没有处理可能发生的异常。如果文件删除失败(例如由于权限问题或文件被锁定),后续的创建和写入操作可能会失败或产生不可预期的结果。此外,整个函数缺乏对文件操作过程中可能出现的其他异常(如磁盘空间不足、权限问题等)的处理。

问题类型: 错误处理与异常管理
文件路径: torchnpugen/__init__.py
行号: 17
问题代码:

        PathManager.remove_path_safety(filepath)
        with os.fdopen(os.open(filepath, os.O_RDWR | os.O_CREAT, stat.S_IWUSR | stat.S_IRUSR), "w") as f:
            f.write(contents)

修改建议:

建议在关键文件操作周围添加适当的异常处理,例如捕获OSError或PermissionError,并在发生错误时提供有意义的错误信息或回滚操作。同时,确保PathManager.remove_path_safety的异常不会导致程序崩溃,或者至少记录错误日志。

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

likedislike
ascend-robotascend-robot成员
1月26日 删除了label:ci-pipeline-running
ascend-robotascend-robot成员
1月26日 添加了label:ci-pipeline-passed
ascend-robot
ascend-robot成员
1月26日 评论:
流水线 PR-pipeline_pytorch#4577 已完成
阶段 任务名 状态 详情
编译构建 Build_X86 >>>
Build_LibTorch >>>
Build_ARM >>>
Build_ARM_inductor 🛑 >>>
Build_X86_py311 🛑 >>>
Build_ARM_py311 🛑 >>>
dist_test_or_not >>>
恶意代码检查 Antipoison >>>
编码安全与规范检查 CodeCheck >>>
check_error >>>
开源片段检查 SCA >>>
开发者测试 UT_DIST_X86 🛑 >>>
UT_X86_Part_01 >>>
UT_X86_Part_02 >>>
UT_ARM_A2_Part_01 🛑 >>>
UT_ARM_A2_Part_02 🛑 >>>
UT_inductor_arm 🛑 >>>
流水线 PR-pipeline_pytorch >>>
likedislike
shaoyf成员
1月26日 评论:

/approve

likedislike
ascend-robotascend-robot成员
1月26日 添加了label:lgtm
王超成员
1月26日 评论:

/approve

likedislike
ascend-robotascend-robot成员
1月26日 添加了label:approved
ascend-robot
ascend-robot成员
1月26日 评论:

Review Guide

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

likedislike
ascend-robotascend-robot成员
1月26日 合入了pull request