已合并
fix: migrate simple onnx parse params plugins #9161
clinglai0517创建于 12 天前
fix: migrate simple onnx parse params plugins #9161
已合并
clinglai0517创建于 12 天前
clinglai0517成员
12 天前

描述

将一批 ONNX 插件的参数解析接口从 ParseParamsFn 迁移到 ParseParamsByOperatorFn,实现onnx插件和protobuf的解耦

涉及算子:

  • Mish
  • FastGelu
  • Size
  • Elu
  • LeakyRelu
  • HardMax

【实现说明】

  • 无属性或仅默认属性的算子,直接切换新注册接口。
  • 对 Elu、LeakyRelu、HardMax,从 ge::Operator 的 attribute JSON 中解析 ONNX 属性。
  • 保留旧逻辑中的默认值行为。
  • 对缺失或非数组形式的 attribute JSON 增加保护,避免无属性场景误返回 FAILED。

关联的Issue

Issue #5115

测试

onnx插件ut编译、执行通过

文档更新

不涉及

类型标签

AI/Agent生成声明

likedislike
Pull Request已成功合入, 合并人@CANN-robot
(感谢 clinglai0517 的贡献)
Cclinglai0517成员
12 天前 创建了 pull request,commit 1d1f01b2
atomgit-bot
atomgit-bot
12 天前 评论:

变更摘要

本 PR 将 Mish、FastGelu、Size、Elu、LeakyRelu、HardMax 六个 ONNX 插件的参数解析接口从 ParseParamsFn 迁移到 ParseParamsByOperatorFn,参数源从 ge::onnx::NodeProto(protobuf)切换为 ge::Operator,实现 ONNX 插件与 protobuf 的解耦。迁移过程中移除了对 NodeProto 的动态/静态转换及对应失败分支,对 Elu、LeakyRelu、HardMax 改为从 ge::Operator 的 attribute JSON 中解析 ONNX 属性,并保留了原默认值行为。

主要改动

  • 注册接口整体切换:六个插件(elu_onnx_plugin.cppleaky_relu_onnx_plugin.cpphardmax_onnx_plugin.cppmish_onnx_plugin.cppnpu_fast_gelu_onnx_plugin.cppsize_onnx_plugin.cpp)的注册由 .ParseParamsFn(...) 改为 .ParseParamsByOperatorFn(...),解析函数签名由 const Message* 改为 const ge::Operator&,并移除 NodeProto 转换及 nullptr 检查。
  • 基于 JSON 的属性解析:Elu、LeakyRelu、HardMax 改为通过 op_src.GetAttr("attribute", attrs_string) 获取属性 JSON,并用 nlohmann::json 解析后按 name 匹配读取 f(浮点)或 i(整型)字段,保留默认值(alpha=1.0、negative_slope=0.01f、axis=-1)。
  • 缺失/非数组属性保护:解析前增加 attrs.contains("attribute") && attrs["attribute"].is_array() 判断,避免无属性场景下误返回 FAILED;JSON 解析异常时记录日志并返回 FAILED
  • 无属性算子直接切换:Mish、FastGelu 的空参数实现直接返回 SUCCESS,Size 保持设置 dtypeDT_INT64,仅切换注册接口,无属性逻辑改动。
likedislike
atomgit-bot
atomgit-bot
12 天前 评论:

代码审查

Final sweep complete — I re-checked the exact lines of each finding (no secondary defects: field/index semantics for "name"/"f"/"i" are correct, and the HardMax "i" field is a JSON number per prior art, so no defect there), and confirmed the three no-op migrations (FastGelu/Mish/Size) preserved behavior with no leftover references to removed symbols.

关闭总结

审查发现(共 2 条,均为 P1)

文件 行号 优先级 问题
activation/elu/framework/elu_onnx_plugin.cpp 25-27 P1 对 JSON 字符串字段 "f" 调用 get<float>(),抛 type_error 后返回 FAILED,任何带 alpha 属性的 Elu 节点解析失败(回归)
activation/leaky_relu/framework/leaky_relu_onnx_plugin.cpp 25-27 P1 同上,LeakyRelu 的 alpha(negative_slope)解析失败

核心证据:本仓库框架的 attribute JSON 中,标量 float 以 字符串 存储(先例 group_normal_relu_onnx_plugin.cppbounding_box_decode_onnx_plugin.cpp 均以 std::string + StrToFloat 解析,后者明确注释 "float type in json has accuracy loss, so we use string type to store it"),而本次新增代码直接 attr["f"].get<float>(),对字符串会抛 nlohmann::json::type_error,被 catch (...) 捕获后返回 FAILED。修复方式见各条 finding 的 suggested_code。

各变更文件审查结论

  • activation/elu/framework/elu_onnx_plugin.cpp发现问题(P1,alpha 浮点解析)
  • activation/fast_gelu/framework/npu_fast_gelu_onnx_plugin.cpp — no issues(纯接口迁移,无属性解析,行为等价)
  • activation/leaky_relu/framework/leaky_relu_onnx_plugin.cpp发现问题(P1,alpha 浮点解析)
  • activation/mish/framework/mish_onnx_plugin.cpp — no issues(纯接口迁移,行为等价)
  • common/src/framework/hardmax_onnx_plugin.cpp — no issues(attr["i"].get<int>() 读取的是 JSON number 字段,与先例一致;默认值与旧逻辑一致)
  • common/src/framework/size_onnx_plugin.cpp — no issues(纯接口迁移,行为等价)

总体风险评估

本次 PR 将 6 个 ONNX 插件的参数解析从 ParseParamsFn 迁移到 ParseParamsByOperatorFn,解耦方向正确,无属性的 3 个算子(FastGelu/Mish/Size)迁移安全。但 Elu 与 LeakyRelu 两个算子对 float 属性字段的解析方式与本框架的 JSON 序列化约定(float 存为字符串)相悖,只要模型显式携带 alpha 属性即触发解析失败(FAILED),属于高概率的功能回归,建议合并前按先例改为字符串 + StrToFloat 解析并补充相应单测。

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

⛔ 需要修改

likedislike
CANN-robotCANN-robot成员
12 天前 添加了label:cann-cla/yes
CANN-robot
CANN-robot成员
12 天前 评论:

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
activation 王星, 汤平川, 刘波 (3/2) 王星, 汤平川 (2/1)
cmake 刘波, 汤平川 (2/2) 刘波 (1/1)
common 刘波, 汤平川 (2/2) 刘波 (1/1)
repo-cann/ops-nn 汤平川, 刘波 (2/2) 刘波, 汤平川 (2/1)
tests 刘波, 汤平川 (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

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

likedislike
此处折叠了105条消息 查看更多
CANN-robotCANN-robot成员
11 天前 添加了label:approved
TangPC
TangPC成员
11 天前 评论:

/lgtm
/approve

likedislike
CANN-robotCANN-robot成员
11 天前 添加了label:lgtm
CANN-robotCANN-robot成员
11 天前 合入了pull request
CANN-robot
CANN-robot成员
11 天前 评论:

Pull Request 已合并或已关闭。

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

likedislike