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] Slice #40587

Merged
merged 14 commits into from
Mar 23, 2022
Merged

[Eager] Slice #40587

merged 14 commits into from
Mar 23, 2022

Conversation

wanghuancoder
Copy link
Contributor

@wanghuancoder wanghuancoder commented Mar 15, 2022

PR types

New features

PR changes

Others

Describe

Eager 对齐老动态图 Slice

@paddle-bot-old
Copy link

Thanks for your contribution!
Please wait for the result of CI firstly. See Paddle CI Manual for details.

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.

some commets

const paddle::platform::Place& place,
paddle::framework::AttributeMap* default_attrs,
bool use_default_attr_map,
const std::map<std::string, std::string>& inplace_map) {
VLOG(6) << "Running On Eager TraceOp with use_default_attr_map: "
<< use_default_attr_map;
TraceOp<egr::EagerVariable>(type, ins, outs, std::move(attrs), place, false,
inplace_map, default_attrs, use_default_attr_map);
TraceOpImpl<egr::EagerVariable>(type, ins, outs, attrs, place, false,
Copy link
Contributor

Choose a reason for hiding this comment

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

why 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.

这个我当面给你讲过,TraceOp会修改attr,所以我需要在前向时,传入attr的引用,让TraceOp完成attr->check相关动作,再将新的attr返回给Eager前向,这样,Eager前向把正确的attr保存到反向使用。

Copy link
Contributor

Choose a reason for hiding this comment

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

ok

@@ -211,6 +228,21 @@ static PyObject* tensor_method__is_initialized(TensorObject* self,
EAGER_CATCH_AND_THROW_RETURN_NULL
}

static PyObject* tensor_method__dense_tensor_is_initialized(TensorObject* self,
Copy link
Contributor

Choose a reason for hiding this comment

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

why do we need this ?

Copy link
Contributor

Choose a reason for hiding this comment

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

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

print(tensor)时,需要看LoDTensor是否初始化。否则我们的打印和老动态图的打印格式无法对齐。

Copy link
Contributor

Choose a reason for hiding this comment

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

但是 Tensor自己的_initialized已经可以完成这个功能了把

@@ -602,6 +638,216 @@ static PyObject* tensor__getitem_index_not_tensor(TensorObject* self,
EAGER_CATCH_AND_THROW_RETURN_NULL
}

static PyObject* tensor_method__setitem_eager_tensor(TensorObject* self,
PyObject* args,
Copy link
Contributor

Choose a reason for hiding this comment

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

better don't copy code

JiabinYang
JiabinYang previously approved these changes Mar 18, 2022
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

zhiqiu
zhiqiu previously approved these changes Mar 21, 2022
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

@phlrain phlrain self-requested a review March 21, 2022 03:03
@wanghuancoder wanghuancoder dismissed stale reviews from zhiqiu and JiabinYang via e912905 March 21, 2022 06:22
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

@wanghuancoder wanghuancoder merged commit b07d239 into PaddlePaddle:develop Mar 23, 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.

3 participants