Fixing issue #17840 #18526

jinboci · 2020-06-09T15:18:11Z

Description

(Brief description on what this PR is about)
Fixing issue #17840. Referring to the tutorial, the problems causing the issue #17840 include:

The TVMOp GPU module was not saved,
The TVMOp GPU module was not loaded or imported.

This pull request fixes the issue by saving the compiled GPU module to libtvmop.cubin and loading it when import mxnet.

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 https://mxnet-ci-doc.s3-accelerate.dualstack.amazonaws.com/PR-$PR_ID/$BUILD_ID/index.html
To the best of my 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

mxnet-bot · 2020-06-09T15:18:14Z

Hey @jinboci , Thanks for submitting the PR
All tests are already queued to run once. If tests fail, you can trigger one or more tests again with the following commands:

To trigger all jobs: @mxnet-bot run ci [all]
To trigger specific jobs: @mxnet-bot run ci [job1, job2]

CI supported jobs: [website, windows-cpu, unix-cpu, centos-gpu, sanity, edge, unix-gpu, windows-gpu, centos-cpu, clang, miscellaneous]

Note:
Only following 3 categories can trigger CI :PR Author, MXNet Committer, Jenkins Admin.
All CI tests must pass before the PR can be merged.

jinboci · 2020-06-10T08:40:15Z

@mxnet-bot run ci [unix-cpu]

mxnet-bot · 2020-06-10T08:40:21Z

Jenkins CI successfully triggered : [unix-cpu]

yzhliu · 2020-06-11T03:17:29Z

contrib/tvmop/compile.py

+        imported_modules
+    except NameError:
+        imported_modules = []
+    if len(imported_modules):


why checking imported_modules while actually accessing func_binary.imported_modules ?

Sorry, I fixed it.

yzhliu · 2020-06-11T03:24:46Z

src/c_api/c_api.cc

+  libpath_module->Load(libpath);
+#if MXNET_USE_CUDA
+  tvm::runtime::TVMOpModule cubin_module;
+  cubin_module.Load("libtvmop.cubin");


it should combine with the libpath, otherwise it is loading from ./libtvmop.cubin

Fixed. Thanks!

jinboci · 2020-06-11T07:10:14Z

@mxnet-bot run ci [all]

mxnet-bot · 2020-06-11T07:10:25Z

Jenkins CI successfully triggered : [unix-cpu, edge, centos-cpu, website, windows-cpu, clang, sanity, centos-gpu, windows-gpu, unix-gpu, miscellaneous]

jinboci · 2020-06-11T07:39:40Z

@mxnet-bot run ci [all]

mxnet-bot · 2020-06-11T07:39:53Z

Jenkins CI successfully triggered : [clang, edge, sanity, unix-cpu, windows-cpu, centos-cpu, miscellaneous, website, windows-gpu, centos-gpu, unix-gpu]

src/c_api/c_api.cc

yzhliu · 2020-06-12T03:32:52Z

src/c_api/c_api.cc

+  libpath_module->Load(libpath);
+#if MXNET_USE_CUDA
+  std::string libpathstr(libpath);
+  std::string cubinpath = libpathstr.substr(0, libpathstr.size() - 11) + "libtvmop.cubin";


would be better to pass libpath as dir, and do libpath + "libtvmop.so" as well to keep consistency.

I rewrite it in a more elegant way :)

Yes, but MXLoadTVMOp is called at:
https://github.com/apache/incubator-mxnet/blob/1bf881f381f91b157a26d9beddcaa8f4960cc038/python/mxnet/tvmop.py#L31-L32
where _LIB_TVM_OP is returned from the
https://github.com/apache/incubator-mxnet/blob/1bf881f381f91b157a26d9beddcaa8f4960cc038/python/mxnet/libinfo.py#L25
, and _LIB_TVM_OP[0] is the path of libtvmop.so.
We may need to modify find_lib_path or write a new function to get the directory that libtvmop.so locates.

yzhliu · 2020-06-12T03:36:54Z

contrib/tvmop/compile.py

+    try:
+        func_binary.imported_modules
+    except NameError:
+        func_binary.imported_modules = []


from https://github.com/apache/incubator-tvm/blob/master/python/tvm/runtime/module.py#L136 we can see func_binary.imported_modules should always exist.

