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

Fix bugs in MKLDNN. #10979

Merged
merged 13 commits into from
May 25, 2018
Merged

Fix bugs in MKLDNN. #10979

merged 13 commits into from
May 25, 2018

Conversation

zheng-da
Copy link
Contributor

@zheng-da zheng-da commented May 17, 2018

Description

This PR is to fix a few corner cases in NDArray:

  • Change the status of sliced NDArrays from a reused NDArray correctly.
  • When an NDArray is reused, the chunk only contains the shape information, but the original data type is gone. When an NDArray is generated with a slice from the NDArray, we lose both the shape and data type information of the original chunk. We need to handle this case correctly.
  • Extend CopyFrom to handle all different cases.
  • Create an MKLDNN memory from a view of an NDArray for writing new data.
  • Fix fallback for the inputs that are views and use MKLDNN format.
  • Handle input NDArrays that are views and use MKLDNN format correctly in Sum.
  • Handle output NDArrays that are views correctly in Sum.
  • Handle 5D NDArrays with MKLDNN format in Elemwise Add correctly.

I also adds more types of NDArrays to cover the cases described above.

Checklist

Essentials

Please feel free to remove inapplicable items for your PR.

  • The PR title starts with [MXNET-$JIRA_ID], where $JIRA_ID refers to the relevant JIRA issue created (except PRs with tiny changes)
  • Changes are complete (i.e. I finished coding on this PR)
  • All changes have test coverage:
  • Unit tests are added for small changes to verify correctness (e.g. adding a new operator)
  • Nightly tests are added for complicated/long-running ones (e.g. changing distributed kvstore)
  • Build tests will be added for build configuration changes (e.g. adding a new build option with NCCL)
  • Code is well-documented:
  • For user-facing API changes, API doc string has been updated.
  • For new C++ functions in header files, their functionalities and arguments are documented.
  • For new examples, README.md is added to explain the what the example does, the source of the dataset, expected performance on test set and reference to the original paper if applicable
  • Check the API doc at http://mxnet-ci-doc.s3-accelerate.dualstack.amazonaws.com/PR-$PR_ID/$BUILD_ID/index.html
  • To the my best knowledge, examples are either not affected by this change, or have been fixed to be compatible with this change

@pengzhao-intel
Copy link
Contributor

pengzhao-intel commented May 17, 2018

@zheng-da Could you add more descriptions (or change title) in order to make the PR readable?

please try the case a3c, const mkldnn::memory *NDArray::GetMKLDNNData will return nullptr since two mem format is not equal.
Do you think we need to fix this issue in this PR too?
https://github.com/apache/incubator-mxnet/tree/master/example/reinforcement-learning/a3c

@ThomasDelteil
Copy link
Contributor

👍 for the descriptive title and PR description

@zheng-da
Copy link
Contributor Author

@pengzhao-intel what error do you see. const mkldnn::memory *NDArray::GetMKLDNNData is allowed to return nullptr. Unless you mean here: https://github.com/apache/incubator-mxnet/blob/master/src/operator/nn/mkldnn/mkldnn_base.cc#L131
This place might be problematic.

@pengzhao-intel
Copy link
Contributor

@zheng-da Exactly. The exception is thrown in here.
We have a local fix for GetMKLDNNData to get the target data format and we will submit the PR soon.

@piiswrong piiswrong merged commit d497b37 into apache:master May 25, 2018
jinhuang415 pushed a commit to jinhuang415/incubator-mxnet that referenced this pull request May 29, 2018
* Fix bugs in MKLDNN.

* add more test cases.

* Fix CopyFrom when it's the view of an NDArray.

* add test.

* check same shape correctly.

* add unit test for CopyFrom.

* Fix warning.

* Add test sum.

* fix sum.

* Fix fallback.

* Fix fallback of sum.

* add tests.

* Update mkldnn.cc
rahul003 pushed a commit to rahul003/mxnet that referenced this pull request Jun 4, 2018
* Fix bugs in MKLDNN.

* add more test cases.

* Fix CopyFrom when it's the view of an NDArray.

* add test.

* check same shape correctly.

* add unit test for CopyFrom.

* Fix warning.

* Add test sum.

* fix sum.

* Fix fallback.

* Fix fallback of sum.

* add tests.

* Update mkldnn.cc
zheng-da added a commit to zheng-da/incubator-mxnet that referenced this pull request Jun 8, 2018
* Fix bugs in MKLDNN.

* add more test cases.

* Fix CopyFrom when it's the view of an NDArray.

* add test.

* check same shape correctly.

* add unit test for CopyFrom.

* Fix warning.

* Add test sum.

* fix sum.

* Fix fallback.

* Fix fallback of sum.

* add tests.

* Update mkldnn.cc
zheng-da added a commit to zheng-da/incubator-mxnet that referenced this pull request Jun 11, 2018
* Fix bugs in MKLDNN.

* add more test cases.

* Fix CopyFrom when it's the view of an NDArray.

* add test.

* check same shape correctly.

* add unit test for CopyFrom.

* Fix warning.

* Add test sum.

* fix sum.

* Fix fallback.

* Fix fallback of sum.

* add tests.

* Update mkldnn.cc
zheng-da added a commit to zheng-da/incubator-mxnet that referenced this pull request Jun 13, 2018
* Fix bugs in MKLDNN.

* add more test cases.

* Fix CopyFrom when it's the view of an NDArray.

* add test.

* check same shape correctly.

* add unit test for CopyFrom.

* Fix warning.

* Add test sum.

* fix sum.

* Fix fallback.

* Fix fallback of sum.

* add tests.

* Update mkldnn.cc
anirudh2290 pushed a commit that referenced this pull request Jun 13, 2018
* Fix bugs in MKLDNN.

* add more test cases.

* Fix CopyFrom when it's the view of an NDArray.

* add test.

* check same shape correctly.

* add unit test for CopyFrom.

* Fix warning.

* Add test sum.

* fix sum.

* Fix fallback.

* Fix fallback of sum.

* add tests.

* Update mkldnn.cc
zheng-da added a commit to zheng-da/incubator-mxnet that referenced this pull request Jun 28, 2018
* Fix bugs in MKLDNN.

* add more test cases.

* Fix CopyFrom when it's the view of an NDArray.

* add test.

* check same shape correctly.

* add unit test for CopyFrom.

* Fix warning.

* Add test sum.

* fix sum.

* Fix fallback.

* Fix fallback of sum.

* add tests.

* Update mkldnn.cc
@zheng-da zheng-da deleted the fix_mkldnn branch July 5, 2018 06:20
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants