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.104】move fusion_repeated_fc_relu/fusion_squared_mat_sub to phi #58300

Merged
merged 16 commits into from
Nov 10, 2023

Conversation

zeroRains
Copy link
Contributor

@zeroRains zeroRains commented Oct 21, 2023

PR types

Others

PR changes

Others

Description

move fusion_repeated_fc_relu/fusion_squared_mat_sub to phi
#57262

@paddle-bot
Copy link

paddle-bot bot commented Oct 21, 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.

@paddle-bot paddle-bot bot added the contributor External developers label Oct 21, 2023
@zeroRains
Copy link
Contributor Author

scaled_dp_attention_functor.h的L330-L333

请问,上面的链接对应的代码部分会因为softmax_sum_maxupdate_out_blk是定义在#if defined(__AVX512F__)中的函数,导致编译失败,这种应该怎么处理

@yuanlehome
Copy link
Contributor

这个是当时在fluid下添加这个算子的PR #54139 ,看起来由于这个条件满足才会去编这个算子
image
在phi下可能也需要类似的控制一下,具体怎么做我先了解一下,晚点回复你~

@yuanlehome
Copy link
Contributor

yuanlehome commented Oct 26, 2023

scaled_dp_attention_functor.h的L330-L333

请问,上面的链接对应的代码部分会因为softmax_sum_maxupdate_out_blk是定义在#if defined(__AVX512F__)中的函数,导致编译失败,这种应该怎么处理

这样写试试
image

if(WITH_AVX
   AND AVX512F_FOUND
   AND AVX512F_FLAG
   AND WITH_MKL)
  # nothing
else()
  list(REMOVE_ITEM kernel_cc "cpu/self_dp_attention_kernel.cc")
endif()

@zeroRains
Copy link
Contributor Author

zeroRains commented Oct 26, 2023

scaled_dp_attention_functor.h的L330-L333
请问,上面的链接对应的代码部分会因为softmax_sum_maxupdate_out_blk是定义在#if defined(__AVX512F__)中的函数,导致编译失败,这种应该怎么处理

这样写试试 image

if(WITH_AVX
   AND AVX512F_FOUND
   AND AVX512F_FLAG
   AND WITH_MKL)
  # nothing
else()
  list(REMOVE_ITEM kernel_cc "cpu/self_dp_attention_kernel.cc")
endif()

出问题的部分

额,已添加(已执行make clean+cmake),但还是说was not declared in this scope
image

@yuanlehome
Copy link
Contributor

yuanlehome commented Oct 26, 2023

scaled_dp_attention_functor.h的L330-L333
请问,上面的链接对应的代码部分会因为softmax_sum_maxupdate_out_blk是定义在#if defined(__AVX512F__)中的函数,导致编译失败,这种应该怎么处理

这样写试试 image

if(WITH_AVX
   AND AVX512F_FOUND
   AND AVX512F_FLAG
   AND WITH_MKL)
  # nothing
else()
  list(REMOVE_ITEM kernel_cc "cpu/self_dp_attention_kernel.cc")
endif()

出问题的部分

额,已添加(已执行make clean+cmake),但还是说was not declared in this scope image

这样写~

if(WITH_AVX
   AND AVX512F_FOUND
   AND AVX512F_FLAG
   AND WITH_MKL)
  # nothing
else()
  list(REMOVE_ITEM kernel_cc "fusion/cpu/self_dp_attention_kernel.cc")
endif()

@zeroRains
Copy link
Contributor Author

scaled_dp_attention_functor.h的L330-L333
请问,上面的链接对应的代码部分会因为softmax_sum_maxupdate_out_blk是定义在#if defined(__AVX512F__)中的函数,导致编译失败,这种应该怎么处理

这样写试试 image

if(WITH_AVX
   AND AVX512F_FOUND
   AND AVX512F_FLAG
   AND WITH_MKL)
  # nothing
else()
  list(REMOVE_ITEM kernel_cc "cpu/self_dp_attention_kernel.cc")
endif()

出问题的部分
额,已添加(已执行make clean+cmake),但还是说was not declared in this scope image

