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

【Hackathon 5th No.103】 fix the bug in moving fc_mkldnn to phi -part #59531

Merged
merged 9 commits into from
Dec 27, 2023

Conversation

zeroRains
Copy link
Contributor

PR types

Bug fixes

PR changes

Others

Description

fix the bug in this pr #58777

@luotao1 luotao1 changed the title 【Hackathon 5th No.103】 fix the but in moving fc_mkldnn to phi 【Hackathon 5th No.103】 fix the bug in moving fc_mkldnn to phi Dec 12, 2023
@luotao1 luotao1 changed the title 【Hackathon 5th No.103】 fix the bug in moving fc_mkldnn to phi 【Hackathon 5th No.103】 fix the bug in moving fc_mkldnn to phi -part Dec 22, 2023
extra :
[bool @ALL_KERNELS_MUST_COMPUTE_RUNTIME_SHAPE@ = true]
attrs : [bool use_mkldnn = false, bool use_quantizer = false, str mkldnn_data_type = "float32", float Scale_in = 1.0f, "float[] Scale_weights = {1.0f}", float Scale_out = 1.0f, bool force_fp32_output = false]
Copy link
Contributor

Choose a reason for hiding this comment

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

ALL_KERNELS_MUST_COMPUTE_RUNTIME_SHAPE需要加上

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@@ -100,7 +100,6 @@ const std::unordered_map<std::string, ExtraAttrPropertySet>
{"mkldnn_data_type", ExtraAttrProperty::ONEDNN},
{"scale_x", ExtraAttrProperty::ONEDNN},
{"scale_y", ExtraAttrProperty::ONEDNN},
{"scale_out", ExtraAttrProperty::ONEDNN},
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.

这个,在下面有一个Scale_out,所以删除了scale_out

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.

已补回

Copy link
Contributor

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

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

paddle/phi/kernels/fusion/gpu/fc_kernel.cu
paddle/phi/kernels/fusion/onednn/fc_kernel.cc
paddle/phi/kernels/fusion/cpu/fc_kernel.cc
FC有三个kernel,都需要同步修改一下~

@yuanlehome
Copy link
Contributor

paddle/phi/kernels/fusion/gpu/fc_kernel.cu paddle/phi/kernels/fusion/onednn/fc_kernel.cc paddle/phi/kernels/fusion/cpu/fc_kernel.cc FC有三个kernel,都需要同步修改一下~

cpu/gpu走的都是fc_kernel_impl.h,不需要改了哈~

@yuanlehome yuanlehome merged commit 5cbf32f into PaddlePaddle:develop Dec 27, 2023
29 checks passed
Wanglongzhi2001 pushed a commit to Wanglongzhi2001/Paddle that referenced this pull request Jan 7, 2024
…addlePaddle#59531)

* try to fix the bug in fc_mkldnn

* fix the missing attr bug

* fix the parameters bug

* remove the paramars in pir

* roback and add attr

* add the scale_out

---------

Co-authored-by: zeroRains <linjunlu@zerorains.com>
@zeroRains zeroRains deleted the 103_fc_fix branch January 30, 2024 12:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants