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

[NewIR]Call _C_ops.xx in both dygraph and static mode #56809

Merged
merged 39 commits into from
Sep 4, 2023

Conversation

0x45f
Copy link
Contributor

@0x45f 0x45f commented Aug 30, 2023

PR types

New features

PR changes

Others

Description

本PR完成以下的工作:

  • 在动态图API的生成文件中添加了代码,生成了eager_op_function.h文件,之前是只有.cc文件。并且eager_op_function中的函数修改为了非static函数,以便在ops_api.cc中进行调用
  • 修改了动静态图切换时对于c++端tracer的处理,之前切换为静态图时c++端tracer并不是none,但是python端的tracer会变为none。在这个PR中进行了统一,切换为静态图时python和c++端的tracer都会修改为None,这样就可以在ops_api.cc中通过c++端的tracer来判断动静态图。同时将c++端的tracer修改为了thread local变量,避免多线程下切换动静态图可能导致的错误。
  • 在python端新增了两个判断mode的函数,分别是in_new_ir_mode和in_dynamic_or_new_ir_mode
  • 修改ops_api.cc的生成逻辑,下沉动静态图的选择逻辑
    • 筛选了一下static_op_function.cc和eager_op_function.cc两个文件,api个数如下所示:

      static api eager api both static only eager only
      893 543 483 410 60
    • 在ops_api.cc中,对于动静态图都有的api,生成了两个分支,在c++端进行动静态图的分发;对于只有静态图有的api(大部分是grad和xpu相关的api),在ops_api.cc中只生成了静态图的分支。在ops_api.cc中将所有生成的api都bind到了core.ir.ops这个module上

      // 统一的api
      static PyObject *mean(PyObject *self, PyObject *args, PyObject *kwargs) {
        if (egr::Controller::Instance().GetCurrentTracer() == nullptr) {
          VLOG(6) << "Call static_api_mean";
          return static_api_mean(self, args, kwargs);
        } else {
          VLOG(6) << "Call eager_api_mean";
          return eager_api_mean(self, args, kwargs);
        }
      }
      
      // 只有静态图分支的api
      static PyObject *rnn_(PyObject *self, PyObject *args, PyObject *kwargs) {
        VLOG(6) << "Call static_api_rnn_";
        return static_api_rnn_(self, args, kwargs);
      }
      
    • 在python端,_ir_ops.xx()可以调用到所有ops_api.cc中的api;而对于_C_ops.xx()在本pr中只将mean这一个动静统一的api加到了_C_ops中,_C_ops中其他的api保持原样,后续会逐渐在白名单中添加api就可以进行覆盖

  • 修改了mean的python api,删除了新ir的分支,统一使用_C_ops.mean()

Pcard-67164

0x45f added 24 commits August 22, 2023 03:20
@paddle-bot
Copy link

paddle-bot bot commented Aug 30, 2023

你的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.

@0x45f 0x45f changed the title [NewIR]Call _C_ops.xx in dy and static mode [NewIR]Call _C_ops.xx in both dy and static mode Aug 30, 2023
@0x45f 0x45f changed the title [NewIR]Call _C_ops.xx in both dy and static mode [NewIR]Call _C_ops.xx in both dygraph and static mode Sep 1, 2023
@Aurelius84 Aurelius84 requested a review from phlrain September 1, 2023 07:22
Aurelius84
Aurelius84 previously approved these changes Sep 1, 2023
Copy link
Contributor

@Aurelius84 Aurelius84 left a comment

Choose a reason for hiding this comment

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

LGTM

YuanRisheng
YuanRisheng previously approved these changes Sep 1, 2023
@@ -209,6 +212,14 @@ def in_dygraph_mode():
return global_var._dygraph_tracer_ is not None


def in_new_ir_mode():
return ir.core._use_new_ir_api() and not in_dygraph_mode()
Copy link
Contributor

Choose a reason for hiding this comment

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

既然这里已经有了in_new_ir_mode,后续可以删掉use_new_ir_api,统一写法

Copy link
Contributor Author

Choose a reason for hiding this comment

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

好的后续可以统一一下,感谢

wanghuancoder
wanghuancoder previously approved these changes Sep 1, 2023
Copy link
Contributor

@wanghuancoder wanghuancoder left a comment

Choose a reason for hiding this comment

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

LGTM

@0x45f 0x45f dismissed stale reviews from wanghuancoder, YuanRisheng, and Aurelius84 via 98cdc85 September 1, 2023 09:33
Copy link
Contributor

@jzhang533 jzhang533 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

@XiaoguangHu01 XiaoguangHu01 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

@zoooo0820 zoooo0820 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 change in python/paddle/fluid

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.

9 participants