这样写~

if(WITH_AVX
   AND AVX512F_FOUND
   AND AVX512F_FLAG
   AND WITH_MKL)
  # nothing
else()
  list(REMOVE_ITEM kernel_cc "fusion/cpu/self_dp_attention_kernel.cc")
endif()

也不行,(:з」∠)

@yuanlehome
Copy link
Contributor

在remove item前后打印一下 kernel_cc,看一下是否真的被移出去了

@zeroRains
Copy link
Contributor Author

remove item前后打印一下 kernel_cc,看一下是否真的被移出去了

他编译的时候走的是if分支,所以没有移出去

@yuanlehome
Copy link
Contributor

e6860733ac476401933c33a8209596f6
参考下这个,设置一下编译属性
image

@zeroRains
Copy link
Contributor Author

e6860733ac476401933c33a8209596f6 参考下这个,设置一下编译属性 image

image
。。。应该没写错吧,好像也不太行。。。

@yuanlehome
Copy link
Contributor

yuanlehome commented Oct 27, 2023

写在if分支里。。

if(WITH_AVX
   AND AVX512F_FOUND
   AND AVX512F_FLAG
   AND WITH_MKL)
  # nothing
else()
  list(REMOVE_ITEM kernel_cc "fusion/cpu/self_dp_attention_kernel.cc")
endif()

不过我估计也不行

@zeroRains
Copy link
Contributor Author

写在if分支里。。

if(WITH_AVX
   AND AVX512F_FOUND
   AND AVX512F_FLAG
   AND WITH_MKL)
  # nothing
else()
  list(REMOVE_ITEM kernel_cc "fusion/cpu/self_dp_attention_kernel.cc")
endif()

不过我估计也不行

确实不行

@paddle-ci-bot
Copy link

paddle-ci-bot bot commented Oct 29, 2023

Sorry to inform you that bfe2cae's CIs have passed for more than 7 days. To prevent PR conflicts, you need to re-run all CIs manually.

@zeroRains
Copy link
Contributor Author

zeroRains commented Oct 30, 2023

写在if分支里。。

if(WITH_AVX
   AND AVX512F_FOUND
   AND AVX512F_FLAG
   AND WITH_MKL)
  # nothing
else()
  list(REMOVE_ITEM kernel_cc "fusion/cpu/self_dp_attention_kernel.cc")
endif()

不过我估计也不行

确实不行

好像写在phi目录下的CMakeList中就可以编译通过了

kernels/fusion/cpu/self_dp_attention_kernel.cc
PROPERTIES COMPILE_FLAGS "-Wno-maybe-uninitialized -mfma ${AVX512F_FLAG}")
else()
list(REMOVE_ITEM kernel_cc "fusion/cpu/self_dp_attention_kernel.cc")
Copy link
Contributor

Choose a reason for hiding this comment

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

这一行需要写到paddle/phi/kernels目录下,不然"fusion/cpu/self_dp_attention_kernel.cc"路径不对

Copy link
Contributor Author

Choose a reason for hiding this comment

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

是否可以将路径改成“"kernels/fusion/cpu/self_dp_attention_kernel.cc"”

Copy link
Contributor

Choose a reason for hiding this comment

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

所以REMOVE_ITEM操作和set_source_files_properties分开写在不同的目录下

Copy link
Contributor Author

@zeroRains zeroRains Oct 30, 2023

Choose a reason for hiding this comment

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

EMMM,我的意思是在paddle/phiCMakeLists中改成这样,是否可行
image

AND AVX512F_FOUND
AND AVX512F_FLAG
AND WITH_MKL)
set_source_files_properties(
Copy link
Contributor

@yuanlehome yuanlehome Oct 30, 2023

Choose a reason for hiding this comment

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

kernels/funcs/scaled_dp_attention_functor.h文件就不需要set_source_files_properties了吧,.cc才是编译单元

Copy link
Contributor Author

Choose a reason for hiding this comment

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

好的我去除一下

set_target_properties(
self_dp_attention_op
PROPERTIES COMPILE_FLAGS "-Wno-maybe-uninitialized -mfma ${AVX512F_FLAG}")

Copy link
Contributor

Choose a reason for hiding this comment

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

这个if分支已经空了,为了不删掉?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

忘记了,我改一下

set_source_files_properties(
kernels/fusion/cpu/self_dp_attention_kernel.cc
PROPERTIES COMPILE_FLAGS "-Wno-maybe-uninitialized -mfma ${AVX512F_FLAG}")
else()
list(REMOVE_ITEM kernel_cc "fusion/cpu/self_dp_attention_kernel.cc")
list(REMOVE_ITEM kernel_cc "kernels/fusion/cpu/self_dp_attention_kernel.cc")
Copy link
Contributor

Choose a reason for hiding this comment

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

把这个else分支删掉,把if判断取反,把REMOVE_ITEM写在kernel/CMakeLists.txt里。kernel_cc不在当前这个文件里。。。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

哦哦,明白了

@zeroRains
Copy link
Contributor Author

你能把self_dp_attention的修改拆出来单独提交一个PR吗?另外两个op的修改还放在这个PR~

好吧

@zeroRains
Copy link
Contributor Author

zeroRains commented Nov 6, 2023

你能把self_dp_attention的修改拆出来单独提交一个PR吗?另外两个op的修改还放在这个PR~

这是self_dp_attention的新pr链接#58715
晚点我再把self_dp_attention的修改从这个pr里移除

@zeroRains zeroRains changed the title 【Hackathon 5th No.104】move self_dp_attention/fusion_repeated_fc_relu/fusion_squared_mat_sub to phi 【Hackathon 5th No.104】move fusion_repeated_fc_relu/fusion_squared_mat_sub to phi Nov 6, 2023
yuanlehome
yuanlehome previously approved these changes Nov 7, 2023
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

infer_meta :
func : FusionRepeatedFCReluInferMeta
kernel :
func : fusion_repeated_fc_relu
Copy link
Contributor

Choose a reason for hiding this comment

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

需要对齐原来的逻辑,指定kernel的data_typex

infer_meta :
func : FusionSquaredMatSubInferMeta
kernel :
func : fusion_squared_mat_sub
Copy link
Contributor

Choose a reason for hiding this comment

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

同上

Comment on lines +2262 to +2263
out->set_dims({i_dims[0], w_dims[sz - 1][1]});
out->share_lod(x);
Copy link
Contributor

Choose a reason for hiding this comment

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

dtype 也设置下

Comment on lines 2297 to 2304
squared_x->set_dims(x_dims);
squared_y->set_dims(y_dims);
squared_xy->set_dims({x_dims[0], y_dims[1]});
out->set_dims({x_dims[0], y_dims[1]});
Copy link
Contributor

Choose a reason for hiding this comment

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

设置 dtype

@CLAassistant
Copy link

CLAassistant commented Nov 7, 2023

CLA assistant check
All committers have signed the CLA.

rollback

add the type

Update fusion.cc
zyfncg
zyfncg previously approved these changes Nov 9, 2023
yuanlehome
yuanlehome previously approved these changes Nov 9, 2023
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

phlrain
phlrain previously approved these changes Nov 9, 2023
Copy link
Collaborator

@phlrain phlrain 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 check_dygraph

XiaoguangHu01
XiaoguangHu01 previously approved these changes Nov 9, 2023
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

sunzhongkai588
sunzhongkai588 previously approved these changes Nov 9, 2023
Copy link
Contributor

@sunzhongkai588 sunzhongkai588 left a comment

Choose a reason for hiding this comment

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

LGTM, no docs changes

@yuanlehome
Copy link
Contributor

这个冲突了,麻烦处理下再合入吧,另一个已经合入了

@luotao1 luotao1 merged commit 5fed7ad into PaddlePaddle:develop Nov 10, 2023
danleifeng pushed a commit to danleifeng/Paddle that referenced this pull request Nov 14, 2023
@zeroRains zeroRains deleted the 104 branch November 24, 2023 14:33
SecretXV pushed a commit to SecretXV/Paddle that referenced this pull request Nov 28, 2023
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.

8 participants