Thank you for your review. I have deleted these lines.

Co-authored-by: Yizhi Liu <[email protected]>

… into new_branch

Ubuntu and others added 7 commits May 7, 2020 06:37

fix the error message of reshape()

6774474

Fixing issue apache#16655 reshape() error message

29ddefe

test pr

7627eaa

fixing apache#17840

087a100

fixing issue apache#17840

75f975b

Merge remote-tracking branch 'upstream/master' into new_branch

aafaa99

fixing issue apache#17840

84c389a

jinboci requested a review from szha as a code owner June 9, 2020 15:18

Ubuntu added 4 commits June 9, 2020 15:20

remove blankspace

47ba73f

fixing bugs

060a3b1

fixing bugs

46256d4

fixing cpu errors

bb8727b

yzhliu reviewed Jun 11, 2020

View reviewed changes

fixing review issues

1e406c7

yzhliu reviewed Jun 12, 2020

View reviewed changes

restoring tvmop tests

c4dbc64

jinboci mentioned this pull request Jun 12, 2020

Restoring TVMOp tests #18542

Draft

7 tasks

jinboci and others added 6 commits June 12, 2020 13:56

Apply suggestions from code review

a92f500

Co-authored-by: Yizhi Liu <[email protected]>

Update c_api.cc

beca3b8

Apply suggestions from code review

2bdeb6b

Merge branch 'new_branch' of https://github.com/JinboCi/incubator-mxnet…

d55a005

… into new_branch

Apply suggestions from code review

3183a90

Merge branch 'test_tvmop' into new_branch

3bb193c

jinboci requested review from aaronmarkham and marcoabreu as code owners June 12, 2020 18:09

jinboci and others added 8 commits June 13, 2020 02:13

Update multiarray.py

dfd98a4

Merge branch 'master' into new_branch

a578b0d

fixing bugs

ebfc059

Update c_api.cc

c75ed75

try fixing libcuda.so.1 cannot open shared object file

0ee8126

reset changes of utls.py

4c6298f

fixing bugs

da96721

Merge branch 'new_branch' of https://github.com/JinboCi/incubator-mxnet…

165f697

… into new_branch

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixing issue #17840 #18526

Fixing issue #17840 #18526

jinboci commented Jun 9, 2020 •

edited

Loading

mxnet-bot commented Jun 9, 2020

jinboci commented Jun 10, 2020

mxnet-bot commented Jun 10, 2020

yzhliu Jun 11, 2020

jinboci Jun 11, 2020

yzhliu Jun 11, 2020

jinboci Jun 11, 2020

jinboci commented Jun 11, 2020

mxnet-bot commented Jun 11, 2020

jinboci commented Jun 11, 2020

mxnet-bot commented Jun 11, 2020

yzhliu Jun 12, 2020

jinboci Jun 12, 2020 •

edited

Loading

yzhliu Jun 12, 2020

jinboci Jun 12, 2020

Fixing issue #17840 #18526

Are you sure you want to change the base?

Fixing issue #17840 #18526

Conversation

jinboci commented Jun 9, 2020 • edited Loading

Description

Checklist

Essentials

Changes

Comments

mxnet-bot commented Jun 9, 2020

jinboci commented Jun 10, 2020

mxnet-bot commented Jun 10, 2020

yzhliu Jun 11, 2020

Choose a reason for hiding this comment

jinboci Jun 11, 2020

Choose a reason for hiding this comment

yzhliu Jun 11, 2020

Choose a reason for hiding this comment

jinboci Jun 11, 2020

Choose a reason for hiding this comment

jinboci commented Jun 11, 2020

mxnet-bot commented Jun 11, 2020

jinboci commented Jun 11, 2020

mxnet-bot commented Jun 11, 2020

yzhliu Jun 12, 2020

Choose a reason for hiding this comment

jinboci Jun 12, 2020 • edited Loading

Choose a reason for hiding this comment

yzhliu Jun 12, 2020

Choose a reason for hiding this comment

jinboci Jun 12, 2020

Choose a reason for hiding this comment

jinboci commented Jun 9, 2020 •

edited

Loading

jinboci Jun 12, 2020 •

edited

Loading