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

[Need approval] Add AdamW-CPU FP32 JIT assembly kernel #42522

Merged
merged 4 commits into from May 9, 2022

Conversation

wozna
Copy link
Contributor

@wozna wozna commented May 5, 2022

PR types

New features

PR changes

OPs

Describe

This PR adds AdamW JIT assembly kernel. This feature was requested by #41950

Before in AdamW, all params were computed with Eigen

param -= lr * lr_ratio_ * coeff_ * param;
and then Adam JIT assembly kernel was used.

In this PR AdamW JIT implementation, params are calculated with vector instructions in the same loop where params and momentums are calculated.

Results for VGG training script "test_image_classification.py"
100 batches of 128 images
Intel(R) Xeon(R) Gold 6248 CPU @ 2.50GHz

Average kernel time [ms]

Threads AdamW with Eigen AdamW JIT
1 0.0667 0.0655724
20 0.133347 0.118731

@paddle-bot-old
Copy link

paddle-bot-old bot commented May 5, 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.

@wozna wozna requested review from jakpiase and jczaja May 6, 2022 06:53
Copy link
Contributor

@jczaja jczaja left a comment

Choose a reason for hiding this comment

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

LGTM

@wozna
Copy link
Contributor Author

wozna commented May 6, 2022

@Aganlengzi In PR-CI-Coverage there is problem in all recent PRs
File "/paddle/tools/check_pr_approval.py", line 28, in check_approval if review["state"] == "APPROVED":
Can it be fixed somehow?

@chenwhql In the file phi/kernels/cpu/adamw_kernel.cc I used JIT kernel and approval is needed for that. Could you please check this?

@Aganlengzi
Copy link
Contributor

@Aganlengzi In PR-CI-Coverage there is problem in all recent PRs
File "/paddle/tools/check_pr_approval.py", line 28, in check_approval if review["state"] == "APPROVED":
Can it be fixed somehow?

hi, re-run will be ok.

Copy link
Contributor

@jakpiase jakpiase left a comment

Choose a reason for hiding this comment

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

LGTM

@lidanqing-intel lidanqing-intel self-requested a review May 9, 2022 05:29
Copy link
Contributor

@lidanqing-intel lidanqing-intel left a comment

Choose a reason for hiding this comment

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

LGTM

@lidanqing-intel
Copy link
Contributor

@Aganlengzi Could you please help with the PR-CI-APPROVAL ?

2022-05-06 05:41:32 2022-05-06 05:41:32 (3.19 MB/s) - 已保存 “bk.txt” [5/5])
2022-05-06 05:41:38 ****************
2022-05-06 05:41:39 0. You must have one RD (chenwhql, MingMingShangTian, YuanRisheng or zyfncg) approval for the including paddle/fluid header in paddle/phi files( paddle/phi/kernels/cpu/adamw_kernel.cc).

@lidanqing-intel lidanqing-intel changed the title Add AdamW-CPU FP32 JIT assembly kernel [Need approval] Add AdamW-CPU FP32 JIT assembly kernel May 9, 2022
Copy link
Contributor

@Aganlengzi Aganlengzi left a comment

Choose a reason for hiding this comment

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

LGTM

@Aganlengzi Aganlengzi merged commit 766c50a into PaddlePaddle:develop May 9, 2022
@paddle-bot-old paddle-bot-old bot removed the contributor External developers label Oct 17, 2022
@wozna wozna deleted the adam_jit_2 branch February 24, 2023 16:09
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.

None yet

6 participants