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

[PHI] Migrate elementwise_(add/mul) kernels #48625

Merged
merged 7 commits into from
Dec 6, 2022

Conversation

Silv3S
Copy link
Member

@Silv3S Silv3S commented Dec 1, 2022

PR types

Others

PR changes

Others

Describe

  • migrate FWD elementwise kernels,
  • remove fluid elementwise code.

@paddle-bot
Copy link

paddle-bot bot commented Dec 1, 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.

@paddle-bot paddle-bot bot added contributor External developers status: proposed labels Dec 1, 2022
@Silv3S Silv3S added Intel and removed contributor External developers status: proposed labels Dec 1, 2022
@Silv3S
Copy link
Member Author

Silv3S commented Dec 1, 2022

There is problem with elementwise_add int kernel migrated to PHI, causing test_analyzer_ernie_int8 to drop accuracy and fail. test_analyzer_ernie_int8 has number of iterations set in tester_helper.h. If it's set to 1, same code is executed for both fluid and PHI version of this kernel. Unfortunately, if more iterations are executed, PHI kernel is producing incorrect values because attributes needed for calculations (Scale_x, Scale_y and Scale_out) are lost after first iteration.

That's because ClearDnnAttrs from #48364 is called. But without calling this method, problem is not fixed - last values which were set for each attribute in first iteration are saved and propagated to the end of this test.

Example for Scale_x (Scale_y and Scale_out have same behavior)

Iteration Scale_X (nr) Fluid PHI (with ClearDnnAttrs) PHI (without ClearDnnAttrs)
1 1 30.6755 30.6755 30.6755
1 2 31.5858 31.5858 31.5858
1 ... ... ... ...
1 25 8.46045 8.46045 8.46045
1 26 27.6632 27.6632 27.6632
2 1 30.6755 1.0 (default, no attr found) 27.6632
2 2 31.5858 1.0 (default, no attr found) 27.6632
2 ... ... ... ...
2 25 8.46045 1.0 (default, no attr found) 27.6632
2 26 27.6632 1.0 (default, no attr found) 27.6632

Could you please help solve this issue? @YuanRisheng, @chenwhql

@YuanRisheng
Copy link
Contributor

YuanRisheng commented Dec 1, 2022

@Silv3S You can try delete Scale_x, Scale_y and Scale_out as shown in the picture. This code is in op_compat.yaml. If that doesn't work, maybe my logic is wrong and I will recur this issue on my machine and debug it. Thank you.
image

@Silv3S
Copy link
Member Author

Silv3S commented Dec 1, 2022

@YuanRisheng thank you for reply. Removing these attributes in op_compat.yaml didn't change anything. Scales are still removed after first iteration.

@YuanRisheng
Copy link
Contributor

@YuanRisheng thank you for reply. Removing these attributes in op_compat.yaml didn't change anything. Scales are still removed after first iteration.

I have fixed this problem, you can try again. Thank you

@Silv3S Silv3S changed the title [PHI] Migrate elementwise_(add/sub/mul) kernels [PHI] Migrate elementwise_(add/mul) kernels Dec 5, 2022
@Silv3S
Copy link
Member Author

Silv3S commented Dec 5, 2022

@YuanRisheng thank you for solving this issue. I checked tests locally and everything works as expected now

@Silv3S
Copy link
Member Author

Silv3S commented Dec 5, 2022

@luotao1 could you please approve modifying unity_build_rule.cmake?

Copy link
Contributor

@luotao1 luotao1 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 unity_build_rule.cmake

@YuanRisheng YuanRisheng merged commit 7575d37 into PaddlePaddle:develop Dec 6, 2022
@Silv3S Silv3S deleted the phi_eltwise_kernels branch December 6, 2022 07:49
lxsbupt pushed a commit to lxsbupt/Paddle that referenced this pull request Dec 17, 2022
* remove fluid code

* init

* typo

* fix merge conflicts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contributor External developers Intel
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants