[RUNTIME] Implement TVMDSOOp(TensorFlow custom op) for TVM runtime #4459

tobegit3hub · 2019-12-03T12:01:50Z

This PR contains three parts to achieve the goal to run TVM op in TensorFlow.

The kernel and definition of TensorFlow custom op.
The Python package in tvm.contrib to load TVMDSOOp easily.
The config and CMake file to build TVMDSOOp easily.

tobegit3hub · 2019-12-04T06:52:02Z

It would be great if @tqchen would help to review this.

tqchen · 2019-12-19T18:05:35Z

@tobegit3hub please fix the lint error. @soiferj @jwfromm @zhiics @FrozenGene would be great if you can help review this pr

zhiics · 2019-12-19T18:25:39Z

Sure. I will do a pass by tomorrow.

CMakeLists.txt

cmake/modules/contrib/TFOP.cmake

python/tvm/contrib/tf_op/module.py

src/contrib/tf_op/tvm_dso_op_kernels.cc

zhiics

I am half way done. Will spend some time on the remaining files. Some high level comments/questions are:

Style, please make sure you keep consistent to the currently code style in TVM. For example, indentation for Python and C++, number of blank lines between functions, etc.
Naming, we might need to debate a bit about TVMDSOOp
Testing, please add some unit tests. I think we need both Python tests and gtest.
I've finished reviewing, but I am wondering how dynamic shape is handled?

python/tvm/contrib/tf_op/module.py

src/contrib/tf_op/index_seq.h

src/contrib/tf_op/tvm_dso_op_kernels.cc

src/contrib/tf_op/index_seq.h

tobegit3hub · 2019-12-23T02:52:16Z

Thanks @FrozenGene and @zhiics . We will fix for these comments soon.

Since TensorFlow allow custom operators to accept dynamic shape whose first dimension is -1, we can get the actual input dimension from input tensor in kernel compute function. The output dimension should be specified by users. Now we support specifying the fixed dimension or pass the shape op which is the transformation of input tensor.

CMakeLists.txt

src/contrib/tf_op/tvm_dso_op_kernels.cc

feat: Update tf tvmdso op by review comments See merge request internal-open-source/incubator-tvm!2

cmake/modules/contrib/TF_TVMDSOOP.cmake

fix: Update with pr comments See merge request internal-open-source/incubator-tvm!3

fix: Fix lint See merge request internal-open-source/incubator-tvm!4

tobegit3hub · 2020-01-17T02:53:33Z

Hi @zhiics and @FrozenGene , we have updated the code for the comments. Would you like to review again when you are free?

wrongtest-intellif · 2020-04-02T09:56:42Z

Hi @zhiics, we have updated the code for the comments. Would you like to review again when you are free? 8ac182f

zhiics · 2020-04-02T15:27:29Z

I think @gmagogsfm had a concern about the variable number of inputs/outputs, has this been fixed?

wrongtest-intellif · 2020-04-03T03:53:26Z

Oh I miss it. @gmagogsfm remind a nice mechanism in TF to handle variable number of input tensors which will greatly simplify the current op definitions.

I tend to make it in next PR, because currently we rely on template argument
NUM_INPUTS to invoke variadic TVM function with array typed input arguments. If we use input: list(T) to define variable inputs, we have to get another way to do the invocation. Any suggestions for this improvement @tobegit3hub ?

gmagogsfm · 2020-04-03T03:58:12Z

@wrongtest
You can invoke TVM packed func with a variadic number of inputs with TVMArgsSetter

https://github.com/apache/incubator-tvm/blob/e0122c0ea68043372220e4e02b81692c34832227/src/runtime/contrib/example_ext_runtime/example_ext_runtime.cc#L191

Let me know if you have other questions, happy to help.

wrongtest-intellif · 2020-04-03T05:39:45Z

Thanks for your help! We update the code and now there is no need to define different op for different input args. @gmagogsfm

zhiics · 2020-04-03T22:54:44Z

I will do a final round review in the next couple of days. I think overall it looks good to me now. @tqchen can you also take a look?

tqchen · 2020-04-07T03:49:28Z

@zhiics @FrozenGene please https://tvm.apache.org/docs/contribute/code_review.html#approve-and-request-changes-explicitly

python/tvm/contrib/tf_op/module.py

cmake/config.cmake

zhiics

