[RUNTIME] Support module based interface runtime

[FrozenGene]

This pr supports #5038.

[tqchen]

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

[FrozenGene]

@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.

[FrozenGene]

@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]

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

[tqchen]

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!

[tqchen]

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:

fromtvm.contribimportgraph_runtimegmod=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 exportedmod_no_params.export_library("xyz.so")
# params will be exportedmod.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]

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]

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

[zhiics]

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.

[FrozenGene]

Could you give me a link about this MetadataModule?

[zhiics]

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

[tqchen]

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]

@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]

@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]

@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]

@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]

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