[MXNET-367] update mkldnn to v0.14 and disable building test examples #10736

ashokei · 2018-04-28T05:35:19Z

Description

updated mkldnn submodule to latest release version, and disabled unnecessary build steps for mkldnn-examples/test

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

ashokei · 2018-04-28T05:37:01Z

@TaoLv @zheng-da updated mkldnn submodule to latest release version, and disabled unnecessary build steps for mkldnn-examples/test. please review, let me know any feedback. thanks.

TaoLv · 2018-04-28T06:10:36Z

Looks good to me. But I guess there will be some issues need to fix. MKL-DNN added some new formats which are not handled in mxnet. Let's wait for ci finishing.

zheng-da · 2018-04-28T06:23:47Z

also looks good to me. We should always attach to the official release from now on. How often do you have a release?

TaoLv · 2018-04-28T06:55:09Z

Seems CI is still using b4137df:

Cloning into '3rdparty/mkldnn'...
Submodule path '3rdparty/mkldnn': checked out 'b4137dfc88e3bf5c6b62e833121802eb8c6696da'

===========
Sorry, I missed your point. You mean if user clones from mxnet repo, they will always get mkldnn code from the master branch with a specific commit id. But if he wants to change the mkldnn version, he can run this bash script to get v0.14, right? That sounds good. But v0.14 is not verified from mxnet framework level currently. So I would not like to recommend using v0.14.

ashokei · 2018-04-28T07:11:53Z

@TaoLv mxnet has been using latest master branch of mkldnn, which means it already has v0.14.

i believe v0.14 comes with several performance enhancements and features. We could go back to v0.13 , but since mxnet has been using master branch, i think we should just go with v0.14.

TaoLv · 2018-04-28T07:16:50Z

@ashokei No, we are not using v0.14. We are using commit b4137df, which was submitted to mkldnn repo at Mar 20.

ashokei · 2018-04-28T07:18:34Z

@TaoLv you are right, this is hardcoded somewhere, we can update to fixed release tag (may be v0.13) until v0.14 is verified with mxnet. I will look into it.

TaoLv · 2018-04-28T07:25:02Z

Yes. I agree that release always means better stability than latest commit in master branch. But I have the same concern with @zheng-da , what's the release period of mkl-dnn? If mkl-dnn has a hot-fix in their master branch, then what should we do? Seems mkldnn team would not release a new version for a hot-fix.

Regarding to other 3rdparty packages, they are pointed to the specific version in the same way of mkl-dnn, a specific commit id, not a release tag.

zheng-da · 2018-04-28T07:28:25Z

actually, why do we not move the mkldnn submodule to the certain commit associated with the release directly? i think it's better than checking the mkldnn release out explicitly in the script.

ashokei · 2018-04-28T07:34:27Z

yes, we could just update submodule to v0.14, i made the changes.

@TaoLv mkl-dnn releases are probably more thoroughly tested than just some random commit, we should at the minimum track latest mkldnn release (currently v0.14), and we could change to hot fix as needed (until v0.15 comes out).

TaoLv · 2018-04-28T07:42:57Z

OK. I agree to change to current commid id b4137df to v0.14's 0e7ca73. Since we always need submit PR and pass ci to update the commid id, I think there shouldn't have any functional issues by this approach. So we don't need wait for next release to resolve some bugs or adding some new features. Commit from master branch should be OK to be a dependency of mxnet, same as other 3rdparty dependencies.

.

marcoabreu · 2018-04-28T07:49:28Z

Could you update all locations of MKLDNN to 0.14? We have different scripts like cmake downloading mkl on the fly and we should check that they are all on-par

yajiedesign · 2018-04-29T02:03:10Z

mark don't forget windows when #10629 is merge

ashokei · 2018-05-02T18:19:22Z

@marcoabreu updated all locations, can you please merge if ok. thanks.

szha · 2018-05-03T05:22:05Z

prepare_mkl.sh

@@ -58,16 +58,16 @@ MXNET_ROOT=`dirname $0`
 USE_MKLML=0
 # NOTE: if you update the following line, please also update the dockerfile at
 # tests/ci_build/Dockerfile.mkl
-VERSION_MATCH=20171227
+VERSION_MATCH=20180406


Since this version is tied to the specific mkldnn commit, should this script live in mkldnn instead?

this script can be used independent of mkldnn (for eg: mshadow with mklml blas api)

I haven't seen such use before. Has this been added as an option to USE_BLAS flag?

USE_MKLML is used inside mshadow build. USE_BLAS can be set to mkl.

szha · 2018-05-03T05:23:55Z

ci/docker/install/ubuntu_mklml.sh

any way to automatically get this from 3rdparty/mkldnn? and this should really be managed by mkldnn because mxnet shouldn't need to directly depend on mklml.

i notice these scripts are called only for docker, as part of docker image build; i think we do not have 3rdparty/mkldnn folder yet , as the actual mxnet build happens later.

Well our dockerfiles even depend on Caffe, so that topic is already pretty over the top :D

@marcoabreu can you merge if ok. let me know if any other concern(s).