Thanks for the effort. LGTM. Let's follow up in separate PRs and see how we can enable the tests to help ppl to try it out.

FrozenGene · 2020-04-07T06:31:07Z

I will review it one round today. If no other comment, I will approve today.

FrozenGene · 2020-04-07T10:04:28Z

Wish we have one doc / tutorial to instruct users how to use this feature in the next pr.

tqchen · 2020-04-08T00:00:12Z

Thanks @tobegit3hub @FrozenGene @zhiics @soiferj @wrongtest @gmagogsfm @yzhliu . This PR is now merged

…pache#4459) * Add implementation of TVMDSOOp * feat: Update cmake script to work with c++11 and in-repo build * feat: Use libtvm as oplib dependency * fix: Add missing link dependency to libtvm * feat: Update tf tvmdso op by review comments * fix: Update with pr comments * fix: Fix lint * feat: Add test script and fix gpu shape * feat: Add test script and fix gpu shape * fix: Conditional build tftvm op for gpu * fix: Conditional build tftvm op for gpu * fix: Fix pylint of tf_op module.py * fix: Fix pylint of tf_op module.py * feat: Conditional enable gpu test for tftvm op * feat: Conditional enable gpu test for tftvm op * feat: Add tf_tvmdsoop test script as an app test * fix: Fix gpu/cpu enabled check on tvm in test script * fix: Make tf tvmdso op test script runnable with pytest * remove unused test script test_tfop_module.py * fix: Remove pushd & popd in tfdsoop test script * fix: Upgrade tftvmop use python3 to find TensorFlow * fix: Upgrade tftvmop use python3 to find TensorFlow * fix: Change target_link_options to target_link_libraries * fix: Add tftvmop build script's c++ option * fix: Add tvm library path to tf op test library path * fix: Debug ci build for tftvm dso op * fix: Fix cmake error and skip tfop test * fix: Fix typo and indentation issues * feat: Use TF list input op def * fix: Fix style and unexpected changes Co-authored-by: baoxinqi <baoxinqi@4paradigm.com> Co-authored-by: Chen Dihao <chendihao@4paradigm.com> Co-authored-by: wrongtest <wrongtest@4paradigm.com>

tobegit3hub and others added 3 commits December 3, 2019 19:49

Add implementation of TVMDSOOp

73726a8

feat: Update cmake script to work with c++11 and in-repo build

9c3f732

feat: Use libtvm as oplib dependency

8361217

tobegit3hub mentioned this pull request Dec 4, 2019

[RFC] Add TVMDSOOp to integrate any TVM operator with TensorFlow #4464

Closed

tobegit3hub changed the title ~~[WIP] Implement TVMDSOOp(TensorFlow custom op) for TVM runtime~~ Implement TVMDSOOp(TensorFlow custom op) for TVM runtime Dec 4, 2019

fix: Add missing link dependency to libtvm

ef387fd

tqchen added the status: need review label Dec 4, 2019

Merge branch 'master' into add_tvmdsoop

41d9b38

tqchen mentioned this pull request Dec 19, 2019

[RFC] Support Tensorflow Op Bridge #3059

Closed

tqchen changed the title ~~Implement TVMDSOOp(TensorFlow custom op) for TVM runtime~~ [RUNTIME] Implement TVMDSOOp(TensorFlow custom op) for TVM runtime Dec 19, 2019

FrozenGene requested changes Dec 20, 2019

View reviewed changes

zhiics requested changes Dec 20, 2019

View reviewed changes

soiferj reviewed Dec 23, 2019

View reviewed changes

CMakeLists.txt Outdated Show resolved Hide resolved

src/contrib/tf_op/tvm_dso_op_kernels.cc Outdated Show resolved Hide resolved

src/contrib/tf_op/tvm_dso_op_kernels.cc Show resolved Hide resolved

feat: Update tf tvmdso op by review comments

056d4b1

tqchen added the status: need update need update based on feedbacks label Jan 5, 2020

tobegit3hub and others added 2 commits January 6, 2020 18:25

Fix conflict of master

9fb271c

Merge branch 'fix/add_tvmdsoop_fix_review' into 'feat/add_tvmdsoop'

a6d9c28

feat: Update tf tvmdso op by review comments See merge request internal-open-source/incubator-tvm!2

yzhliu reviewed Jan 8, 2020

View reviewed changes

cmake/modules/contrib/TF_TVMDSOOP.cmake Outdated Show resolved Hide resolved

