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

[Doc] Add MKL-DNN operator list #14891

Merged
merged 33 commits into from
May 18, 2019
Merged

Conversation

TaoLv
Copy link
Member

@TaoLv TaoLv commented May 6, 2019

Description

Add MKL-DNN optimized operator list and graph fusion pattern list.

This PR is part of the plan in #14399

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

Changes

  • Feature1, tests, (and when applicable, API doc)
  • Feature2, tests, (and when applicable, API doc)

Comments

  • If this change is a backward incompatible change, why must this change be made.
  • Interesting edge cases to note here

@TaoLv TaoLv requested a review from szha as a code owner May 6, 2019 07:00
@TaoLv
Copy link
Member Author

TaoLv commented May 6, 2019

@pengzhao-intel please review~

Copy link
Contributor

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

Minor comments in line.

docs/tutorials/mkldnn/operator_list.md Outdated Show resolved Hide resolved
docs/tutorials/mkldnn/operator_list.md Outdated Show resolved Hide resolved
docs/tutorials/mkldnn/operator_list.md Outdated Show resolved Hide resolved
@TaoLv
Copy link
Member Author

TaoLv commented May 6, 2019

@xinyu-intel @ciyongch please help to review. Thank you.

docs/install/index.md Outdated Show resolved Hide resolved
Copy link
Contributor

@xinyu-intel xinyu-intel left a comment

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.

.

docs/tutorials/mkldnn/MKLDNN_README.md Outdated Show resolved Hide resolved
@anirudhacharya
Copy link
Member

@mxnet-label-bot add [pr-awaiting-review]

@marcoabreu marcoabreu added the pr-awaiting-review PR is waiting for code review label May 6, 2019
@TaoLv
Copy link
Member Author

TaoLv commented May 7, 2019

Copy link
Contributor

@aaronmarkham aaronmarkham left a comment

Choose a reason for hiding this comment

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

Please test the install index page on a preview. It tends to break a lot and very easily.

docs/faq/perf.md Outdated Show resolved Hide resolved
docs/faq/perf.md Outdated Show resolved Hide resolved
docs/tutorials/mkldnn/operator_list.md Show resolved Hide resolved
| FullyConnected (INT8) + re-quantization | |


