Skip to content
This repository has been archived by the owner on Nov 17, 2023. It is now read-only.

fail to fall back when sparse arrays are passed to MKLDNN-enabled operators. #11448

Closed
3 of 12 tasks
zheng-da opened this issue Jun 28, 2018 · 15 comments
Closed
3 of 12 tasks

Comments

@zheng-da
Copy link
Contributor

zheng-da commented Jun 28, 2018

Currently, the MKLDNN-enabled operators, such as convolution and pooling, can't handle sparse arrays correctly. The reason is that the storage inference of these operators doesn't return the right dispatch mode.
The MKLDNN-enabled operators include:

We may also need to test the operators below:

  • Concat
  • Copy
  • Sum
  • Flatten

@haojin2 @azai91 @pengzhao-intel @TaoLv

@pengzhao-intel
Copy link
Contributor

@zheng-da Could you help provide a test case?
Does cuDNN backend handle these cases?

@frankfliu
Copy link
Contributor

Thank you for submitting the issue! @sandeep-krishnamurthy requesting this be labeled.

@zheng-da
Copy link
Contributor Author

@eric-haibin-lin @haojin2 could you please provide a test case?

@eric-haibin-lin
Copy link
Member

There’s an existing test batch norm training test disabled due to flakyness

@eric-haibin-lin
Copy link
Member

concat is tested and fixed.

@haojin2
Copy link
Contributor

haojin2 commented Jun 28, 2018

I'll take FullyConnected if no one's working on it right now.

@zheng-da
Copy link
Contributor Author

i created a PR for batchnorm.

@pengzhao-intel
Copy link
Contributor

pengzhao-intel commented Jun 29, 2018

what else need to cover? @luobao-intel can help

@zheng-da
Copy link
Contributor Author

zheng-da commented Jul 2, 2018

@pengzhao-intel @zouluobao everything I listed needs to be covered. The fix for BatchNorm has been merged and @haojin2 is working on FullyConnected. We need to fix other operators.

@pengzhao-intel
Copy link
Contributor

@luobao-intel will handle other OPs :)

@zheng-da
Copy link
Contributor Author

zheng-da commented Jul 3, 2018

@pengzhao-intel @luobao-intel we need to fix them and merge the PRs to the v1.3 release, which is probably before the end of next week.

@haojin2
Copy link
Contributor

haojin2 commented Jul 18, 2018

@zheng-da fullyconnected is done in #11498, if you want to add the MKLDNN backward storage type inference for it please go ahead and do so. Thanks!

@zheng-da
Copy link
Contributor Author

@haojin2 I thought you have fixed its backward storage type inference.

@pengzhao-intel
Copy link
Contributor

@zheng-da related PR are merged. Could you verify if all issues are fixed and close the issue?

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

No branches or pull requests

6 participants