[RUNTIME] Support module based interface runtime #5753

FrozenGene · 2020-06-09T12:18:20Z

This pr supports #5038.

include/tvm/runtime/graph_runtime.h

include/tvm/runtime/graph_runtime_factory.h

tqchen · 2020-06-12T15:34:08Z

@FrozenGene please let us know when it is ready for review

FrozenGene · 2020-06-13T12:04:40Z

@FrozenGene please let us know when it is ready for review

sure. i will update the status inside this pr and will notify you after completion.

python/tvm/runtime/module.py

FrozenGene · 2020-06-23T12:13:00Z

@tqchen I think you could start to review the core part now and wish to listen your feedback. All the functionality (except the new graph_runtime api signature) of graph runtime (except debug graph runtime / vm) are done. The usage could be refer tests/python/unittest/test_module_runtime_interface.py

FrozenGene · 2020-06-24T14:26:44Z

gental ping @tqchen I am working on debug_graph_runtime / vm, but i think you could start to review core part :-)

tqchen · 2020-06-29T18:42:04Z

I will spend sometime to review the PR this week

ANSHUMAN87

@FrozenGene : Thanks for the PR! Great work 👍
I have done initial round of review. Please find some high level comments and some queries. Hope it helps. Thanks!

python/tvm/contrib/graph_runtime.py

python/tvm/rpc/client.py

python/tvm/runtime/graph_runtime_factory.py

python/tvm/runtime/module.py

src/runtime/graph/graph_runtime.h

src/runtime/graph/graph_runtime_factory.cc

src/runtime/graph/graph_runtime_factory.h

python/tvm/contrib/graph_runtime.py

python/tvm/rpc/client.py

python/tvm/runtime/graph_runtime_factory.py

src/runtime/graph/graph_runtime_factory.h

src/runtime/graph/graph_runtime.h

python/tvm/runtime/module.py

python/tvm/runtime/graph_runtime_factory.py

tqchen · 2020-07-06T16:44:05Z

Thanks @FrozenGene I made some initial comments. Would like to followup on the general design directions. The PR as it is implements the features we want. However, it is equally important to think about the minimalism.

In particular, we want to implement the feature using a minimum set of concepts(APIs). The runtime module based interface is more like a interface convention instead of a common implementation that we use for packaging. We can imagine different kinds of implementations, GraphRuntimeFactory is one of them(for graph executions). We would also like as much de-coupling as possible.

So the ley challenge is -- how can we implement the features using as a minimum set of API interface as possible.

We can dissect the current API into two category of functionalities

F0: Load the module in, execute
F1: Result of the relay.build, backward compatibility, write the module out.

Notably, F0 and F1 does not have to use the same runtime.module implementations.

Minimum Design for F0

If we focus on F0, we can find that we only need one interface for graph runtime in the C++ side(via Module API) -- the creation function:

from tvm.contrib import graph_runtime
gmod = graph_runtime.GraphModule(mod['resnet18'](tvm.cpu(0)))

Notably, in the use cases of F0, we do not need the GraphRuntimeFactory wrapper(as the wrapper itself is primarily for backward compatiblity reasons).

Minimum Design for F1

If we do not need to support the additional features(e.g. disable package_params or get_params). Then no additional API is needed.

We would certainly need the GraphFactoryModule wrapper to hold the return value of relay.build. However, note that the wrapper is only needed for backward compatibility reason only. As a result, we do not need to place GraphFactoryModule in the runtime folder, instead, we can just place it near under relay.backend, or close to graph_runtime.py for now. When we deprecate the old runtime API eventually, we could remove the python wrapper.

Discussions

From the above discussions, we can find that the only really necessary API is the factory creation function. We could certainly expose get_params so users can obtain the parameters.

The current way of implementing disable_params should be simplified. First of all, we would prefer stateless classes as much as possible, so the API that switch a flag on and off is not really a good idea.

One potential way to address the problem is to still use a compositional API:

mod = relay.build()
# return a new GraphFactoryModule with params removed
# mod_no_params does not need the GraphFactoryModule wrapping.
mod_no_params = mod["remove_params"]()
# no params will be exported
mod_no_params.export_library("xyz.so")
# params will be exported
mod.export_library("xyz.so")

We can discuss more API naming choices. Another parallel thread is how to create a debug runtime(if available). In this case, we could simply do mod["debug_create"]("default", ctx.cpu(0)).

FrozenGene · 2020-07-11T13:16:51Z

Thanks @tqchen @zhiics review. I will update code to address these comments tomorrow.

About tvm::map, do we have plan to move it into runtime/container like our tvm::String?

FrozenGene · 2020-07-13T10:49:43Z

@tqchen @zhiics could you help to review it again?

