Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[OpAttr]Add SupportTensor for OpMaker with whitelist mechanism #45084

Merged
merged 6 commits into from
Aug 17, 2022

Conversation

Aurelius84
Copy link
Contributor

@Aurelius84 Aurelius84 commented Aug 11, 2022

PR types

Others

PR changes

Others

Describe

What's New?

1. 拆分attribute.h

将TypedChecker 相关逻辑从 attribute.h 中拆分独立到 attribute_checker.h中。考虑到后续checker逻辑可能会持续完善,依赖的头文件会更多,影响 attribute.h 作为最底层头文件的作用,故将其拆分,避免 attribute.h 依赖更多头文件。

2. 扩展OpProto.Attr

在OpProto.Attr 中添加了 support_tensor字段,用于标记一个Attribute是否具备「支持可变Attribute」的能力。

为什么要在 Proto 里标记?
答:因为飞桨的一些下游组件与主框架基本解耦的。比如Paddle2ONNX组件,其仅依赖于主框架中的 framework.proto。在Proto添加此标记,可以更便捷地与下游组件进行信息同步,也可避免对下游组件侵入式的代码修改。

3. TypedVarInfoChecker

添加了 TypedVarInfoChecker逻辑,主要用于可变Attribute的必要性检查,包括:

  • 对于 IntArray 的Attribute
    • 若传入的是vector<VarDesc*>,则每个 var_desc->shape 必须是 [1] 或者 [-1]
    • 若传入的是 VarDesc*,则var_desc.shape 的 rank 必须为 1
  • 对于 Scalar 的 Attribute
    • 则传入的 VarDesc* 的shape必须为 [1] 或者 [-1]

4. 其他

关于Coverage CI中提到此PR会导致coverage的build size增长3G的原因,目前怀疑是在framework.proto增加了字段,为底层逻辑修改,导致上层编译单元均依赖了此头文件(proto.pb.h)

@paddle-bot
Copy link

paddle-bot bot commented Aug 11, 2022

你的PR提交成功,感谢你对开源项目的贡献!
请关注后续CI自动化测试结果,详情请参考Paddle-CI手册
Your PR has been submitted. Thanks for your contribution!
Please wait for the result of CI firstly. See Paddle CI Manual for details.

@Aurelius84 Aurelius84 changed the title [OpAttr]Add SupportTensor for OpMaker [OpAttr]Add SupportTensor for OpMaker with whitelist mechanism Aug 12, 2022
0x45f
0x45f previously approved these changes Aug 12, 2022
Copy link
Contributor

@0x45f 0x45f left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Collaborator

@jiangjiajun jiangjiajun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@chenwhql chenwhql left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

paddle/fluid/framework/framework.proto Show resolved Hide resolved
Copy link
Contributor

@tianshuo78520a tianshuo78520a left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM for whl size

@Aurelius84 Aurelius84 merged commit 2594935 into PaddlePaddle:develop Aug 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants