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

[BYOC-DNNL] add support for more ops and fusion patterns #9995

Merged
merged 1 commit into from
Feb 22, 2022

Conversation

crazydemo
Copy link

Enabled OP List:

  • Convolution 1D-3D
  • Deconvolution 2D-3D
  • Pooling 2D-3D (avg / max)
  • Elementwise OPs (abs, clip, exp, log, sqrt, round, logsumexp, relu, leaky_relu, tanh, sigmoid, softmax)

@tmoreau89
Copy link
Contributor

CC @apeskov and @mbs-octoml

@crazydemo
Copy link
Author

Hi, @apeskov and @mbs-octoml. Could you please take some time to review this PR? Look forward to your comments.

@mbs-octoml
Copy link
Contributor

@mikepapadim can you take a look at a this one, thanks.

@mikepapadim
Copy link
Contributor

@mikepapadim can you take a look at a this one, thanks.

I ll take a look!

@crazydemo
Copy link
Author

@mikepapadim can you take a look at a this one, thanks.

I ll take a look!

Hi @mikepapadim, do you have any comments?

@crazydemo
Copy link
Author

@tmoreau89 @mbs-octoml @mikepapadim @masahi Could you please help me review this PR?

@masahi
Copy link
Member

masahi commented Feb 16, 2022

I can merge this after you pass CI.

@crazydemo
Copy link
Author

I can merge this after you pass CI.
Thank you! I will check!

@crazydemo
Copy link
Author

Hi @masahi , I wonder which version of OneDNN is used for CI.
I have one build error, which says 'eltwise_logsigmoid' is not a member of 'dnnl::algorithm', and this algo is supported by OneDNNv1.8 or the later version.
Could you help me skip this check?

@masahi
Copy link
Member

masahi commented Feb 16, 2022

wget -q https://github.com/oneapi-src/oneDNN/releases/download/v1.5/dnnl_lnx_1.5.0_cpu_gomp.tgz

Probably too old, we should update that first.

@crazydemo
Copy link
Author

I will try to update to OneDNN V2.2.

@crazydemo crazydemo requested a review from leandron as a code owner February 16, 2022 06:03
@masahi
Copy link
Member

masahi commented Feb 16, 2022

Note that changing the docker script won't immediately update the CI environment (so the build will fail again).You need to send that change separately first like #10179. I can take care of actually updating the ci image.

@crazydemo
Copy link
Author

Note that changing the docker script won't immediately update the CI environment (so the build will fail again).You need to send that change separately first like #10179. I can take care of actually updating the ci image.

Do I need to submit an RFC, or just submit an independent PR?

@masahi
Copy link
Member

masahi commented Feb 16, 2022

No need for another rfc.

@vpirogov
Copy link

The latest oneDNN release is v2.5.2

@crazydemo
Copy link
Author

The latest oneDNN release is v2.5.2

Yes, the latest release version is v2.5.2, but only v2.2 has dnnl_lnx_2.2.0_cpu_gomp.tgz

@crazydemo
Copy link
Author

The PR, updating CI has been submitted #10266 @masahi

@vpirogov
Copy link

@crazydemo, right, we stopped publishing binary releases on Github after v2.2. Binaries for later versions are available on conda-forge.

@crazydemo
Copy link
Author

@vpirogov Thank you so much! I will update the CI to the latest version.

@crazydemo
Copy link
Author

hi @masahi , how can I restart the ci check?

@masahi
Copy link
Member

masahi commented Feb 18, 2022

So this is how it works.

CI update is an involved process. So please wait until at least tomorrow.

@crazydemo
Copy link
Author

Got it! Thank you for all your help and explanation!

@masahi
Copy link
Member

masahi commented Feb 18, 2022

@crazydemo it's ready now.

Copy link
Member

@masahi masahi left a comment

Choose a reason for hiding this comment

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

LGTM except one minor nit. Can be addressed in a later PR if more is coming.

@masahi
Copy link
Member

masahi commented Feb 21, 2022

@crazydemo please look at the CI issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants