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

Add eager layout autotune #45409

Merged
merged 13 commits into from
Sep 5, 2022
Merged

Conversation

AnnaTrainingG
Copy link
Contributor

@AnnaTrainingG AnnaTrainingG commented Aug 25, 2022

PR types

Others

PR changes

OPs

Describe

Add eager layout autotune
本PR 修改了Conv.py中实现,不涉及功能修改及接口变动
新动态图单测测试结果:
image

老动态图测试结果
image

自动代码生成效果 dygraph_functions.cc:
image
自动代码生成效果 eager_final_state_op_function_impl.h:
image

@paddle-bot
Copy link

paddle-bot bot commented Aug 25, 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.

@AnnaTrainingG AnnaTrainingG force-pushed the DataLayout branch 3 times, most recently from c71bffc to f6774d7 Compare August 31, 2022 02:55
void SetOutTensorLayout(
std::vector<paddle::experimental::Tensor*>* out_tensor) {
for (size_t i = 0; i < out_tensor->size(); i++) {
VLOG(4) << "out layout is"
Copy link
Contributor

Choose a reason for hiding this comment

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

这里打印的log会不会有点多?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

当前版本为基础版本,为便于功能调试添加的打印,后续会统一删除

LAYOUT_LOGIC_TEMPLATE=\
"""
if (paddle::imperative::LayoutAutoTune::Instance().UseLayoutAutoTune()) {{
VLOG(5) << "Check and Prepare For LAYOUT";
Copy link
Contributor

Choose a reason for hiding this comment

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

这里可以把API名带上,LOG读起来会更直观些

Copy link
Contributor Author

Choose a reason for hiding this comment

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

好的,当前版本为基础版本,为便于功能调试添加的打印,后续会统一调整

layout_autotune_attr) == 0:
layout_logic_str = ""
else:
# after_call_str = f"return {forward_function_name}({layout_inputs_call_args_str});\n"
Copy link
Contributor

Choose a reason for hiding this comment

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

注释的代码可以去掉

Copy link
Contributor Author

Choose a reason for hiding this comment

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

好的

Comment on lines +24 to +29
// For agnostic op like add / relu
inline std::shared_ptr<EagerLayoutTransformer> EagerLayoutAutotune(
const std::string& op_name,
const paddle::small_vector<std::vector<paddle::experimental::Tensor>,
kSlotSmallVectorSize>& tensors_vector) {
VLOG(3) << " Optimze Layout agnostic op: " << op_name;
Copy link
Contributor

Choose a reason for hiding this comment

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

agnostic op 是指?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

与layout类型无关的OP 如exp relu等


protected:
std::string op_name_;
std::string final_layout_;
Copy link
Contributor

Choose a reason for hiding this comment

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

这里final_layout_为什么使用string而不是DataLayout类型?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

当前为基础版本,为便于打印,后续将统一修改为DataLayout

std::unordered_set<std::string> heavily_input_{"x", "y", "input"};
};

class EagerConcatOpTransformer
Copy link
Contributor

Choose a reason for hiding this comment

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

为什么这些Op的Transformer需要手写实现?这个开发成本似乎比较高

Copy link
Contributor Author

@AnnaTrainingG AnnaTrainingG Sep 1, 2022

Choose a reason for hiding this comment

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

该类OP可通过特定转换完成功能支持,无需添加Transpose, 后续同类OP较多时可复用

Comment on lines +76 to +78
DataLayout desired_layout_{DataLayout::UNDEFINED};

DataLayout default_layout_{DataLayout::UNDEFINED};
Copy link
Contributor

Choose a reason for hiding this comment

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

可能需要加注释解释下desired_layout_default_layout_的功能含义

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统一添加

if (var != nullptr && (paddle::imperative::GetDataLayout(var) ==
LayoutAutoTune::Instance().GetDesiredLayout())) {
in_layout = LayoutAutoTune::Instance().GetDesiredLayout();
break;
}
}
}
SetVarsLayout(outs, in_layout);
VLOG(3) << "Optimze Layout agnostic op: " << type_ << " "
<< paddle::framework::DataLayoutToString(in_layout);
Copy link
Contributor

Choose a reason for hiding this comment

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

DataLayout重载了<<,应该可以直接打印,不用再加 DataLayoutToString

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统一调整打印

@@ -52,12 +52,16 @@ inline bool NeedTransformPlace(const paddle::platform::Place& input,
return ret;
}

inline bool NeedTransformLayout(const DataLayout& input,
inline bool NeedTransformLayout(const paddle::platform::Place& place,
Copy link
Contributor

Choose a reason for hiding this comment

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

phi下面需要使用phi::Place

place参数建议放在inputtarget之后

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 修改

Copy link
Contributor

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

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

some comments


LAYOUT_LOGIC_TEMPLATE=\
"""
if (paddle::imperative::LayoutAutoTune::Instance().UseLayoutAutoTune()) {{
Copy link
Contributor

Choose a reason for hiding this comment

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

How about merge this into egr::Controller, too many singleton instance looks weird..

Copy link
Contributor Author

@AnnaTrainingG AnnaTrainingG Sep 5, 2022

Choose a reason for hiding this comment

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

此处是为统计新动态图与老动态的的判断条件

Copy link
Contributor

Choose a reason for hiding this comment

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

Controller里是有一个Tracer的我理解是不是可以共用

@@ -992,6 +1008,9 @@ def GenerateForwardDefinitionAndDeclaration(self, is_inplaced):
amp_tensors_vector_optional_list = []
amp_autocast_list = []
amp_autocast_optional_list = []
layout_autotune_list = []
Copy link
Contributor

Choose a reason for hiding this comment

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

Add some comment block to mark autotune code gen, or seal this into a function

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统一修改

Copy link
Contributor

Choose a reason for hiding this comment

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

o k

@@ -1023,6 +1048,9 @@ def GenerateForwardDefinitionAndDeclaration(self, is_inplaced):
amp_autocast_list.append(
f"auto NEW_{name} = egr::EagerAmpAutoCast(\"{name}\", {name}, amp_dst_dtype, op_name);\n"
)
layout_autotune_list.append(
Copy link
Contributor

Choose a reason for hiding this comment

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

Bad name, tmp var name should only have lower case letters.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

这个是为了复用amp的参数列表,为避免修改他人代码,直接使用了NEW_,
当前PR为基础版本后续将提PR统计修改完善

@@ -1037,6 +1065,9 @@ def GenerateForwardDefinitionAndDeclaration(self, is_inplaced):
amp_autocast_optional_list.append(
f"auto NEW_{name} = egr::EagerAmpAutoCasts(\"{name}\", {name}, amp_dst_dtype, op_name);\n"
)
layout_autotune_optional_list.append(
Copy link
Contributor

Choose a reason for hiding this comment

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

same

Copy link
Contributor Author

Choose a reason for hiding this comment

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

这个是为了复用amp的参数列表,为避免修改他人代码,直接使用了NEW_,
当前PR为基础版本后续将提PR统计修改完善

@@ -1047,10 +1078,59 @@ def GenerateForwardDefinitionAndDeclaration(self, is_inplaced):
amp_autocast_list.append(
f"auto NEW_{name} = egr::EagerAmpAutoCasts(\"{name}\", {name}, amp_dst_dtype, op_name);\n"
)
layout_autotune_list.append(
f"auto NEW_{name} = transformer->TransInTensor(\"{name}\", {name});\n"
Copy link
Contributor

Choose a reason for hiding this comment

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

same

Copy link
Contributor Author

Choose a reason for hiding this comment

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

这个是为了复用amp的参数列表,为避免修改他人代码,直接使用了NEW_,
当前PR为基础版本后续将提PR统计修改完善


inputs_args_definition_list[pos] = arg_str
inputs_args_declaration_list[pos] = arg_str

# for layout autotune attr
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any possible to make this a independent code block, as we've already had too many functional code here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

目前没有其他方案

Copy link
Contributor

Choose a reason for hiding this comment

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

我的意思把这个部分的代码统一整合到一个方法中,因为目前已经有很多字功能代码在code_gen里了我们后期也需要整理下

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 for now, but plz fix mentioned problem in later PR ASAP

@AnnaTrainingG AnnaTrainingG merged commit d7d9807 into PaddlePaddle:develop Sep 5, 2022
lanxianghit pushed a commit that referenced this pull request Sep 20, 2022
cherry-pick from #45826
LayoutAutotune 支持 inplace 类型的OP
 根据 Add eager layout autotune #45409 修改意见调整UseAutotune
将LayoutAutotune判断放到controller中,与AMP 判断保持一致
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