tqchen assigned yzhliu Jan 15, 2020

baoxinqi and others added 2 commits January 16, 2020 16:24

fix: Update with pr comments

69df549

Merge branch 'feat/UpdatePR' into 'feat/add_tvmdsoop'

23ce5b2

fix: Update with pr comments See merge request internal-open-source/incubator-tvm!3

tobegit3hub requested review from zhiics and FrozenGene January 16, 2020 08:58

baoxinqi and others added 2 commits January 16, 2020 17:30

fix: Fix lint

c2e72a6

Merge branch 'fix/FixLint' into 'feat/add_tvmdsoop'

e390c49

fix: Fix lint See merge request internal-open-source/incubator-tvm!4

wrongtest-intellif force-pushed the add_tvmdsoop branch from 79a5fa3 to 3984459 Compare April 3, 2020 05:34

feat: Use TF list input op def

9343669

wrongtest-intellif force-pushed the add_tvmdsoop branch from 3984459 to 9343669 Compare April 3, 2020 05:36

tqchen approved these changes Apr 7, 2020

View reviewed changes

tqchen reviewed Apr 7, 2020

View reviewed changes

python/tvm/contrib/tf_op/module.py Outdated Show resolved Hide resolved

tqchen requested changes Apr 7, 2020

View reviewed changes

cmake/config.cmake Outdated Show resolved Hide resolved

fix: Fix style and unexpected changes

38af1ce

zhiics approved these changes Apr 7, 2020

View reviewed changes

FrozenGene approved these changes Apr 7, 2020

View reviewed changes

tqchen approved these changes Apr 7, 2020

View reviewed changes

tqchen merged commit 53a4ad3 into apache:master Apr 7, 2020

tqchen added status: accepted and removed status: need review status: need test case need test cases to cover the change status: need update need update based on feedbacks labels Apr 8, 2020

ZihengJiang mentioned this pull request Sep 25, 2020

TVM v0.7 Release Note Candidate #6486

Closed

zhanghaohit deleted the add_tvmdsoop branch December 17, 2020 02:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[RUNTIME] Implement TVMDSOOp(TensorFlow custom op) for TVM runtime #4459

[RUNTIME] Implement TVMDSOOp(TensorFlow custom op) for TVM runtime #4459

tobegit3hub commented Dec 3, 2019 •

edited

Loading

tobegit3hub commented Dec 4, 2019

tqchen commented Dec 19, 2019

zhiics commented Dec 19, 2019

zhiics left a comment

tobegit3hub commented Dec 23, 2019 •

edited

Loading

tobegit3hub commented Jan 17, 2020

wrongtest-intellif commented Apr 2, 2020

zhiics commented Apr 2, 2020

wrongtest-intellif commented Apr 3, 2020

gmagogsfm commented Apr 3, 2020

wrongtest-intellif commented Apr 3, 2020

zhiics commented Apr 3, 2020

tqchen commented Apr 7, 2020

zhiics left a comment

FrozenGene commented Apr 7, 2020

FrozenGene commented Apr 7, 2020

tqchen commented Apr 8, 2020

[RUNTIME] Implement TVMDSOOp(TensorFlow custom op) for TVM runtime #4459

[RUNTIME] Implement TVMDSOOp(TensorFlow custom op) for TVM runtime #4459

Conversation

tobegit3hub commented Dec 3, 2019 • edited Loading

tobegit3hub commented Dec 4, 2019

tqchen commented Dec 19, 2019

zhiics commented Dec 19, 2019

zhiics left a comment

Choose a reason for hiding this comment

tobegit3hub commented Dec 23, 2019 • edited Loading

tobegit3hub commented Jan 17, 2020

wrongtest-intellif commented Apr 2, 2020

zhiics commented Apr 2, 2020

wrongtest-intellif commented Apr 3, 2020

gmagogsfm commented Apr 3, 2020

wrongtest-intellif commented Apr 3, 2020

zhiics commented Apr 3, 2020

tqchen commented Apr 7, 2020

zhiics left a comment

Choose a reason for hiding this comment

FrozenGene commented Apr 7, 2020

FrozenGene commented Apr 7, 2020

tqchen commented Apr 8, 2020

tobegit3hub commented Dec 3, 2019 •

edited

Loading

tobegit3hub commented Dec 23, 2019 •

edited

Loading