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

[Eager] generate eager core ops, only 4 ops #37813

Merged
merged 43 commits into from
Dec 8, 2021
Merged

[Eager] generate eager core ops, only 4 ops #37813

merged 43 commits into from
Dec 8, 2021

Conversation

wanghuancoder
Copy link
Contributor

@wanghuancoder wanghuancoder commented Dec 3, 2021

PR types

New features

PR changes

Others

Describe

本PR初步调通Eager模式下,4个OP向Python端暴露,具体提交内容包括:

  1. 动态图框架API接口自动代码生成模块的开发
  2. 暂时开放4个OP的自动代码生成,并与底层执行逻辑的自动代码生成借口联调成功
  3. 添加了一系列PyObject数据类型,向各种形式的EagerTensor转换的基础函数
  4. 改造_C_ops模块,使其能在老动态图ops和新动态图eager.ops之间切换,做到python API可无缝切换访问两种ops,并使用eager_guard()控制_C_ops的切换
  5. 修复了一个bug:用户调用OP API后获得的Tensor为空,原因是,动态图中间态期间,需要通过eagertensor.SyncToTensor()将Variable转化为EagerTensor

Copy link
Contributor

@JiabinYang JiabinYang left a comment

Choose a reason for hiding this comment

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

one comment and fix ci plz

@@ -81,13 +82,36 @@


@signature_safe_contextmanager
def eager_guard():
def eager_mode_place_guard(place):
Copy link
Contributor

Choose a reason for hiding this comment

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

it seems we don't need this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Controller中的Place变量是:std::shared_ptrpaddle::platform::Place expected_place_ = nullptr; 默认是nullptr,如果不先进行place配置(_set_expected_place),执行其它API会崩溃。

jim19930609
jim19930609 previously approved these changes Dec 7, 2021
// is not used in imperative mode, so we drop those inputs when generating OP
// functions. While, for very few OPs, the dispensable inputs are used, we
// need to manually specify them in this map.
std::map<std::string, std::set<std::string>> op_ins_map = {
Copy link
Contributor

Choose a reason for hiding this comment

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

In a former PR, I moved some of these maps (for now, only "op_ins_map" and "op_outs_map") to a header file at "paddle/fluid/pybind/op_function_generator.h", so different code generators can share the global maps.

Let's rearrange this part in a separate PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

我在本PR重新调整了这些数组,感谢!

JiabinYang
JiabinYang previously approved these changes Dec 7, 2021
Copy link
Contributor

@JiabinYang JiabinYang left a comment

Choose a reason for hiding this comment

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

LGTM

XiaoguangHu01
XiaoguangHu01 previously approved these changes Dec 7, 2021
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.

LG API

Copy link
Contributor

@zhiqiu zhiqiu 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 op_function_generator.cc

zhiqiu
zhiqiu previously approved these changes Dec 7, 2021
TCChenlong
TCChenlong previously approved these changes Dec 7, 2021
Copy link
Contributor

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

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

@JiabinYang JiabinYang 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.

LG API

Copy link
Contributor

@zhiqiu zhiqiu left a comment

Choose a reason for hiding this comment

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

LGTM

@wanghuancoder wanghuancoder merged commit 52f63cd into PaddlePaddle:develop Dec 8, 2021
Zjq9409 pushed a commit to Zjq9409/Paddle that referenced this pull request Dec 10, 2021
* refine a test case, test=develop

* publish python c api for eager, test=develop

* revert modify about test_allclose_layer.py, test=develop

* refine, test=develop

* refine, test=develop

* refine, test=develop

* refine, test=develop

* refine, test=develop

* refine, test=develop

* delete numpy includes, use pybind11 numpy.h, test=develop

* refine, test=develop

* refine, test=develop

* refine, test=develop

* suport eager error msg, and add grad test case, test=develop

* refine, test=develop

* refine, test=develop

* generate eager core ops, only 4 ops, test=develop

* refine, test=develop

* refine, test=develop

* refine, test=develop

* refine, test=develop

* refine, test=develop

* refine, test=develop

* refine, test=develop

* refine, test=develop

* refine, test=develop

* refine, test=develop

* refine, test=develop

* refine, test=develop

* refine, test=develop

* refine, test=develop
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.

8 participants