tqchen edited a comment on pull request #5753:
URL: https://github.com/apache/incubator-tvm/pull/5753#issuecomment-654347387
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:
```python
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:
```python
mod = relay.build()
mod_no_params = mod["remove_params"]()
# no params will be exported
mod_no_params.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))```.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