…examples (#10736)" This reverts commit 3c7afcc.

…examples (#10736)" (#10808) This reverts commit 3c7afcc.

…apache#10736) * update mkldnn to v0.14 and disable building test examples * update mkldnn to v0.14 release * use git submodule tagging for mkldnn * update mklml to latest version

…examples (apache#10736)" (apache#10808) This reverts commit 3c7afcc.

…apache#10736) * update mkldnn to v0.14 and disable building test examples * update mkldnn to v0.14 release * use git submodule tagging for mkldnn * update mklml to latest version

…examples (apache#10736)" (apache#10808) This reverts commit 3c7afcc.

…apache#10736) * update mkldnn to v0.14 and disable building test examples * update mkldnn to v0.14 release * use git submodule tagging for mkldnn * update mklml to latest version

…examples (apache#10736)" (apache#10808) This reverts commit 3c7afcc.

…apache#10736) * update mkldnn to v0.14 and disable building test examples * update mkldnn to v0.14 release * use git submodule tagging for mkldnn * update mklml to latest version

…examples (apache#10736)" (apache#10808) This reverts commit 3c7afcc.

update mkldnn to v0.14 and disable building test examples

a262298

zheng-da approved these changes Apr 28, 2018

View reviewed changes

ashokei changed the title ~~update mkldnn to v0.14 and disable building test examples~~ [MXNET-367] update mkldnn to v0.14 and disable building test examples Apr 28, 2018

ashokei added 2 commits April 28, 2018 00:30

update mkldnn to v0.14 release

14d931e

use git submodule tagging for mkldnn

23d8ff9

marcoabreu previously approved these changes Apr 28, 2018

View reviewed changes

update mklml to latest version

60b0386

ashokei requested a review from szha as a code owner April 28, 2018 15:51

szha reviewed May 3, 2018

View reviewed changes

marcoabreu merged commit 3c7afcc into apache:master May 4, 2018

marcoabreu added a commit that referenced this pull request May 4, 2018

Revert "[MXNET-367] update mkldnn to v0.14 and disable building test …

4368230

…examples (#10736)" This reverts commit 3c7afcc.

marcoabreu mentioned this pull request May 4, 2018

Revert "[MXNET-367] update mkldnn to v0.14 and disable building test examples" #10808

Merged

marcoabreu added a commit that referenced this pull request May 4, 2018

Revert "[MXNET-367] update mkldnn to v0.14 and disable building test …

66365ef

…examples (#10736)" (#10808) This reverts commit 3c7afcc.

anirudh2290 pushed a commit to anirudh2290/mxnet that referenced this pull request May 7, 2018

Revert "[MXNET-367] update mkldnn to v0.14 and disable building test …

e28f84a

…examples (apache#10736)" (apache#10808) This reverts commit 3c7afcc.

jinhuang415 pushed a commit to jinhuang415/incubator-mxnet that referenced this pull request May 29, 2018

Revert "[MXNET-367] update mkldnn to v0.14 and disable building test …

60ce708

…examples (apache#10736)" (apache#10808) This reverts commit 3c7afcc.

rahul003 pushed a commit to rahul003/mxnet that referenced this pull request Jun 4, 2018

Revert "[MXNET-367] update mkldnn to v0.14 and disable building test …

597599c

…examples (apache#10736)" (apache#10808) This reverts commit 3c7afcc.

zheng-da pushed a commit to zheng-da/incubator-mxnet that referenced this pull request Jun 28, 2018

Revert "[MXNET-367] update mkldnn to v0.14 and disable building test …

8ff5b3e

…examples (apache#10736)" (apache#10808) This reverts commit 3c7afcc.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MXNET-367] update mkldnn to v0.14 and disable building test examples #10736

[MXNET-367] update mkldnn to v0.14 and disable building test examples #10736

ashokei commented Apr 28, 2018 •

edited

ashokei commented Apr 28, 2018 •

edited

TaoLv commented Apr 28, 2018 •

edited

zheng-da commented Apr 28, 2018

TaoLv commented Apr 28, 2018 •

edited

ashokei commented Apr 28, 2018

TaoLv commented Apr 28, 2018

ashokei commented Apr 28, 2018

TaoLv commented Apr 28, 2018

zheng-da commented Apr 28, 2018

ashokei commented Apr 28, 2018

TaoLv commented Apr 28, 2018 •

edited

marcoabreu commented Apr 28, 2018

yajiedesign commented Apr 29, 2018 •

edited

ashokei commented May 2, 2018

szha May 3, 2018

ashokei May 3, 2018

szha May 3, 2018

ashokei May 3, 2018

szha May 3, 2018

ashokei May 3, 2018

marcoabreu May 3, 2018

marcoabreu May 3, 2018

ashokei May 4, 2018

[MXNET-367] update mkldnn to v0.14 and disable building test examples #10736

[MXNET-367] update mkldnn to v0.14 and disable building test examples #10736

Conversation

ashokei commented Apr 28, 2018 • edited

Description

Checklist

Essentials

Changes

Comments

ashokei commented Apr 28, 2018 • edited

TaoLv commented Apr 28, 2018 • edited

zheng-da commented Apr 28, 2018

TaoLv commented Apr 28, 2018 • edited

ashokei commented Apr 28, 2018

TaoLv commented Apr 28, 2018

ashokei commented Apr 28, 2018

TaoLv commented Apr 28, 2018

zheng-da commented Apr 28, 2018

ashokei commented Apr 28, 2018

TaoLv commented Apr 28, 2018 • edited

marcoabreu commented Apr 28, 2018

yajiedesign commented Apr 29, 2018 • edited

ashokei commented May 2, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ashokei commented Apr 28, 2018 •

edited

ashokei commented Apr 28, 2018 •

edited

TaoLv commented Apr 28, 2018 •

edited

TaoLv commented Apr 28, 2018 •

edited

TaoLv commented Apr 28, 2018 •

edited

yajiedesign commented Apr 29, 2018 •

edited