To install MXNet MKL-DNN backend, please refer to [MKL-DNN backend readme](http://mxnet.incubator.apache.org/tutorials/mkldnn/MKLDNN_README.html)
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use relative links. You should be able to use MKLDNN_README.md. That being said, I'd move that readme content to the index page and link to index.md.

docs/tutorials/mkldnn/operator_list.md Outdated Show resolved Hide resolved
@@ -124,20 +124,38 @@ Indicate your preferred configuration. Then, follow the customized commands to i
$ pip install mxnet
```

MXNet offers pip packages with MKL-DNN enabled which will be much faster when running on Intel hardware. Try the following command line to install it and find performance numbers and tuning guide in <a href="https://mxnet.incubator.apache.org/versions/master/faq/perf.html#intel-cpu">performance on Intel CPU</a>.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
MXNet offers pip packages with MKL-DNN enabled which will be much faster when running on Intel hardware. Try the following command line to install it and find performance numbers and tuning guide in <a href="https://mxnet.incubator.apache.org/versions/master/faq/perf.html#intel-cpu">performance on Intel CPU</a>.
MKL-DNN enabled pip packages are optimized for Intel hardware. You can find performance numbers in the <a href="../..//faq/perf.md#intel-cpu">MXNet tuning guide</a>.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hi @aaronmarkham, the link here will be compiled to https://mxnet.incubator.apache.org/versions/faq/perf.md#intel-cpu which is a blank page. Do you think I can use http://mxnet.io/faq/perf.html#intel-cpu instead here?

Copy link
Member Author

Choose a reason for hiding this comment

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

docs/install/index.md Outdated Show resolved Hide resolved
docs/install/index.md Outdated Show resolved Hide resolved
docs/install/index.md Outdated Show resolved Hide resolved
docs/tutorials/mkldnn/MKLDNN_README.md Outdated Show resolved Hide resolved
@TaoLv
Copy link
Member Author

TaoLv commented May 14, 2019

@aaronmarkham @ciyongch @pengzhao-intel please take another look - I added some description about how the subgraph backend variable is used and added a reference link in the env var page.

Copy link
Contributor

@aaronmarkham aaronmarkham left a comment

Choose a reason for hiding this comment

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

Some minor grammar suggestions. And maybe one link issue...
If you're going back in there, I'd still rename mkldnn_readme.md to index.md and adjust any backlinks to it.

docs/faq/env_var.md Outdated Show resolved Hide resolved
docs/install/index.md Outdated Show resolved Hide resolved
docs/tutorials/mkldnn/MKLDNN_README.md Outdated Show resolved Hide resolved
TaoLv and others added 3 commits May 15, 2019 14:04
@TaoLv
Copy link
Member Author

TaoLv commented May 15, 2019

Thank you for the fix @aaronmarkham . I'm not familiar with the compilation from md files to html files, but I not notice there is already an index.md in the folder (and other folders under tutorials). Are you suggesting to overwrite the existing index.md?

@aaronmarkham
Copy link
Contributor

Thank you for the fix @aaronmarkham . I'm not familiar with the compilation from md files to html files, but I not notice there is already an index.md in the folder (and other folders under tutorials). Are you suggesting to overwrite the existing index.md?

Since others are working on that file right now in another PR, let's leave it alone for now.

@pengzhao-intel
Copy link
Contributor

@TaoLv I merged two MKLDNN related PRs just now and please check if the doc needs the related change.
I think we can merge this PR when all is finalized.

@TaoLv
Copy link
Member Author

TaoLv commented May 17, 2019

@TaoLv I merged two MKLDNN related PRs just now and please check if the doc needs the related change.
I think we can merge this PR when all is finalized.

Sure. Will change accordingly~

@TaoLv
Copy link
Member Author

TaoLv commented May 17, 2019

@pengzhao-intel Change the environmental variable name from MKLDNN_POST_QUANTIZE to MKLDNN_QUANTIZE accordingly. And also remove the ReLU constraint since we support more activation fusion now. Please review again~

@pengzhao-intel
Copy link
Contributor

It's great @TaoLv, please retrigger the CI and I will merge soon.

@pengzhao-intel
Copy link
Contributor

update env_var.md again :(

@pengzhao-intel pengzhao-intel merged commit e11a23d into apache:master May 18, 2019
haohuanw pushed a commit to haohuanw/incubator-mxnet that referenced this pull request Jun 23, 2019
* improve mkldnn document

* fix

* enable fusion

* adjust table

* fix comments

* promote mxnet-mkl package

* Update docs/tutorials/mkldnn/MKLDNN_README.md

Co-Authored-By: TaoLv <[email protected]>

* Update docs/install/index.md

Co-Authored-By: TaoLv <[email protected]>

* Update docs/install/index.md

Co-Authored-By: TaoLv <[email protected]>

* Update docs/install/index.md

Co-Authored-By: TaoLv <[email protected]>

* Update docs/install/index.md

Co-Authored-By: TaoLv <[email protected]>

* Update docs/tutorials/mkldnn/operator_list.md

Co-Authored-By: TaoLv <[email protected]>

* Update docs/faq/perf.md

Co-Authored-By: TaoLv <[email protected]>

* Update docs/faq/perf.md

Co-Authored-By: TaoLv <[email protected]>

* Update docs/tutorials/mkldnn/operator_list.md

Co-Authored-By: TaoLv <[email protected]>

* Update docs/tutorials/mkldnn/operator_list.md

Co-Authored-By: TaoLv <[email protected]>

* fix markdown table

* fix comments

* Update docs/faq/env_var.md

Co-Authored-By: Aaron Markham <[email protected]>

* Update docs/install/index.md

Co-Authored-By: Aaron Markham <[email protected]>

* Update docs/tutorials/mkldnn/MKLDNN_README.md

Co-Authored-By: Aaron Markham <[email protected]>

* change name of env variable

* retrigger ci

* Update env_var.md
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
pr-awaiting-review PR is waiting for code review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants