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.

Sign up for GitHub

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

Jump to bottom

[OneDNN] Enable oneDNN by default in Paddle Inference API #58560

Merged
merged 28 commits into from
Nov 16, 2023
Merged

[OneDNN] Enable oneDNN by default in Paddle Inference API #58560

merged 28 commits into from
Nov 16, 2023

Conversation

zhanglirong1999
Copy link
Contributor

PR types

Others

PR changes

Others

Description

We want to make OneDNN default on paddle.

@paddle-bot paddle-bot bot added the contributor External developers label Nov 1, 2023
@zhanglirong1999 zhanglirong1999 force-pushed the oneDNN_default_on branch 3 times, most recently from 9e0f6a2 to b6c5856 Compare November 7, 2023 02:17
@zhanglirong1999
Copy link
Contributor Author

fast_tokenizer op only support cpu. Like test_faster_tokenizer_op.py must manually disable mkldnn

@zhanglirong1999 zhanglirong1999 force-pushed the oneDNN_default_on branch 9 times, most recently from 1d50270 to a3190ad Compare November 10, 2023 10:22
@tianshuo78520a
Copy link
Contributor

这个PR会导致在PR-CI-Kunlun-R200 CI中 test_pad_op_xpu 单测一直timeout，单测hang住，显存无法正常释放，请排查下原因再提交代码。

@zhanglirong1999 zhanglirong1999 force-pushed the oneDNN_default_on branch 8 times, most recently from c67af90 to 7c3361c Compare November 14, 2023 02:02
@zhanglirong1999
Copy link
Contributor Author

zhanglirong1999 commented Nov 14, 2023 •
edited
Loading

Default OneDNN

  • Inference推理：默认在avx2开启EnableMKLDNN(), 可手动config->DisableMKLDNN()关闭. 对于开启了EnableXpu(), EnableCUDNN()的，会自动DisableMKLDNN

  • 静态图训练：暂时不支持默认开启OneDNN训练

    1. 需要更多测试oneDNN对训练的加速
    2. static mode需要手动用BuildStrategy对图进行优化，并不会自动执行OneDNN的所有pass，因此优化效果可能不显著
    3. 在flags.cc中对FLAGS_use_mkldnn = True的全局设置，此时还未对cpu info初始化，拿不到avx2的信息。如果后续需要support default on Training, 需要在executor中有更多的修改去支持

  • 对于CI中的单元测试影响

    1. 对继承于OpTest的op单侧无影响，op下use_mkldnn的attribute默认依旧为false, 测试mkldnn时才需要手动开启
    2. 推理和pass测试的少量单侧受到影响，如果后续需要用原生CPU进行测试，注意config是否disable了mkldnn

@xinyu-intel xinyu-intel changed the title [OneDNN] default on paddle [OneDNN] Enable oneDNN by default in Paddle Inference API Nov 14, 2023
@xinyu-intel xinyu-intel self-requested a review November 14, 2023 11:50
@xinyu-intel
Copy link
Contributor

@vivienfanghuagood @tianshuo78520a Please help on a review and check with the approval in PR-CI-Static-Check. Thanks:)

vivienfanghuagood
vivienfanghuagood approved these changes Nov 16, 2023
View reviewed changes
Copy link
Contributor

@vivienfanghuagood vivienfanghuagood left a comment

Choose a reason for hiding this comment

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

LGTM

Ligoml
Ligoml approved these changes Nov 16, 2023
View reviewed changes
Copy link
Contributor

@Ligoml Ligoml left a comment

Choose a reason for hiding this comment

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

LGTM

XiaoguangHu01
XiaoguangHu01 approved these changes Nov 16, 2023
View reviewed changes
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

xinyu-intel
xinyu-intel approved these changes Nov 16, 2023
View reviewed changes
Copy link
Contributor

@xinyu-intel xinyu-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

@xinyu-intel xinyu-intel merged commit 442151b into PaddlePaddle:develop Nov 16, 2023
28 checks passed
@EmmonsCurse EmmonsCurse mentioned this pull request Nov 22, 2023
Closed
SecretXV pushed a commit to SecretXV/Paddle that referenced this pull request Nov 28, 2023
@zhanglirong1999 @SecretXV
[OneDNN] Enable oneDNN by default in Paddle Inference API (PaddlePadd…
3a53059 
…le#58560)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@YangQun1 YangQun1 YangQun1 left review comments

@xinyu-intel xinyu-intel xinyu-intel approved these changes

@Ligoml Ligoml Ligoml approved these changes

@XiaoguangHu01 XiaoguangHu01 XiaoguangHu01 approved these changes

@vivienfanghuagood vivienfanghuagood vivienfanghuagood approved these changes

Assignees

@vivienfanghuagood vivienfanghuagood

Labels
contributor External developers Intel
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@zhanglirong1999 @tianshuo78520a @xinyu-intel @YangQun1 @Ligoml @XiaoguangHu01 @vivienfanghuagood