python/tvm/relay/backend/graph_runtime_factory.py

zhiics · 2020-07-13T15:57:10Z

src/runtime/graph/graph_runtime_factory.cc

+    names.emplace_back(v.first);
+    arrays.emplace_back(const_cast<DLTensor*>(v.second.operator->()));
+  }
+  uint64_t sz = arrays.size();


With the MetadataModule, we should be able to remove the serialization and deserialization of params for GraphRuntime and the factory. That may affect downstream users. I can take a stab on it later.

Could you give me a link about this MetadataModule?

It was introduced in #5770. We should not do it in this pr. This just makes you aware of it.

python/tvm/relay/backend/graph_runtime_factory.py

tqchen · 2020-07-14T03:31:05Z

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

@jwfromm can you also take a look?

zhiics

LGTM

ANSHUMAN87 · 2020-07-14T07:01:07Z

@FrozenGene : Sorry for late pitch in! With the latest change, i am not able to find dealing with multiple modules like previously done. Can you please point me to one or give an example how to do it. Thanks!

FrozenGene · 2020-07-14T07:17:20Z

@FrozenGene : With the latest change, i am not able to find dealing with multiple modules like previously done. Can you please point me to one or give an example how to do it. Thanks!

@ANSHUMAN87 Ah...yes. Latest change of this pr doesn't contain this part. The main reasons is our compiler doesn't be ready for it. For example, imagine we have one resnet18 model and resnet50 model for CPU, our compiler will not generate unique name for them. Both models will have the same function name like fused_nn_contrib_conv2d_NCHWc_add. So when our compiler is ready for this, we could enable multi model support. Current pr has considered this situation and we could add this support easily. Previous pr you see the multi model support is one model for CPU, one model for GPU, it just work around and bypass this issue.

ANSHUMAN87 · 2020-07-14T07:39:23Z

@FrozenGene : Thanks a lot! I got it now. So may be we add this as a note in the original issue tracker for this feature. So that we can comeback to it at later point.
To me the multiple module support is the key attraction 🙂

ANSHUMAN87

LGTM. Thanks @FrozenGene 👍

FrozenGene · 2020-07-14T07:51:15Z

@FrozenGene : Thanks a lot! I got it now. So may be we add this as a note in the original issue tracker for this feature. So that we can comeback to it at later point.
To me the multiple module support is the key attraction 🙂

I have listed it in original RFC (#5038)

tqchen · 2020-07-15T03:07:56Z

Thanks @FrozenGene , this PR is now merged. Thanks @zhiics @ANSHUMAN87

FrozenGene mentioned this pull request Jun 9, 2020

[RFC] Module based Model Runtime Interface #5038

Closed

2 tasks

tqchen requested changes Jun 9, 2020

View reviewed changes

include/tvm/runtime/graph_runtime.h Outdated Show resolved Hide resolved

include/tvm/runtime/graph_runtime_factory.h Outdated Show resolved Hide resolved

tqchen self-assigned this Jun 12, 2020

tqchen added the status: WIP label Jun 12, 2020

FrozenGene force-pushed the model_based_runtime branch from 4ca9e56 to 5563dea Compare June 15, 2020 12:43

FrozenGene commented Jun 15, 2020

View reviewed changes

python/tvm/runtime/module.py Outdated Show resolved Hide resolved

FrozenGene force-pushed the model_based_runtime branch 2 times, most recently from 9ecd15f to 3391c66 Compare June 23, 2020 11:57

ANSHUMAN87 reviewed Jul 2, 2020

View reviewed changes

ANSHUMAN87 reviewed Jul 4, 2020

View reviewed changes

FrozenGene force-pushed the model_based_runtime branch 2 times, most recently from bd8063b to 2f16dde Compare July 6, 2020 10:09

tqchen requested changes Jul 6, 2020

View reviewed changes

tqchen added the status: need update need update based on feedbacks label Jul 6, 2020

FrozenGene marked this pull request as ready for review July 9, 2020 06:17

FrozenGene changed the title ~~[Draft] Support Module based interface runtime~~ Support Module based interface runtime Jul 9, 2020

FrozenGene changed the title ~~Support Module based interface runtime~~ Support module based interface runtime Jul 9, 2020

FrozenGene added 7 commits July 9, 2020 16:31

Support Module based interface runtime

3fbf640

remove unnecessary comment

3fbd368

support rpc (except params issue)

cb101ac

solve rpc issue

95803d0

support package params

166e009

[Complete all the functionality] Support multi models of package params

52adf79

refactor graph runtime module list

081af5f

FrozenGene added 3 commits July 13, 2020 14:52

address comments

e796d4b

update doc comments

03793b3

add get_json for debug graph runtime

d7f44a9

FrozenGene added 2 commits July 13, 2020 19:23

comment fix

5c23a07

Trigger CI

54bbdc8

zhiics reviewed Jul 13, 2020

View reviewed changes

tqchen requested changes Jul 14, 2020

View reviewed changes

python/tvm/relay/backend/graph_runtime_factory.py Outdated Show resolved Hide resolved

update

a83333a

zhiics approved these changes Jul 14, 2020

View reviewed changes

ANSHUMAN87 approved these changes Jul 14, 2020

View reviewed changes

tqchen approved these changes Jul 15, 2020

View reviewed changes

tqchen changed the title ~~Support module based interface runtime~~ [RUNTIME] Support module based interface runtime Jul 15, 2020

tqchen merged commit 9fcde21 into apache:master Jul 15, 2020

tqchen added status: accepted and removed status: WIP status: need update need update based on feedbacks labels Jul 15, 2020

FrozenGene deleted the model_based_runtime branch July 16, 2020 02:39

trevor-m pushed a commit to trevor-m/tvm that referenced this pull request Aug 26, 2020

[RUNTIME] Support module based interface runtime (apache#5753)

0ef4a35

trevor-m pushed a commit to trevor-m/tvm that referenced this pull request Aug 26, 2020

[RUNTIME] Support module based interface runtime (apache#5753)

4509044

trevor-m pushed a commit to trevor-m/tvm that referenced this pull request Sep 2, 2020

[RUNTIME] Support module based interface runtime (apache#5753)

30caee5

trevor-m pushed a commit to neo-ai/tvm that referenced this pull request Sep 3, 2020

[RUNTIME] Support module based interface runtime (apache#5753)

31d559b

ZihengJiang mentioned this pull request Sep 25, 2020

TVM v0.7 Release Note Candidate #6486

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[RUNTIME] Support module based interface runtime #5753

[RUNTIME] Support module based interface runtime #5753

FrozenGene commented Jun 9, 2020 •

edited

Loading

tqchen commented Jun 12, 2020

FrozenGene commented Jun 13, 2020

FrozenGene commented Jun 23, 2020

FrozenGene commented Jun 24, 2020

tqchen commented Jun 29, 2020

ANSHUMAN87 left a comment

tqchen commented Jul 6, 2020 •

edited

Loading

FrozenGene commented Jul 11, 2020 •

edited

Loading

FrozenGene commented Jul 13, 2020

zhiics Jul 13, 2020

FrozenGene Jul 14, 2020

zhiics Jul 14, 2020

tqchen commented Jul 14, 2020

zhiics left a comment

ANSHUMAN87 commented Jul 14, 2020 •

edited

Loading

FrozenGene commented Jul 14, 2020 •

edited

Loading

ANSHUMAN87 commented Jul 14, 2020

ANSHUMAN87 left a comment

FrozenGene commented Jul 14, 2020

tqchen commented Jul 15, 2020 •

edited

Loading

[RUNTIME] Support module based interface runtime #5753

[RUNTIME] Support module based interface runtime #5753

Conversation

FrozenGene commented Jun 9, 2020 • edited Loading

tqchen commented Jun 12, 2020

FrozenGene commented Jun 13, 2020

FrozenGene commented Jun 23, 2020

FrozenGene commented Jun 24, 2020

tqchen commented Jun 29, 2020

ANSHUMAN87 left a comment

Choose a reason for hiding this comment

tqchen commented Jul 6, 2020 • edited Loading

Minimum Design for F0

Minimum Design for F1

Discussions

FrozenGene commented Jul 11, 2020 • edited Loading

FrozenGene commented Jul 13, 2020

zhiics Jul 13, 2020

Choose a reason for hiding this comment

FrozenGene Jul 14, 2020

Choose a reason for hiding this comment

zhiics Jul 14, 2020

Choose a reason for hiding this comment

tqchen commented Jul 14, 2020

zhiics left a comment

Choose a reason for hiding this comment

ANSHUMAN87 commented Jul 14, 2020 • edited Loading

FrozenGene commented Jul 14, 2020 • edited Loading

ANSHUMAN87 commented Jul 14, 2020

ANSHUMAN87 left a comment

Choose a reason for hiding this comment

FrozenGene commented Jul 14, 2020

tqchen commented Jul 15, 2020 • edited Loading

FrozenGene commented Jun 9, 2020 •

edited

Loading

tqchen commented Jul 6, 2020 •

edited

Loading

FrozenGene commented Jul 11, 2020 •

edited

Loading

ANSHUMAN87 commented Jul 14, 2020 •

edited

Loading

FrozenGene commented Jul 14, 2020 •

edited

Loading

tqchen commented Jul 15, 2020 •

edited

Loading