You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by GitBox <gi...@apache.org> on 2020/06/10 23:45:47 UTC

[GitHub] [incubator-tvm] zhiics opened a new pull request #5770: [BYOC][runtime] Separate code and metadata for CSourceModule

zhiics opened a new pull request #5770:
URL: https://github.com/apache/incubator-tvm/pull/5770


   This PR attempts to separate metadata initialization and source code compilation (when necessary). The discussion can be found here: https://discuss.tvm.ai/t/byoc-runtime-json-runtime-for-byoc/6579/15
   
   The goal is to have a standalone wrapper (`ModuleInitWrapper`) to take care of module initialization for different runtimes. In the BYOC case, we save constants and code separately. The constant would be loaded by the wrapper while the code is handled by csourcemodule. The end-to-end flow of using a BYOC example would be like the following:
   
   ```python
   # the pipeline for annotation and partitioning is the same.
   # Given an already partitioned IRModule, mod
   graph, lib, params = relay.build(mod, target="llvm", params=params)
   
   # new_lib contains the host lib, source_metadata_mods contains a list of modules that wraps
   # csource or (json for execution engine when needed) and metadata
   new_lib, source_metadata_mods = lib.unwrap_modules()
   if source_metadata_mods:
       ext_mod = tvm.target.SourceMetadataModule(source_metadata_mods[0])
       init_mod = runtime.ModuleInitWrapper(metadata)
       init_mod.import_module(tvm.target.CSourceModule(ext_mod.source))
       new_lib.import_module(init_mod)
       new_lib.export_library("lib.so")
   ```


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-tvm] zhiics commented on a change in pull request #5770: [BYOC][runtime] Separate code and metadata for CSourceModule

Posted by GitBox <gi...@apache.org>.
zhiics commented on a change in pull request #5770:
URL: https://github.com/apache/incubator-tvm/pull/5770#discussion_r439592645



##########
File path: src/target/source/source_module.cc
##########
@@ -152,8 +153,92 @@ runtime::Module DeviceSourceModuleCreate(
   return runtime::Module(n);
 }
 
+// A helper used to wrap different types of modules and pass through packedfunc.
+// This module will never be used for compilation and execution.
+class ModuleClassWrapperNode : public runtime::ModuleNode {
+ public:
+  ModuleClassWrapperNode() = default;
+  const char* type_key() const { return "module_class_wrapper"; }
+  PackedFunc GetFunction(const std::string& name, const ObjectPtr<Object>& sptr_to_self) final {
+    LOG(FATAL) << "Cannot execute module wrapper";
+    return PackedFunc();
+  }
+};
+
+runtime::Module ModuleClassWrapperCreate() {
+  auto n = make_object<ModuleClassWrapperNode>();
+  return runtime::Module(n);
+}
+
+// Pack the source code and metadata, where source code could be any
+// user-defined code, i.e. c source code, json graph representation, etc.
+class SourceMetadataModuleNode final : public runtime::ModuleNode {
+ public:
+  SourceMetadataModuleNode(const String& func_symbol, const String& code, const String& source_type,
+                           const Array<String>& variables, const Array<runtime::NDArray>& metadata)
+      : func_symbol_(func_symbol),
+        code_(code),
+        source_type_(source_type),
+        variables_(variables),
+        metadata_(metadata) {}
+
+  PackedFunc GetFunction(const std::string& name, const ObjectPtr<Object>& sptr_to_self) final {
+    if (name == "get_source") {
+      return PackedFunc([sptr_to_self, this](TVMArgs args, TVMRetValue* rv) { *rv = this->code_; });
+    } else if (name == "get_source_type") {
+      return PackedFunc(
+          [sptr_to_self, this](TVMArgs args, TVMRetValue* rv) { *rv = this->source_type_; });
+    } else if (name == "get_symbol") {
+      return PackedFunc(
+          [sptr_to_self, this](TVMArgs args, TVMRetValue* rv) { *rv = this->func_symbol_; });
+    } else if (name == "get_vars") {
+      return PackedFunc(
+          [sptr_to_self, this](TVMArgs args, TVMRetValue* rv) { *rv = this->variables_; });
+    } else if (name == "get_metadata") {
+      return PackedFunc(

Review comment:
       This module saves the data including source and metadata from the partitioned graphs. It is only used for packaging purpose. CSourceMoudle and later on other modules (e.g json runtime module) will take the code from it. The `ModuleInitWrapper` will take the variables and metadata from it. 
   
   If we can let MetadataModule take all {var: ndarray} mapping and let CSourceModule get the needed variables. We should be able to remove this module as well.




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-tvm] tqchen commented on a change in pull request #5770: [BYOC][runtime] Separate code and metadata for CSourceModule

Posted by GitBox <gi...@apache.org>.
tqchen commented on a change in pull request #5770:
URL: https://github.com/apache/incubator-tvm/pull/5770#discussion_r439596323



##########
File path: src/runtime/module_init_wrapper.cc
##########
@@ -0,0 +1,234 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*!
+ * \file src/runtime/module_init_wrapper.cc
+ * \brief A wrapper for initializing modules using metadata
+ */
+#include <tvm/node/container.h>
+#include <tvm/runtime/ndarray.h>
+#include <tvm/runtime/packed_func.h>
+#include <tvm/runtime/registry.h>
+
+#include <cstdint>
+#include <sstream>
+
+#include "file_util.h"
+
+namespace tvm {
+namespace runtime {
+
+using StringNDArrayMap = std::unordered_map<String, runtime::NDArray, ObjectHash, ObjectEqual>;
+
+class CSourceMetadataInitializer {
+ public:
+  explicit CSourceMetadataInitializer(const StringNDArrayMap& metadata) : metadata_(metadata) {}
+
+  template <typename T>

Review comment:
       Can we change the subgraph generator to instead expose `InitMod0Func0(Array<NDArray>)` instead? so that we don't have to compile the meta data into the C source?




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-tvm] zhiics commented on a change in pull request #5770: [BYOC][runtime] Separate code and metadata for CSourceModule

Posted by GitBox <gi...@apache.org>.
zhiics commented on a change in pull request #5770:
URL: https://github.com/apache/incubator-tvm/pull/5770#discussion_r439618825



##########
File path: src/target/source/source_module.cc
##########
@@ -152,8 +153,92 @@ runtime::Module DeviceSourceModuleCreate(
   return runtime::Module(n);
 }
 
+// A helper used to wrap different types of modules and pass through packedfunc.
+// This module will never be used for compilation and execution.
+class ModuleClassWrapperNode : public runtime::ModuleNode {
+ public:
+  ModuleClassWrapperNode() = default;
+  const char* type_key() const { return "module_class_wrapper"; }
+  PackedFunc GetFunction(const std::string& name, const ObjectPtr<Object>& sptr_to_self) final {
+    LOG(FATAL) << "Cannot execute module wrapper";
+    return PackedFunc();
+  }
+};
+
+runtime::Module ModuleClassWrapperCreate() {
+  auto n = make_object<ModuleClassWrapperNode>();
+  return runtime::Module(n);
+}
+
+// Pack the source code and metadata, where source code could be any
+// user-defined code, i.e. c source code, json graph representation, etc.
+class SourceMetadataModuleNode final : public runtime::ModuleNode {
+ public:
+  SourceMetadataModuleNode(const String& func_symbol, const String& code, const String& source_type,
+                           const Array<String>& variables, const Array<runtime::NDArray>& metadata)
+      : func_symbol_(func_symbol),
+        code_(code),
+        source_type_(source_type),
+        variables_(variables),
+        metadata_(metadata) {}
+
+  PackedFunc GetFunction(const std::string& name, const ObjectPtr<Object>& sptr_to_self) final {
+    if (name == "get_source") {
+      return PackedFunc([sptr_to_self, this](TVMArgs args, TVMRetValue* rv) { *rv = this->code_; });
+    } else if (name == "get_source_type") {
+      return PackedFunc(
+          [sptr_to_self, this](TVMArgs args, TVMRetValue* rv) { *rv = this->source_type_; });
+    } else if (name == "get_symbol") {
+      return PackedFunc(
+          [sptr_to_self, this](TVMArgs args, TVMRetValue* rv) { *rv = this->func_symbol_; });
+    } else if (name == "get_vars") {
+      return PackedFunc(
+          [sptr_to_self, this](TVMArgs args, TVMRetValue* rv) { *rv = this->variables_; });
+    } else if (name == "get_metadata") {
+      return PackedFunc(

Review comment:
       Okay, I left a TODO in the code for this. Let me give it a try in this PR.




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-tvm] zhiics commented on a change in pull request #5770: [RUNTIME] Introduce MetadataModule to separate code compilation/interpretation and weight initialization

Posted by GitBox <gi...@apache.org>.
zhiics commented on a change in pull request #5770:
URL: https://github.com/apache/incubator-tvm/pull/5770#discussion_r442490987



##########
File path: python/tvm/contrib/graph_runtime.py
##########
@@ -162,7 +162,12 @@ def set_input(self, key=None, value=None, **params):
             keys = list(params.keys())
             keys.sort(key=lambda x: -np.prod(params[x].shape))
             for k in keys:
-                self._get_input(k).copyfrom(params[k])
+                # TODO(zhiics) Skip the weights for submodule in a better way.
+                # We should use MetadataModule for initialization and remove

Review comment:
       Yes, there is a warning in the C++ side.
   
   https://github.com/apache/incubator-tvm/blob/d8c80c382f02052b07da1235190a5b6c7acea994/src/runtime/graph/graph_runtime.cc#L93




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-tvm] zhiics commented on a change in pull request #5770: [BYOC][runtime] Separate code and metadata for CSourceModule

Posted by GitBox <gi...@apache.org>.
zhiics commented on a change in pull request #5770:
URL: https://github.com/apache/incubator-tvm/pull/5770#discussion_r439527783



##########
File path: python/tvm/runtime/module.py
##########
@@ -33,6 +33,25 @@
 ProfileResult = namedtuple("ProfileResult", ["mean", "results"])
 
 
+def ModuleInitWrapper(variables, metadata):
+    """Create a module initialization wrapper.

Review comment:
       ah, I thought we want to allow users to play with these APIs in the frontend. NVM, I can hide them in the backend and some wrappers can be removed.




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-tvm] zhiics commented on a change in pull request #5770: [BYOC][runtime] Separate code and metadata for CSourceModule

Posted by GitBox <gi...@apache.org>.
zhiics commented on a change in pull request #5770:
URL: https://github.com/apache/incubator-tvm/pull/5770#discussion_r441870459



##########
File path: python/tvm/contrib/graph_runtime.py
##########
@@ -162,7 +162,11 @@ def set_input(self, key=None, value=None, **params):
             keys = list(params.keys())
             keys.sort(key=lambda x: -np.prod(params[x].shape))
             for k in keys:
-                self._get_input(k).copyfrom(params[k])
+                # TODO(zhiics) Skip the weights for submodule in a better way.
+                # We could get all inputs required by graphruntime first,
+                # we should use MetadataModule for initialization.

Review comment:
       yeah, that is something what I was thinking of as well. Let me spend some time to think about some possible ways when I get cycles. I will post an RFC with some plans because graph runtime affects apparently every downstream user. 




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-tvm] zhiics commented on a change in pull request #5770: [BYOC][runtime] Separate code and metadata for CSourceModule

Posted by GitBox <gi...@apache.org>.
zhiics commented on a change in pull request #5770:
URL: https://github.com/apache/incubator-tvm/pull/5770#discussion_r439757500



##########
File path: src/target/source/source_module.cc
##########
@@ -152,8 +153,92 @@ runtime::Module DeviceSourceModuleCreate(
   return runtime::Module(n);
 }
 
+// A helper used to wrap different types of modules and pass through packedfunc.
+// This module will never be used for compilation and execution.
+class ModuleClassWrapperNode : public runtime::ModuleNode {
+ public:
+  ModuleClassWrapperNode() = default;
+  const char* type_key() const { return "module_class_wrapper"; }
+  PackedFunc GetFunction(const std::string& name, const ObjectPtr<Object>& sptr_to_self) final {
+    LOG(FATAL) << "Cannot execute module wrapper";
+    return PackedFunc();
+  }
+};
+
+runtime::Module ModuleClassWrapperCreate() {
+  auto n = make_object<ModuleClassWrapperNode>();
+  return runtime::Module(n);
+}
+
+// Pack the source code and metadata, where source code could be any
+// user-defined code, i.e. c source code, json graph representation, etc.
+class SourceMetadataModuleNode final : public runtime::ModuleNode {
+ public:
+  SourceMetadataModuleNode(const String& func_symbol, const String& code, const String& source_type,
+                           const Array<String>& variables, const Array<runtime::NDArray>& metadata)
+      : func_symbol_(func_symbol),
+        code_(code),
+        source_type_(source_type),
+        variables_(variables),
+        metadata_(metadata) {}
+
+  PackedFunc GetFunction(const std::string& name, const ObjectPtr<Object>& sptr_to_self) final {
+    if (name == "get_source") {
+      return PackedFunc([sptr_to_self, this](TVMArgs args, TVMRetValue* rv) { *rv = this->code_; });
+    } else if (name == "get_source_type") {
+      return PackedFunc(
+          [sptr_to_self, this](TVMArgs args, TVMRetValue* rv) { *rv = this->source_type_; });
+    } else if (name == "get_symbol") {
+      return PackedFunc(
+          [sptr_to_self, this](TVMArgs args, TVMRetValue* rv) { *rv = this->func_symbol_; });
+    } else if (name == "get_vars") {
+      return PackedFunc(
+          [sptr_to_self, this](TVMArgs args, TVMRetValue* rv) { *rv = this->variables_; });
+    } else if (name == "get_metadata") {
+      return PackedFunc(

Review comment:
       @tqchen I refactored CSourceModule generator for subfunctions to initialize the submodules. Now I am working on this. But I have a few questions
   
   Suppose we have:
   ```
   MetadataModule:
       - Graphruntime LLVM module(uses `p0 = const0, p1 = const1`)
       - Accelerator CSourceModule0, foo_0 (uses `a = const1,  b = const2`)
       - Accelerator CSourceModule1, foo_1 (uses `c = const1, d = const3`)
   ```
   
   - How do we collect all required Metadata? Maybe we can collect them through graph_runtime_codegen/vmcompiler because the params (var: ndarray pairs) used by graphruntime/VM is from there. We can visit into other functions to collect for the other two submodules. 
   - How do we dispatch the correct constants to the corresponding submodule,  `InitFoo0(const1, const2)`? It looks to me that MetadataModule is not aware of any const to submodule mapping. I current stored all the variables and ndarrays. Should we also need to have each submodule store the required const_vars? By this, `MetadataModule` can query which ndarrays a submodule would require by looking at the list of `const_vars`. However, this would mean we need to have a `const_vars/params` field for different runtime modules.
   
   Am I missing thing? 




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-tvm] lhutton1 commented on a change in pull request #5770: [BYOC][runtime] Separate code and metadata for CSourceModule

Posted by GitBox <gi...@apache.org>.
lhutton1 commented on a change in pull request #5770:
URL: https://github.com/apache/incubator-tvm/pull/5770#discussion_r440167425



##########
File path: python/tvm/runtime/module.py
##########
@@ -222,21 +222,29 @@ def evaluator(*args):
         except NameError:
             raise NameError("time_evaluate is only supported when RPC is enabled")
 
-    def _collect_dso_modules(self):
-        """Helper function to collect dso modules, then return it."""
-        visited, stack, dso_modules = set(), [], []
+    def _collect_dso_metadata_modules(self):
+        """
+        Helper function to collect dso modules and metadata init module. There
+        is at most one medata init module if it exists.

Review comment:
       *metadata

##########
File path: python/tvm/runtime/module.py
##########
@@ -222,21 +222,29 @@ def evaluator(*args):
         except NameError:
             raise NameError("time_evaluate is only supported when RPC is enabled")
 
-    def _collect_dso_modules(self):
-        """Helper function to collect dso modules, then return it."""
-        visited, stack, dso_modules = set(), [], []
+    def _collect_dso_metadata_modules(self):
+        """
+        Helper function to collect dso modules and metadata init module. There
+        is at most one medata init module if it exists.
+        """
+        visited, stack, dso_modules, metadata_init = set(), [], [], None
         # append root module
         visited.add(self)
         stack.append(self)
         while stack:
             module = stack.pop()
             if module._dso_exportable():
                 dso_modules.append(module)
+            elif module.type_key == "module_init":
+                assert not metadata_init, \
+                        "At most one module initializer is allowed"

Review comment:
       How does this interact in the case of multiple backends that require the use of metadata module? For example it may be the case in the future that we want to specify something like: DNNL (first priority), then C-Codegen (second priority), then TVM (fallback). If only one metadata module is allowed, would this contain information for both DNNL and C-Codegen?




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-tvm] zhiics commented on a change in pull request #5770: [BYOC][runtime] Separate code and metadata for CSourceModule

Posted by GitBox <gi...@apache.org>.
zhiics commented on a change in pull request #5770:
URL: https://github.com/apache/incubator-tvm/pull/5770#discussion_r441870459



##########
File path: python/tvm/contrib/graph_runtime.py
##########
@@ -162,7 +162,11 @@ def set_input(self, key=None, value=None, **params):
             keys = list(params.keys())
             keys.sort(key=lambda x: -np.prod(params[x].shape))
             for k in keys:
-                self._get_input(k).copyfrom(params[k])
+                # TODO(zhiics) Skip the weights for submodule in a better way.
+                # We could get all inputs required by graphruntime first,
+                # we should use MetadataModule for initialization.

Review comment:
       yeah, that is something what I was thinking of as well. Let me spend some time to think about some possible ways when I get some cycles. I will post an RFC with some plans because graph runtime affects apparently every downstream users. 




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-tvm] zhiics commented on a change in pull request #5770: [BYOC][runtime] Separate code and metadata for CSourceModule

Posted by GitBox <gi...@apache.org>.
zhiics commented on a change in pull request #5770:
URL: https://github.com/apache/incubator-tvm/pull/5770#discussion_r439670048



##########
File path: src/runtime/module_init_wrapper.cc
##########
@@ -0,0 +1,234 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*!
+ * \file src/runtime/module_init_wrapper.cc
+ * \brief A wrapper for initializing modules using metadata
+ */
+#include <tvm/node/container.h>
+#include <tvm/runtime/ndarray.h>
+#include <tvm/runtime/packed_func.h>
+#include <tvm/runtime/registry.h>
+
+#include <cstdint>
+#include <sstream>
+
+#include "file_util.h"
+
+namespace tvm {
+namespace runtime {
+
+using StringNDArrayMap = std::unordered_map<String, runtime::NDArray, ObjectHash, ObjectEqual>;
+
+class CSourceMetadataInitializer {
+ public:
+  explicit CSourceMetadataInitializer(const StringNDArrayMap& metadata) : metadata_(metadata) {}
+
+  template <typename T>

Review comment:
       This should be possible. Let me spend some time to refactor the c source code generator over the weekend.




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-tvm] zhiics commented on pull request #5770: [BYOC][runtime] Separate code and metadata for CSourceModule

Posted by GitBox <gi...@apache.org>.
zhiics commented on pull request #5770:
URL: https://github.com/apache/incubator-tvm/pull/5770#issuecomment-645177764


   @FrozenGene The current implementation doesn't actually change any user/frontend APIs. We added the `MetadataModule` to take care of weight initialization and separate it from code compilation. So you would have:
   
   ```
   MetadataModule:
       - DSO module
       - Accelerator CSourceModule0
       - Accelerator CSourceModule1
   ```
   
   As you can see, `MetadataModule` manages the params, but we will still have code in the submodules. The serialization and deserialization make sure that the module structure is always maintained correctly. Therefore, we can still have params/lib/json in one module. 
   
   In other words, it should not affect the way we package the final artifacts, but it changes the way we compile certain modules (e.g. CSourceModule) and the way we initialize modules that need constant data.
   
   


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-tvm] zhiics commented on a change in pull request #5770: [BYOC][runtime] Separate code and metadata for CSourceModule

Posted by GitBox <gi...@apache.org>.
zhiics commented on a change in pull request #5770:
URL: https://github.com/apache/incubator-tvm/pull/5770#discussion_r439203922



##########
File path: src/relay/backend/contrib/dnnl/codegen.cc
##########
@@ -392,22 +381,46 @@ class DNNLModuleCodegen : public CSourceModuleCodegenBase {
     code_stream_ << "using namespace tvm::runtime::contrib;\n";
     code_stream_ << "\n";
 
+    String func_symbol("all");

Review comment:
       I think it is okay that this is code a bit redundant to the one in c_codegen, because we are planning to remove this in the followup PRs and replace it with a json runtime.




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-tvm] tqchen commented on a change in pull request #5770: [BYOC][runtime] Separate code and metadata for CSourceModule

Posted by GitBox <gi...@apache.org>.
tqchen commented on a change in pull request #5770:
URL: https://github.com/apache/incubator-tvm/pull/5770#discussion_r439592861



##########
File path: src/runtime/module_init_wrapper.cc
##########
@@ -0,0 +1,234 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*!
+ * \file src/runtime/module_init_wrapper.cc
+ * \brief A wrapper for initializing modules using metadata
+ */
+#include <tvm/node/container.h>
+#include <tvm/runtime/ndarray.h>
+#include <tvm/runtime/packed_func.h>
+#include <tvm/runtime/registry.h>
+
+#include <cstdint>
+#include <sstream>
+
+#include "file_util.h"
+
+namespace tvm {
+namespace runtime {
+
+using StringNDArrayMap = std::unordered_map<String, runtime::NDArray, ObjectHash, ObjectEqual>;
+
+class CSourceMetadataInitializer {
+ public:
+  explicit CSourceMetadataInitializer(const StringNDArrayMap& metadata) : metadata_(metadata) {}
+
+  template <typename T>

Review comment:
       Do we really need this one? My guess is that we can return MetaInitWrapper(that wraps CSourceModule) and our module serialization mechanism should take care of loading the meta-data back, without having to serialie the meta data to the C source.

##########
File path: src/runtime/module_init_wrapper.cc
##########
@@ -0,0 +1,234 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*!
+ * \file src/runtime/module_init_wrapper.cc
+ * \brief A wrapper for initializing modules using metadata
+ */
+#include <tvm/node/container.h>
+#include <tvm/runtime/ndarray.h>
+#include <tvm/runtime/packed_func.h>
+#include <tvm/runtime/registry.h>
+
+#include <cstdint>
+#include <sstream>
+
+#include "file_util.h"
+
+namespace tvm {
+namespace runtime {
+
+using StringNDArrayMap = std::unordered_map<String, runtime::NDArray, ObjectHash, ObjectEqual>;
+
+class CSourceMetadataInitializer {
+ public:
+  explicit CSourceMetadataInitializer(const StringNDArrayMap& metadata) : metadata_(metadata) {}
+
+  template <typename T>

Review comment:
       see also https://tvm.apache.org/docs/dev/introduction_to_module_serialization.html




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-tvm] zhiics commented on a change in pull request #5770: [BYOC][runtime] Separate code and metadata for CSourceModule

Posted by GitBox <gi...@apache.org>.
zhiics commented on a change in pull request #5770:
URL: https://github.com/apache/incubator-tvm/pull/5770#discussion_r441870459



##########
File path: python/tvm/contrib/graph_runtime.py
##########
@@ -162,7 +162,11 @@ def set_input(self, key=None, value=None, **params):
             keys = list(params.keys())
             keys.sort(key=lambda x: -np.prod(params[x].shape))
             for k in keys:
-                self._get_input(k).copyfrom(params[k])
+                # TODO(zhiics) Skip the weights for submodule in a better way.
+                # We could get all inputs required by graphruntime first,
+                # we should use MetadataModule for initialization.

Review comment:
       yeah, that is something what I was thinking of as well. Let me spend some time to think about some possible ways when I get some cycles. I will post an RFC with some plans because graph runtime affects apparently every downstream user. 




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-tvm] FrozenGene commented on pull request #5770: [BYOC][runtime] Separate code and metadata for CSourceModule

Posted by GitBox <gi...@apache.org>.
FrozenGene commented on pull request #5770:
URL: https://github.com/apache/incubator-tvm/pull/5770#issuecomment-645171944


   Maybe I miss something. so maybe this is considered. That is I don't see `unwrap_modules`. I ask this because I think it is related with our next module based interface runtime. The usage like [this](https://github.com/apache/incubator-tvm/pull/5753/files#diff-82bcc5ddf3cb06075848655807dfcbd5R136-R157):
   ```python
   with relay.build_config(opt_level=3):
       complied_graph_lib = relay.build_module.build(
           mod, "llvm", params=params)
   
   from tvm.contrib import util
   temp = util.tempdir()
   file_name = "deploy_lib.so"
   path_lib = temp.relpath(file_name)
   complied_graph_lib.export_library(path_lib)
   loaded_lib = tvm.runtime.load_module(path_lib)
   ctx = tvm.cpu(0)
   gmod = loaded_lib['default'](ctx)
   set_input = gmod["set_input"]
   run = gmod["run"]
   get_output = gmod["get_output"]
   data = np.random.uniform(-1, 1, size=(1, 3, 224, 224)).astype("float32")
   set_input("data", tvm.nd.array(data))
   run()
   out = get_output(0).asnumpy()
   ```
   As you could see we will wrap `params` / `lib` / `json` into one lib. So I want to see the logic of `unwrap_modules` and to see how to connect with it. 
   
   THANKS!


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-tvm] tqchen commented on a change in pull request #5770: [BYOC][runtime] Separate code and metadata for CSourceModule

Posted by GitBox <gi...@apache.org>.
tqchen commented on a change in pull request #5770:
URL: https://github.com/apache/incubator-tvm/pull/5770#discussion_r441826518



##########
File path: python/tvm/contrib/graph_runtime.py
##########
@@ -162,7 +162,11 @@ def set_input(self, key=None, value=None, **params):
             keys = list(params.keys())
             keys.sort(key=lambda x: -np.prod(params[x].shape))
             for k in keys:
-                self._get_input(k).copyfrom(params[k])
+                # TODO(zhiics) Skip the weights for submodule in a better way.
+                # We could get all inputs required by graphruntime first,
+                # we should use MetadataModule for initialization.

Review comment:
       This part of the logic seems to be a bit too hacky, cryptic Shall we simply skip inside the set_params instead? If the weights are emedded in the metadata, do we still allow overriding of weights?




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-tvm] zhiics commented on a change in pull request #5770: [BYOC][runtime] Separate code and metadata for CSourceModule

Posted by GitBox <gi...@apache.org>.
zhiics commented on a change in pull request #5770:
URL: https://github.com/apache/incubator-tvm/pull/5770#discussion_r441850480



##########
File path: python/tvm/contrib/graph_runtime.py
##########
@@ -162,7 +162,11 @@ def set_input(self, key=None, value=None, **params):
             keys = list(params.keys())
             keys.sort(key=lambda x: -np.prod(params[x].shape))
             for k in keys:
-                self._get_input(k).copyfrom(params[k])
+                # TODO(zhiics) Skip the weights for submodule in a better way.
+                # We could get all inputs required by graphruntime first,
+                # we should use MetadataModule for initialization.

Review comment:
       Yeah, we can do some checking at the C++ side as well. We will need to change graph runtime later to let it have a "__init" function so that we can initialize it with params. And we will need to remove some of the current ways to set the params up. For example, the `set_input`  API should not take `**params` and the chunk of hacky code will be removed. That needs some more thinking and may worth an RFC. 




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-tvm] zhiics commented on a change in pull request #5770: [BYOC][runtime] Separate code and metadata for CSourceModule

Posted by GitBox <gi...@apache.org>.
zhiics commented on a change in pull request #5770:
URL: https://github.com/apache/incubator-tvm/pull/5770#discussion_r439595480



##########
File path: src/runtime/module_init_wrapper.cc
##########
@@ -0,0 +1,234 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*!
+ * \file src/runtime/module_init_wrapper.cc
+ * \brief A wrapper for initializing modules using metadata
+ */
+#include <tvm/node/container.h>
+#include <tvm/runtime/ndarray.h>
+#include <tvm/runtime/packed_func.h>
+#include <tvm/runtime/registry.h>
+
+#include <cstdint>
+#include <sstream>
+
+#include "file_util.h"
+
+namespace tvm {
+namespace runtime {
+
+using StringNDArrayMap = std::unordered_map<String, runtime::NDArray, ObjectHash, ObjectEqual>;
+
+class CSourceMetadataInitializer {
+ public:
+  explicit CSourceMetadataInitializer(const StringNDArrayMap& metadata) : metadata_(metadata) {}
+
+  template <typename T>

Review comment:
       This is needed for partitioned subgraphs because they expect the plain c source data as the input. 




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-tvm] tqchen commented on a change in pull request #5770: [RUNTIME] Introduce MetadataModule to separate code compilation/interpretation and weight initialization

Posted by GitBox <gi...@apache.org>.
tqchen commented on a change in pull request #5770:
URL: https://github.com/apache/incubator-tvm/pull/5770#discussion_r442477655



##########
File path: python/tvm/contrib/graph_runtime.py
##########
@@ -162,7 +162,12 @@ def set_input(self, key=None, value=None, **params):
             keys = list(params.keys())
             keys.sort(key=lambda x: -np.prod(params[x].shape))
             for k in keys:
-                self._get_input(k).copyfrom(params[k])
+                # TODO(zhiics) Skip the weights for submodule in a better way.
+                # We should use MetadataModule for initialization and remove

Review comment:
       Shall we shoot an warning at least if val is not available?




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-tvm] zhiics commented on a change in pull request #5770: [BYOC][runtime] Separate code and metadata for CSourceModule

Posted by GitBox <gi...@apache.org>.
zhiics commented on a change in pull request #5770:
URL: https://github.com/apache/incubator-tvm/pull/5770#discussion_r439595480



##########
File path: src/runtime/module_init_wrapper.cc
##########
@@ -0,0 +1,234 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*!
+ * \file src/runtime/module_init_wrapper.cc
+ * \brief A wrapper for initializing modules using metadata
+ */
+#include <tvm/node/container.h>
+#include <tvm/runtime/ndarray.h>
+#include <tvm/runtime/packed_func.h>
+#include <tvm/runtime/registry.h>
+
+#include <cstdint>
+#include <sstream>
+
+#include "file_util.h"
+
+namespace tvm {
+namespace runtime {
+
+using StringNDArrayMap = std::unordered_map<String, runtime::NDArray, ObjectHash, ObjectEqual>;
+
+class CSourceMetadataInitializer {
+ public:
+  explicit CSourceMetadataInitializer(const StringNDArrayMap& metadata) : metadata_(metadata) {}
+
+  template <typename T>

Review comment:
       This is needed for partitioned subgraphs because they expect the plain c source data as the input to compile together with the C source code.




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-tvm] tqchen commented on a change in pull request #5770: [BYOC][runtime] Separate code and metadata for CSourceModule

Posted by GitBox <gi...@apache.org>.
tqchen commented on a change in pull request #5770:
URL: https://github.com/apache/incubator-tvm/pull/5770#discussion_r439593516



##########
File path: src/target/source/source_module.cc
##########
@@ -152,8 +153,92 @@ runtime::Module DeviceSourceModuleCreate(
   return runtime::Module(n);
 }
 
+// A helper used to wrap different types of modules and pass through packedfunc.
+// This module will never be used for compilation and execution.
+class ModuleClassWrapperNode : public runtime::ModuleNode {
+ public:
+  ModuleClassWrapperNode() = default;
+  const char* type_key() const { return "module_class_wrapper"; }
+  PackedFunc GetFunction(const std::string& name, const ObjectPtr<Object>& sptr_to_self) final {
+    LOG(FATAL) << "Cannot execute module wrapper";
+    return PackedFunc();
+  }
+};
+
+runtime::Module ModuleClassWrapperCreate() {
+  auto n = make_object<ModuleClassWrapperNode>();
+  return runtime::Module(n);
+}
+
+// Pack the source code and metadata, where source code could be any
+// user-defined code, i.e. c source code, json graph representation, etc.
+class SourceMetadataModuleNode final : public runtime::ModuleNode {
+ public:
+  SourceMetadataModuleNode(const String& func_symbol, const String& code, const String& source_type,
+                           const Array<String>& variables, const Array<runtime::NDArray>& metadata)
+      : func_symbol_(func_symbol),
+        code_(code),
+        source_type_(source_type),
+        variables_(variables),
+        metadata_(metadata) {}
+
+  PackedFunc GetFunction(const std::string& name, const ObjectPtr<Object>& sptr_to_self) final {
+    if (name == "get_source") {
+      return PackedFunc([sptr_to_self, this](TVMArgs args, TVMRetValue* rv) { *rv = this->code_; });
+    } else if (name == "get_source_type") {
+      return PackedFunc(
+          [sptr_to_self, this](TVMArgs args, TVMRetValue* rv) { *rv = this->source_type_; });
+    } else if (name == "get_symbol") {
+      return PackedFunc(
+          [sptr_to_self, this](TVMArgs args, TVMRetValue* rv) { *rv = this->func_symbol_; });
+    } else if (name == "get_vars") {
+      return PackedFunc(
+          [sptr_to_self, this](TVMArgs args, TVMRetValue* rv) { *rv = this->variables_; });
+    } else if (name == "get_metadata") {
+      return PackedFunc(

Review comment:
       It would be great to see if we can reduce to only MetaDataModule, and reuse mechanism here https://tvm.apache.org/docs/dev/introduction_to_module_serialization.html for serialization and packaging.




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-tvm] zhiics commented on pull request #5770: [BYOC][runtime] Separate code and metadata for CSourceModule

Posted by GitBox <gi...@apache.org>.
zhiics commented on pull request #5770:
URL: https://github.com/apache/incubator-tvm/pull/5770#issuecomment-643348640


   cc @tqchen @comaniac @junrushao1994 @FrozenGene @lhutton1 @trevor-m @masahi 


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-tvm] zhiics commented on a change in pull request #5770: [BYOC][runtime] Separate code and metadata for CSourceModule

Posted by GitBox <gi...@apache.org>.
zhiics commented on a change in pull request #5770:
URL: https://github.com/apache/incubator-tvm/pull/5770#discussion_r440266641



##########
File path: python/tvm/runtime/module.py
##########
@@ -222,21 +222,29 @@ def evaluator(*args):
         except NameError:
             raise NameError("time_evaluate is only supported when RPC is enabled")
 
-    def _collect_dso_modules(self):
-        """Helper function to collect dso modules, then return it."""
-        visited, stack, dso_modules = set(), [], []
+    def _collect_dso_metadata_modules(self):
+        """
+        Helper function to collect dso modules and metadata init module. There
+        is at most one medata init module if it exists.
+        """
+        visited, stack, dso_modules, metadata_init = set(), [], [], None
         # append root module
         visited.add(self)
         stack.append(self)
         while stack:
             module = stack.pop()
             if module._dso_exportable():
                 dso_modules.append(module)
+            elif module.type_key == "module_init":
+                assert not metadata_init, \
+                        "At most one module initializer is allowed"

Review comment:
       The python side changes will be all removed. We have a MetadataModule to dispatch the initialization. We don't need to consider about priority because they symbols are unique.




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-tvm] lhutton1 commented on a change in pull request #5770: [BYOC][runtime] Separate code and metadata for CSourceModule

Posted by GitBox <gi...@apache.org>.
lhutton1 commented on a change in pull request #5770:
URL: https://github.com/apache/incubator-tvm/pull/5770#discussion_r440275397



##########
File path: python/tvm/runtime/module.py
##########
@@ -222,21 +222,29 @@ def evaluator(*args):
         except NameError:
             raise NameError("time_evaluate is only supported when RPC is enabled")
 
-    def _collect_dso_modules(self):
-        """Helper function to collect dso modules, then return it."""
-        visited, stack, dso_modules = set(), [], []
+    def _collect_dso_metadata_modules(self):
+        """
+        Helper function to collect dso modules and metadata init module. There
+        is at most one medata init module if it exists.
+        """
+        visited, stack, dso_modules, metadata_init = set(), [], [], None
         # append root module
         visited.add(self)
         stack.append(self)
         while stack:
             module = stack.pop()
             if module._dso_exportable():
                 dso_modules.append(module)
+            elif module.type_key == "module_init":
+                assert not metadata_init, \
+                        "At most one module initializer is allowed"

Review comment:
       Makes sense thanks, I was thinking one MetadataModule would be created for each backend.




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-tvm] FrozenGene commented on a change in pull request #5770: [BYOC][runtime] Separate code and metadata for CSourceModule

Posted by GitBox <gi...@apache.org>.
FrozenGene commented on a change in pull request #5770:
URL: https://github.com/apache/incubator-tvm/pull/5770#discussion_r441298275



##########
File path: python/tvm/contrib/graph_runtime.py
##########
@@ -162,7 +162,11 @@ def set_input(self, key=None, value=None, **params):
             keys = list(params.keys())
             keys.sort(key=lambda x: -np.prod(params[x].shape))
             for k in keys:
-                self._get_input(k).copyfrom(params[k])
+                # TODO(zhiics) Skip the weights for submodule in a better way.
+                # We could get all inputs required by graphruntime first,
+                # we should use MetadataModule for initialization.
+                if "_const_" not in k:

Review comment:
       Is `_const_` only existed for external module?




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-tvm] zhiics commented on pull request #5770: [BYOC][runtime] Separate code and metadata for CSourceModule

Posted by GitBox <gi...@apache.org>.
zhiics commented on pull request #5770:
URL: https://github.com/apache/incubator-tvm/pull/5770#issuecomment-644899553


   @tqchen I removed all wrappers, made `MetadataModule` internal, and dispatched the initialization of each submodule per function/symbol base. There is now no user API change. But I had to add a field the CSourceModule to let it know what metadata it needs. This, however, doesn't affect the compilation and the original use/semantic of `CSourceModule` because we only need this information to help build up the `MetadataModule`. 


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-tvm] zhiics commented on a change in pull request #5770: [BYOC][runtime] Separate code and metadata for CSourceModule

Posted by GitBox <gi...@apache.org>.
zhiics commented on a change in pull request #5770:
URL: https://github.com/apache/incubator-tvm/pull/5770#discussion_r441870459



##########
File path: python/tvm/contrib/graph_runtime.py
##########
@@ -162,7 +162,11 @@ def set_input(self, key=None, value=None, **params):
             keys = list(params.keys())
             keys.sort(key=lambda x: -np.prod(params[x].shape))
             for k in keys:
-                self._get_input(k).copyfrom(params[k])
+                # TODO(zhiics) Skip the weights for submodule in a better way.
+                # We could get all inputs required by graphruntime first,
+                # we should use MetadataModule for initialization.

Review comment:
       yeah, that is something what I was thinking of as well. Let me spend some time to think about some possible ways. I will post an RFC with some plans because graph runtime affects apparently every downstream users. 




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-tvm] comaniac commented on a change in pull request #5770: [BYOC][runtime] Separate code and metadata for CSourceModule

Posted by GitBox <gi...@apache.org>.
comaniac commented on a change in pull request #5770:
URL: https://github.com/apache/incubator-tvm/pull/5770#discussion_r441748628



##########
File path: src/relay/backend/contrib/codegen_c/codegen_c.h
##########
@@ -299,6 +301,55 @@ class CodegenCBase {
     return dtype;
   }
 
+  /*!
+   * \brief Creates a checker to check if the NDArray pool is initialized
+   *
+   * \param symobl The Symbol of the current function
+   *
+   * \return The created checker
+   */
+  std::string CreateInitChecker(const std::string& symbol) const {
+    std::ostringstream oss;
+    oss << "CHECK(!" << symbol
+        << "_consts.empty()) << \"DNNL source module hasn't been initialized.\";\n";

Review comment:
       s/DNNL/C/




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-tvm] tqchen commented on a change in pull request #5770: [BYOC][runtime] Separate code and metadata for CSourceModule

Posted by GitBox <gi...@apache.org>.
tqchen commented on a change in pull request #5770:
URL: https://github.com/apache/incubator-tvm/pull/5770#discussion_r441869481



##########
File path: python/tvm/contrib/graph_runtime.py
##########
@@ -162,7 +162,11 @@ def set_input(self, key=None, value=None, **params):
             keys = list(params.keys())
             keys.sort(key=lambda x: -np.prod(params[x].shape))
             for k in keys:
-                self._get_input(k).copyfrom(params[k])
+                # TODO(zhiics) Skip the weights for submodule in a better way.
+                # We could get all inputs required by graphruntime first,
+                # we should use MetadataModule for initialization.

Review comment:
       Perhaps it is possible to automatically call the `__init` function in module construction and binary loading time ?




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-tvm] zhiics commented on a change in pull request #5770: [BYOC][runtime] Separate code and metadata for CSourceModule

Posted by GitBox <gi...@apache.org>.
zhiics commented on a change in pull request #5770:
URL: https://github.com/apache/incubator-tvm/pull/5770#discussion_r439528514



##########
File path: src/runtime/module_init_wrapper.cc
##########
@@ -0,0 +1,234 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*!
+ * \file src/runtime/module_init_wrapper.cc
+ * \brief A wrapper for initializing modules using metadata
+ */
+#include <tvm/node/container.h>
+#include <tvm/runtime/ndarray.h>
+#include <tvm/runtime/packed_func.h>
+#include <tvm/runtime/registry.h>
+
+#include <cstdint>
+#include <sstream>
+
+#include "file_util.h"
+
+namespace tvm {
+namespace runtime {
+
+using StringNDArrayMap = std::unordered_map<String, runtime::NDArray, ObjectHash, ObjectEqual>;
+
+class CSourceMetadataInitializer {
+ public:
+  explicit CSourceMetadataInitializer(const StringNDArrayMap& metadata) : metadata_(metadata) {}
+
+  template <typename T>
+  void GetElements(const std::string& var_name, const std::string& type_name,
+                   const runtime::NDArray& arr) {
+    // Get the number of elements.
+    int64_t num_elems = 1;
+    for (auto i : arr.Shape()) num_elems *= i;
+    stream_ << "static " << type_name << " " << var_name << "[" << num_elems << "] = {";
+    T* ptr = static_cast<T*>(arr->data);
+    for (int64_t i = 0; i < num_elems - 1; i++) {
+      stream_ << ptr[i] << ",";
+    }
+    if (num_elems > 0) stream_ << ptr[num_elems - 1];
+    stream_ << "};\n";
+  }
+
+  std::string Init() {
+    for (const auto& it : metadata_) {
+      std::string var_name = it.first.operator std::string();
+      runtime::NDArray data = it.second;
+      CHECK_EQ(data->dtype.lanes, 1U);
+      if (data->dtype.code == kDLFloat) {
+        if (data->dtype.bits == 32) {
+          stream_.precision(std::numeric_limits<float>::digits10 + 1);
+          GetElements<float>(var_name, "float", data);
+        } else {
+          CHECK_EQ(data->dtype.bits, 64);
+          stream_.precision(std::numeric_limits<double>::digits10 + 1);
+          GetElements<double>(var_name, "double", data);
+        }
+      } else if (data->dtype.code == kDLUInt) {
+        if (data->dtype.bits == 8) {
+          GetElements<uint8_t>(var_name, "uint8_t", data);
+        } else {
+          CHECK_EQ(data->dtype.bits, 32);
+          GetElements<uint32_t>(var_name, "uint32_t", data);
+        }
+      } else {
+        if (data->dtype.bits == 8) {
+          GetElements<int8_t>(var_name, "int8_t", data);
+        } else {
+          CHECK_EQ(data->dtype.bits, 32);
+          GetElements<int32_t>(var_name, "int32_t", data);
+        }
+      }
+    }
+    return stream_.str();
+  }
+
+ private:
+  /*! \brief The stream to print constant data. */
+  std::ostringstream stream_;
+  /*! \brief variable name to NDArray mapping. */
+  StringNDArrayMap metadata_;
+};
+
+class ModuleInitWrapper : public runtime::ModuleNode {

Review comment:
       how about `MetadataInitModule` or just `MetadataModule`?




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-tvm] tqchen merged pull request #5770: [RUNTIME] Introduce MetadataModule to separate code compilation/interpretation and weight initialization

Posted by GitBox <gi...@apache.org>.
tqchen merged pull request #5770:
URL: https://github.com/apache/incubator-tvm/pull/5770


   


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-tvm] tqchen commented on a change in pull request #5770: [BYOC][runtime] Separate code and metadata for CSourceModule

Posted by GitBox <gi...@apache.org>.
tqchen commented on a change in pull request #5770:
URL: https://github.com/apache/incubator-tvm/pull/5770#discussion_r439596323



##########
File path: src/runtime/module_init_wrapper.cc
##########
@@ -0,0 +1,234 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*!
+ * \file src/runtime/module_init_wrapper.cc
+ * \brief A wrapper for initializing modules using metadata
+ */
+#include <tvm/node/container.h>
+#include <tvm/runtime/ndarray.h>
+#include <tvm/runtime/packed_func.h>
+#include <tvm/runtime/registry.h>
+
+#include <cstdint>
+#include <sstream>
+
+#include "file_util.h"
+
+namespace tvm {
+namespace runtime {
+
+using StringNDArrayMap = std::unordered_map<String, runtime::NDArray, ObjectHash, ObjectEqual>;
+
+class CSourceMetadataInitializer {
+ public:
+  explicit CSourceMetadataInitializer(const StringNDArrayMap& metadata) : metadata_(metadata) {}
+
+  template <typename T>

Review comment:
       Can we change the subgraph generator to generate `InitMod0Func0(Array<NDArray>)` instead? so that we don't have to compile the meta data into the C source?




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-tvm] zhiics commented on a change in pull request #5770: [BYOC][runtime] Separate code and metadata for CSourceModule

Posted by GitBox <gi...@apache.org>.
zhiics commented on a change in pull request #5770:
URL: https://github.com/apache/incubator-tvm/pull/5770#discussion_r439757500



##########
File path: src/target/source/source_module.cc
##########
@@ -152,8 +153,92 @@ runtime::Module DeviceSourceModuleCreate(
   return runtime::Module(n);
 }
 
+// A helper used to wrap different types of modules and pass through packedfunc.
+// This module will never be used for compilation and execution.
+class ModuleClassWrapperNode : public runtime::ModuleNode {
+ public:
+  ModuleClassWrapperNode() = default;
+  const char* type_key() const { return "module_class_wrapper"; }
+  PackedFunc GetFunction(const std::string& name, const ObjectPtr<Object>& sptr_to_self) final {
+    LOG(FATAL) << "Cannot execute module wrapper";
+    return PackedFunc();
+  }
+};
+
+runtime::Module ModuleClassWrapperCreate() {
+  auto n = make_object<ModuleClassWrapperNode>();
+  return runtime::Module(n);
+}
+
+// Pack the source code and metadata, where source code could be any
+// user-defined code, i.e. c source code, json graph representation, etc.
+class SourceMetadataModuleNode final : public runtime::ModuleNode {
+ public:
+  SourceMetadataModuleNode(const String& func_symbol, const String& code, const String& source_type,
+                           const Array<String>& variables, const Array<runtime::NDArray>& metadata)
+      : func_symbol_(func_symbol),
+        code_(code),
+        source_type_(source_type),
+        variables_(variables),
+        metadata_(metadata) {}
+
+  PackedFunc GetFunction(const std::string& name, const ObjectPtr<Object>& sptr_to_self) final {
+    if (name == "get_source") {
+      return PackedFunc([sptr_to_self, this](TVMArgs args, TVMRetValue* rv) { *rv = this->code_; });
+    } else if (name == "get_source_type") {
+      return PackedFunc(
+          [sptr_to_self, this](TVMArgs args, TVMRetValue* rv) { *rv = this->source_type_; });
+    } else if (name == "get_symbol") {
+      return PackedFunc(
+          [sptr_to_self, this](TVMArgs args, TVMRetValue* rv) { *rv = this->func_symbol_; });
+    } else if (name == "get_vars") {
+      return PackedFunc(
+          [sptr_to_self, this](TVMArgs args, TVMRetValue* rv) { *rv = this->variables_; });
+    } else if (name == "get_metadata") {
+      return PackedFunc(

Review comment:
       @tqchen I refactored CSourceModule generator for subfunctions to initialize the submodules. Now I am working on this. But I have a few questions
   
   Suppose we have:
   ```
   MetadataModule:
       - Graphruntime LLVM module(uses p0 = const0, p1 = const1)
       - Accelerator CSourceModule0, foo_0 (uses a = const1,  b = const2)
       - Accelerator CSourceModule1, foo_1 (uses c = const1, d = const3)
   ```
   
   - How do we collect all required Metadata? Maybe we can collect them through graph_runtime_codegen/vmcompiler because the params (var: ndarray pairs) used by graphruntime/VM is from there. We can visit into other functions to collect for the other two submodules. 
   - How do we dispatch the correct constants to the corresponding submodule,  `InitFoo0(const1, const2)`? It looks to me that MetadataModule is not aware of any const to submodule mapping. I current stored all the variables and ndarrays. Should we also need to have each submodule store the required const_vars? By this, `MetadataModule` can query which ndarrays a submodule would require by looking at the list of `const_vars`. However, this would mean we need to have a `const_vars/params` field for different runtime modules.
   
   Am I missing anything? 




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-tvm] zhiics commented on a change in pull request #5770: [BYOC][runtime] Separate code and metadata for CSourceModule

Posted by GitBox <gi...@apache.org>.
zhiics commented on a change in pull request #5770:
URL: https://github.com/apache/incubator-tvm/pull/5770#discussion_r440266641



##########
File path: python/tvm/runtime/module.py
##########
@@ -222,21 +222,29 @@ def evaluator(*args):
         except NameError:
             raise NameError("time_evaluate is only supported when RPC is enabled")
 
-    def _collect_dso_modules(self):
-        """Helper function to collect dso modules, then return it."""
-        visited, stack, dso_modules = set(), [], []
+    def _collect_dso_metadata_modules(self):
+        """
+        Helper function to collect dso modules and metadata init module. There
+        is at most one medata init module if it exists.
+        """
+        visited, stack, dso_modules, metadata_init = set(), [], [], None
         # append root module
         visited.add(self)
         stack.append(self)
         while stack:
             module = stack.pop()
             if module._dso_exportable():
                 dso_modules.append(module)
+            elif module.type_key == "module_init":
+                assert not metadata_init, \
+                        "At most one module initializer is allowed"

Review comment:
       The python side changes will be all removed.




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-tvm] comaniac commented on a change in pull request #5770: [BYOC][runtime] Separate code and metadata for CSourceModule

Posted by GitBox <gi...@apache.org>.
comaniac commented on a change in pull request #5770:
URL: https://github.com/apache/incubator-tvm/pull/5770#discussion_r441694863



##########
File path: src/relay/backend/contrib/codegen_c/codegen.cc
##########
@@ -76,43 +77,31 @@ class CodegenC : public MemoizedExprTranslator<std::vector<Output>>, public Code
   }
 
   std::vector<Output> VisitExpr_(const ConstantNode* cn) final {
-    // Note this is for demonstration purpose. ConstantNode doesn't necessarily
-    // belong to calls. We need to revisit this when tuples come into play.
-
     std::ostringstream decl_stream;
     std::ostringstream buf_stream;
 
     Output output;
-    output.name = "const_" + std::to_string(const_idx_++);
-
-    runtime::NDArray array = cn->data;
-    const auto& shape = array.Shape();
-
-    // Get the number of elements.
-    int64_t num_elems = 1;
-    for (auto i : shape) num_elems *= i;
-
+    // Get const: static_cast<float*>(dnnl_0_consts[0]->data)

Review comment:
       s/dnnl_0/gcc_0/

##########
File path: src/relay/backend/compile_engine.cc
##########
@@ -38,6 +38,7 @@
 #include <tvm/te/schedule.h>
 #include <tvm/te/schedule_pass.h>
 
+#include <cstdio>

Review comment:
       Remove this line?

##########
File path: src/relay/backend/contrib/codegen_c/codegen_c.h
##########
@@ -190,7 +209,12 @@ class CodegenCBase {
    */
   std::string JitImpl(const std::string& ext_func_id, const Array<Var>& args,
                       const std::vector<std::string>& buf_decl,
-                      const std::vector<std::string>& body, const std::vector<Output>& outs) {
+                      const std::vector<std::string>& body, const std::string& const_arr,

Review comment:
       `const_arr` seems a bit confusion. Maybe `const_arr_name` or something like that. (ditto in some other places)

##########
File path: src/runtime/metadata_module.cc
##########
@@ -0,0 +1,220 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*!
+ * \file src/runtime/metadata_module.cc
+ * \brief A wrapper for initializing imported modules using metadata. This
+ * module is intended to be used by various runtime in the TVM stack, i.e.
+ * graph runtime, relay VM, AOT runtime, and various user defined runtimes. It
+ * paves the way to separate the code and metedata, which makes compilation
+ * and/or interpretation more convenient. In addition, the clear separation of
+ * code and metadata significantly reduces the efforts for handling external
+ * codegen and runtimes.
+ */
+#include <tvm/node/container.h>
+#include <tvm/runtime/ndarray.h>
+#include <tvm/runtime/packed_func.h>
+#include <tvm/runtime/registry.h>
+
+#include <cstdint>
+#include <sstream>
+
+#include "meta_data.h"
+
+namespace tvm {
+namespace runtime {
+
+/*!
+ * \brief The metadata module is designed to manage initialization of the
+ * imported submodules.
+ */
+class MetadataModuleNode : public ModuleNode {
+ public:
+  MetadataModuleNode(const std::unordered_map<std::string, NDArray>& metadata,
+                     const std::unordered_map<std::string, std::vector<std::string>>& sym_vars)
+      : metadata_(metadata), sym_vars_(sym_vars) {}
+
+  PackedFunc GetFunction(const std::string& name, const ObjectPtr<Object>& sptr_to_self) final {
+    // Initialize and memoize the module.
+    // Usually, we have some warmup runs. The module initialization should be
+    // done at this stage. Therefore, runtime overhead is not a concern.
+    if (initialized_.count(name) == 0) {
+      this->InitSubModule(name);
+      initialized_[name] = true;
+    }
+
+    // Run the module.
+    // Normally we would only have a limited number of submodules. The runtime
+    // symobl lookup overhead should be minimal.
+    CHECK(!this->imports().empty());
+    for (Module it : this->imports()) {
+      PackedFunc pf = it.GetFunction(name);
+      if (pf != nullptr) return pf;
+    }
+    return PackedFunc(nullptr);
+  }
+
+  const char* type_key() const { return "metadata"; }
+
+  /*!
+   * \brief Get the list of metadata that is required by the given module.
+   * \param symbol The symbol that is being queried.
+   * \return The list of needed NDArray.
+   */
+  Array<NDArray> GetRequiredMetadata(const std::string& symbol) {
+    Array<NDArray> ret;
+    CHECK_GT(sym_vars_.count(symbol), 0U) << "Not symbol is recorded for " << symbol;
+    std::vector<std::string> vars = sym_vars_[symbol];
+    for (const auto& it : vars) {
+      CHECK_GT(metadata_.count(it), 0U) << "Found not recorded constant variable: " << it;
+      ret.push_back(metadata_[it]);
+    }
+    return ret;
+  }
+
+  /*!
+   * \brief Initialize each imported module.
+   * \param symobl The symbol used for initializing a module. It is also used
+   * for runtime lookup.
+   *
+   * \note  A module could be like the following:
+   *  MetadataModuleNode (contains all the metadata)
+   *    - CSourceModule
+   *    - JSON runtime module
+   *
+   *  The initializer iterates through the imported modules and intilizes the
+   *  found module accordingly by passing the needed metadata into it.
+   */
+  void InitSubModule(const std::string& symbol) {
+    PackedFunc init(nullptr);
+    for (Module it : this->imports()) {
+      // Get the initialization function from the imported modules.
+      std::string init_name = "__init_" + symbol;
+      init = it.GetFunction(init_name, false);
+      if (init != nullptr) {
+        auto md = GetRequiredMetadata(symbol);
+        // Initialize the module with metadata.
+        init(md);
+        break;
+      }
+    }
+  }
+
+  void SaveToBinary(dmlc::Stream* stream) final {
+    std::vector<std::string> variables;
+    std::vector<NDArray> metadata;
+    for (const auto& it : metadata_) {
+      String var_name = it.first;
+      variables.push_back(var_name);
+      metadata.push_back(it.second);
+    }
+
+    // Save all variables in the function.
+    stream->Write(variables);
+    // Save all constant data.
+    uint64_t sz = static_cast<uint64_t>(metadata.size());
+    stream->Write(sz);
+    for (uint64_t i = 0; i < sz; i++) {
+      metadata[i].Save(stream);
+    }
+
+    // Save the symbol to list of required constant variables mapping
+    std::vector<std::string> symbols;
+    std::vector<std::vector<std::string>> const_vars;
+    for (const auto& it : sym_vars_) {
+      symbols.push_back(it.first);
+      const_vars.push_back(it.second);
+    }
+
+    stream->Write(symbols);
+    sz = static_cast<uint64_t>(sym_vars_.size());
+    stream->Write(sz);
+    for (uint64_t i = 0; i < sz; i++) {
+      stream->Write(const_vars[i]);
+    }
+  }
+
+  static Module LoadFromBinary(void* strm) {
+    dmlc::Stream* stream = static_cast<dmlc::Stream*>(strm);
+
+    // Load the variables.
+    std::vector<std::string> variables;
+    CHECK(stream->Read(&variables)) << "Loading variables failed";
+    uint64_t sz;
+    CHECK(stream->Read(&sz, sizeof(sz))) << "Loading medata size failed";
+    CHECK_EQ(static_cast<size_t>(sz), variables.size())

Review comment:
       I think we can keep this cast to make `size_t m_size = static_cast<size_t>(sz)` so that the rest loops in this function can all use `size_t i`.

##########
File path: src/runtime/metadata_module.cc
##########
@@ -0,0 +1,220 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*!
+ * \file src/runtime/metadata_module.cc
+ * \brief A wrapper for initializing imported modules using metadata. This
+ * module is intended to be used by various runtime in the TVM stack, i.e.
+ * graph runtime, relay VM, AOT runtime, and various user defined runtimes. It
+ * paves the way to separate the code and metedata, which makes compilation
+ * and/or interpretation more convenient. In addition, the clear separation of
+ * code and metadata significantly reduces the efforts for handling external
+ * codegen and runtimes.
+ */
+#include <tvm/node/container.h>
+#include <tvm/runtime/ndarray.h>
+#include <tvm/runtime/packed_func.h>
+#include <tvm/runtime/registry.h>
+
+#include <cstdint>
+#include <sstream>
+
+#include "meta_data.h"
+
+namespace tvm {
+namespace runtime {
+
+/*!
+ * \brief The metadata module is designed to manage initialization of the
+ * imported submodules.
+ */
+class MetadataModuleNode : public ModuleNode {
+ public:
+  MetadataModuleNode(const std::unordered_map<std::string, NDArray>& metadata,
+                     const std::unordered_map<std::string, std::vector<std::string>>& sym_vars)
+      : metadata_(metadata), sym_vars_(sym_vars) {}
+
+  PackedFunc GetFunction(const std::string& name, const ObjectPtr<Object>& sptr_to_self) final {
+    // Initialize and memoize the module.
+    // Usually, we have some warmup runs. The module initialization should be
+    // done at this stage. Therefore, runtime overhead is not a concern.
+    if (initialized_.count(name) == 0) {
+      this->InitSubModule(name);
+      initialized_[name] = true;
+    }
+
+    // Run the module.
+    // Normally we would only have a limited number of submodules. The runtime
+    // symobl lookup overhead should be minimal.
+    CHECK(!this->imports().empty());
+    for (Module it : this->imports()) {
+      PackedFunc pf = it.GetFunction(name);
+      if (pf != nullptr) return pf;
+    }
+    return PackedFunc(nullptr);
+  }
+
+  const char* type_key() const { return "metadata"; }
+
+  /*!
+   * \brief Get the list of metadata that is required by the given module.
+   * \param symbol The symbol that is being queried.
+   * \return The list of needed NDArray.
+   */
+  Array<NDArray> GetRequiredMetadata(const std::string& symbol) {
+    Array<NDArray> ret;
+    CHECK_GT(sym_vars_.count(symbol), 0U) << "Not symbol is recorded for " << symbol;
+    std::vector<std::string> vars = sym_vars_[symbol];
+    for (const auto& it : vars) {
+      CHECK_GT(metadata_.count(it), 0U) << "Found not recorded constant variable: " << it;
+      ret.push_back(metadata_[it]);
+    }
+    return ret;
+  }
+
+  /*!
+   * \brief Initialize each imported module.
+   * \param symobl The symbol used for initializing a module. It is also used
+   * for runtime lookup.
+   *
+   * \note  A module could be like the following:
+   *  MetadataModuleNode (contains all the metadata)
+   *    - CSourceModule
+   *    - JSON runtime module
+   *
+   *  The initializer iterates through the imported modules and intilizes the
+   *  found module accordingly by passing the needed metadata into it.
+   */
+  void InitSubModule(const std::string& symbol) {
+    PackedFunc init(nullptr);
+    for (Module it : this->imports()) {
+      // Get the initialization function from the imported modules.
+      std::string init_name = "__init_" + symbol;
+      init = it.GetFunction(init_name, false);
+      if (init != nullptr) {
+        auto md = GetRequiredMetadata(symbol);
+        // Initialize the module with metadata.
+        init(md);
+        break;
+      }
+    }
+  }
+
+  void SaveToBinary(dmlc::Stream* stream) final {
+    std::vector<std::string> variables;
+    std::vector<NDArray> metadata;
+    for (const auto& it : metadata_) {
+      String var_name = it.first;
+      variables.push_back(var_name);
+      metadata.push_back(it.second);
+    }
+
+    // Save all variables in the function.
+    stream->Write(variables);
+    // Save all constant data.
+    uint64_t sz = static_cast<uint64_t>(metadata.size());
+    stream->Write(sz);
+    for (uint64_t i = 0; i < sz; i++) {
+      metadata[i].Save(stream);
+    }
+
+    // Save the symbol to list of required constant variables mapping
+    std::vector<std::string> symbols;
+    std::vector<std::vector<std::string>> const_vars;
+    for (const auto& it : sym_vars_) {
+      symbols.push_back(it.first);
+      const_vars.push_back(it.second);
+    }
+
+    stream->Write(symbols);
+    sz = static_cast<uint64_t>(sym_vars_.size());
+    stream->Write(sz);
+    for (uint64_t i = 0; i < sz; i++) {
+      stream->Write(const_vars[i]);
+    }
+  }
+
+  static Module LoadFromBinary(void* strm) {
+    dmlc::Stream* stream = static_cast<dmlc::Stream*>(strm);
+
+    // Load the variables.
+    std::vector<std::string> variables;
+    CHECK(stream->Read(&variables)) << "Loading variables failed";
+    uint64_t sz;
+    CHECK(stream->Read(&sz, sizeof(sz))) << "Loading medata size failed";
+    CHECK_EQ(static_cast<size_t>(sz), variables.size())
+        << "The number of variables and ndarray counts must match";
+    // Load the list of ndarray.
+    std::vector<NDArray> arrays;
+    for (uint64_t i = 0; i < sz; i++) {
+      NDArray temp;
+      temp.Load(stream);
+      arrays.push_back(temp);
+    }
+
+    std::unordered_map<std::string, NDArray> metadata;
+    for (size_t i = 0; i < variables.size(); i++) {
+      CHECK_EQ(metadata.count(variables[i]), 0U);
+      metadata[variables[i]] = arrays[i];
+    }
+
+    // Load the symbol to list of required constant variables mapping
+    std::vector<std::string> symbols;
+    CHECK(stream->Read(&symbols)) << "Loading symbols failed";
+    CHECK(stream->Read(&sz, sizeof(sz))) << "Loading number of symbols failed";
+    CHECK_EQ(static_cast<size_t>(sz), symbols.size());
+    std::vector<std::vector<std::string>> const_vars;
+    for (uint64_t i = 0; i < sz; i++) {
+      std::vector<std::string> vars;
+      CHECK(stream->Read(&vars)) << "Loading const variables failed";
+      const_vars.push_back(vars);
+    }
+
+    std::unordered_map<std::string, std::vector<std::string>> sym_vars;
+    for (uint64_t i = 0; i < sz; i++) {
+      sym_vars[symbols[i]] = const_vars[i];
+    }
+
+    auto n = make_object<MetadataModuleNode>(metadata, sym_vars);
+    return Module(n);
+  }
+
+ private:
+  /*!
+   * \brief Record if a module is initialized. It is needed by imported
+   * modules using execution engine.
+   */
+  std::unordered_map<std::string, bool> initialized_;

Review comment:
       `unordered_set` fits better for this case, but I suppose `unordered_map` will be replaced with `runtime::Map` in the future so it should be fine to keep it.

##########
File path: src/runtime/metadata_module.cc
##########
@@ -0,0 +1,220 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*!
+ * \file src/runtime/metadata_module.cc
+ * \brief A wrapper for initializing imported modules using metadata. This
+ * module is intended to be used by various runtime in the TVM stack, i.e.
+ * graph runtime, relay VM, AOT runtime, and various user defined runtimes. It
+ * paves the way to separate the code and metedata, which makes compilation
+ * and/or interpretation more convenient. In addition, the clear separation of
+ * code and metadata significantly reduces the efforts for handling external
+ * codegen and runtimes.
+ */
+#include <tvm/node/container.h>
+#include <tvm/runtime/ndarray.h>
+#include <tvm/runtime/packed_func.h>
+#include <tvm/runtime/registry.h>
+
+#include <cstdint>
+#include <sstream>
+
+#include "meta_data.h"
+
+namespace tvm {
+namespace runtime {
+
+/*!
+ * \brief The metadata module is designed to manage initialization of the
+ * imported submodules.
+ */
+class MetadataModuleNode : public ModuleNode {
+ public:
+  MetadataModuleNode(const std::unordered_map<std::string, NDArray>& metadata,
+                     const std::unordered_map<std::string, std::vector<std::string>>& sym_vars)
+      : metadata_(metadata), sym_vars_(sym_vars) {}
+
+  PackedFunc GetFunction(const std::string& name, const ObjectPtr<Object>& sptr_to_self) final {
+    // Initialize and memoize the module.
+    // Usually, we have some warmup runs. The module initialization should be
+    // done at this stage. Therefore, runtime overhead is not a concern.
+    if (initialized_.count(name) == 0) {
+      this->InitSubModule(name);
+      initialized_[name] = true;
+    }
+
+    // Run the module.
+    // Normally we would only have a limited number of submodules. The runtime
+    // symobl lookup overhead should be minimal.
+    CHECK(!this->imports().empty());
+    for (Module it : this->imports()) {
+      PackedFunc pf = it.GetFunction(name);
+      if (pf != nullptr) return pf;
+    }
+    return PackedFunc(nullptr);
+  }
+
+  const char* type_key() const { return "metadata"; }
+
+  /*!
+   * \brief Get the list of metadata that is required by the given module.
+   * \param symbol The symbol that is being queried.
+   * \return The list of needed NDArray.
+   */
+  Array<NDArray> GetRequiredMetadata(const std::string& symbol) {
+    Array<NDArray> ret;
+    CHECK_GT(sym_vars_.count(symbol), 0U) << "Not symbol is recorded for " << symbol;
+    std::vector<std::string> vars = sym_vars_[symbol];
+    for (const auto& it : vars) {
+      CHECK_GT(metadata_.count(it), 0U) << "Found not recorded constant variable: " << it;
+      ret.push_back(metadata_[it]);
+    }
+    return ret;
+  }
+
+  /*!
+   * \brief Initialize each imported module.
+   * \param symobl The symbol used for initializing a module. It is also used
+   * for runtime lookup.
+   *
+   * \note  A module could be like the following:
+   *  MetadataModuleNode (contains all the metadata)
+   *    - CSourceModule
+   *    - JSON runtime module
+   *
+   *  The initializer iterates through the imported modules and intilizes the
+   *  found module accordingly by passing the needed metadata into it.
+   */
+  void InitSubModule(const std::string& symbol) {
+    PackedFunc init(nullptr);
+    for (Module it : this->imports()) {
+      // Get the initialization function from the imported modules.
+      std::string init_name = "__init_" + symbol;
+      init = it.GetFunction(init_name, false);
+      if (init != nullptr) {
+        auto md = GetRequiredMetadata(symbol);
+        // Initialize the module with metadata.
+        init(md);
+        break;
+      }
+    }
+  }
+
+  void SaveToBinary(dmlc::Stream* stream) final {
+    std::vector<std::string> variables;
+    std::vector<NDArray> metadata;
+    for (const auto& it : metadata_) {
+      String var_name = it.first;
+      variables.push_back(var_name);
+      metadata.push_back(it.second);
+    }
+
+    // Save all variables in the function.
+    stream->Write(variables);
+    // Save all constant data.
+    uint64_t sz = static_cast<uint64_t>(metadata.size());
+    stream->Write(sz);
+    for (uint64_t i = 0; i < sz; i++) {
+      metadata[i].Save(stream);
+    }
+
+    // Save the symbol to list of required constant variables mapping
+    std::vector<std::string> symbols;
+    std::vector<std::vector<std::string>> const_vars;
+    for (const auto& it : sym_vars_) {
+      symbols.push_back(it.first);
+      const_vars.push_back(it.second);
+    }
+
+    stream->Write(symbols);
+    sz = static_cast<uint64_t>(sym_vars_.size());
+    stream->Write(sz);
+    for (uint64_t i = 0; i < sz; i++) {
+      stream->Write(const_vars[i]);
+    }
+  }
+
+  static Module LoadFromBinary(void* strm) {
+    dmlc::Stream* stream = static_cast<dmlc::Stream*>(strm);
+
+    // Load the variables.
+    std::vector<std::string> variables;
+    CHECK(stream->Read(&variables)) << "Loading variables failed";
+    uint64_t sz;
+    CHECK(stream->Read(&sz, sizeof(sz))) << "Loading medata size failed";

Review comment:
       s/medata/metadata/

##########
File path: src/runtime/metadata_module.cc
##########
@@ -0,0 +1,220 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*!
+ * \file src/runtime/metadata_module.cc
+ * \brief A wrapper for initializing imported modules using metadata. This
+ * module is intended to be used by various runtime in the TVM stack, i.e.
+ * graph runtime, relay VM, AOT runtime, and various user defined runtimes. It
+ * paves the way to separate the code and metedata, which makes compilation
+ * and/or interpretation more convenient. In addition, the clear separation of
+ * code and metadata significantly reduces the efforts for handling external
+ * codegen and runtimes.
+ */
+#include <tvm/node/container.h>
+#include <tvm/runtime/ndarray.h>
+#include <tvm/runtime/packed_func.h>
+#include <tvm/runtime/registry.h>
+
+#include <cstdint>
+#include <sstream>
+
+#include "meta_data.h"
+
+namespace tvm {
+namespace runtime {
+
+/*!
+ * \brief The metadata module is designed to manage initialization of the
+ * imported submodules.
+ */
+class MetadataModuleNode : public ModuleNode {
+ public:
+  MetadataModuleNode(const std::unordered_map<std::string, NDArray>& metadata,
+                     const std::unordered_map<std::string, std::vector<std::string>>& sym_vars)
+      : metadata_(metadata), sym_vars_(sym_vars) {}
+
+  PackedFunc GetFunction(const std::string& name, const ObjectPtr<Object>& sptr_to_self) final {
+    // Initialize and memoize the module.
+    // Usually, we have some warmup runs. The module initialization should be
+    // done at this stage. Therefore, runtime overhead is not a concern.
+    if (initialized_.count(name) == 0) {
+      this->InitSubModule(name);
+      initialized_[name] = true;
+    }
+
+    // Run the module.
+    // Normally we would only have a limited number of submodules. The runtime
+    // symobl lookup overhead should be minimal.
+    CHECK(!this->imports().empty());
+    for (Module it : this->imports()) {
+      PackedFunc pf = it.GetFunction(name);
+      if (pf != nullptr) return pf;
+    }
+    return PackedFunc(nullptr);
+  }
+
+  const char* type_key() const { return "metadata"; }
+
+  /*!
+   * \brief Get the list of metadata that is required by the given module.
+   * \param symbol The symbol that is being queried.
+   * \return The list of needed NDArray.
+   */
+  Array<NDArray> GetRequiredMetadata(const std::string& symbol) {
+    Array<NDArray> ret;
+    CHECK_GT(sym_vars_.count(symbol), 0U) << "Not symbol is recorded for " << symbol;
+    std::vector<std::string> vars = sym_vars_[symbol];
+    for (const auto& it : vars) {
+      CHECK_GT(metadata_.count(it), 0U) << "Found not recorded constant variable: " << it;
+      ret.push_back(metadata_[it]);
+    }
+    return ret;
+  }
+
+  /*!
+   * \brief Initialize each imported module.
+   * \param symobl The symbol used for initializing a module. It is also used
+   * for runtime lookup.
+   *
+   * \note  A module could be like the following:
+   *  MetadataModuleNode (contains all the metadata)
+   *    - CSourceModule
+   *    - JSON runtime module
+   *
+   *  The initializer iterates through the imported modules and intilizes the
+   *  found module accordingly by passing the needed metadata into it.
+   */
+  void InitSubModule(const std::string& symbol) {
+    PackedFunc init(nullptr);
+    for (Module it : this->imports()) {
+      // Get the initialization function from the imported modules.
+      std::string init_name = "__init_" + symbol;
+      init = it.GetFunction(init_name, false);
+      if (init != nullptr) {
+        auto md = GetRequiredMetadata(symbol);
+        // Initialize the module with metadata.
+        init(md);
+        break;
+      }
+    }

Review comment:
       Better to have a CHECK to make sure it was initialized correctly.

##########
File path: src/runtime/metadata_module.cc
##########
@@ -0,0 +1,220 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*!
+ * \file src/runtime/metadata_module.cc
+ * \brief A wrapper for initializing imported modules using metadata. This
+ * module is intended to be used by various runtime in the TVM stack, i.e.
+ * graph runtime, relay VM, AOT runtime, and various user defined runtimes. It
+ * paves the way to separate the code and metedata, which makes compilation
+ * and/or interpretation more convenient. In addition, the clear separation of
+ * code and metadata significantly reduces the efforts for handling external
+ * codegen and runtimes.
+ */
+#include <tvm/node/container.h>
+#include <tvm/runtime/ndarray.h>
+#include <tvm/runtime/packed_func.h>
+#include <tvm/runtime/registry.h>
+
+#include <cstdint>
+#include <sstream>
+
+#include "meta_data.h"
+
+namespace tvm {
+namespace runtime {
+
+/*!
+ * \brief The metadata module is designed to manage initialization of the
+ * imported submodules.
+ */
+class MetadataModuleNode : public ModuleNode {
+ public:
+  MetadataModuleNode(const std::unordered_map<std::string, NDArray>& metadata,
+                     const std::unordered_map<std::string, std::vector<std::string>>& sym_vars)
+      : metadata_(metadata), sym_vars_(sym_vars) {}
+
+  PackedFunc GetFunction(const std::string& name, const ObjectPtr<Object>& sptr_to_self) final {
+    // Initialize and memoize the module.
+    // Usually, we have some warmup runs. The module initialization should be
+    // done at this stage. Therefore, runtime overhead is not a concern.
+    if (initialized_.count(name) == 0) {
+      this->InitSubModule(name);
+      initialized_[name] = true;
+    }
+
+    // Run the module.
+    // Normally we would only have a limited number of submodules. The runtime
+    // symobl lookup overhead should be minimal.
+    CHECK(!this->imports().empty());
+    for (Module it : this->imports()) {
+      PackedFunc pf = it.GetFunction(name);
+      if (pf != nullptr) return pf;
+    }
+    return PackedFunc(nullptr);
+  }
+
+  const char* type_key() const { return "metadata"; }
+
+  /*!
+   * \brief Get the list of metadata that is required by the given module.
+   * \param symbol The symbol that is being queried.
+   * \return The list of needed NDArray.
+   */
+  Array<NDArray> GetRequiredMetadata(const std::string& symbol) {
+    Array<NDArray> ret;
+    CHECK_GT(sym_vars_.count(symbol), 0U) << "Not symbol is recorded for " << symbol;

Review comment:
       s/Not/No/

##########
File path: src/relay/backend/contrib/codegen_c/codegen.cc
##########
@@ -76,43 +77,31 @@ class CodegenC : public MemoizedExprTranslator<std::vector<Output>>, public Code
   }
 
   std::vector<Output> VisitExpr_(const ConstantNode* cn) final {
-    // Note this is for demonstration purpose. ConstantNode doesn't necessarily
-    // belong to calls. We need to revisit this when tuples come into play.
-
     std::ostringstream decl_stream;
     std::ostringstream buf_stream;
 
     Output output;
-    output.name = "const_" + std::to_string(const_idx_++);
-
-    runtime::NDArray array = cn->data;
-    const auto& shape = array.Shape();
-
-    // Get the number of elements.
-    int64_t num_elems = 1;
-    for (auto i : shape) num_elems *= i;
-
+    // Get const: static_cast<float*>(dnnl_0_consts[0]->data)
+    output.name = "static_cast<float*>(" + ext_func_id_ + "_consts[" + std::to_string(const_idx_) +
+                  "]->data)";
     const auto* type_node = cn->checked_type().as<TensorTypeNode>();
     CHECK(type_node);
     const auto& dtype = GetDtypeString(type_node);
-    // Define a const buffer: float const_0[64] = {1.0, 2.0, ...};
-    //
-    // Technically, you may need: static float* const_0 = (float*)malloc(4 * 64)
-    // to avoid possible stack overflow.
-    buf_stream << dtype << " " << output.name << "[" << num_elems << "] = {";
-    if (dtype == "float") {
-      float* p_flt = static_cast<float*>(array->data);
-      for (int64_t i = 0; i < num_elems - 1; i++) buf_stream << p_flt[i] << ", ";
-      if (num_elems) buf_stream << p_flt[num_elems - 1];
-    } else if (dtype == "int") {
-      int* p_flt = static_cast<int*>(array->data);
-      for (int64_t i = 0; i < num_elems - 1; i++) buf_stream << p_flt[i] << ", ";
-      if (num_elems) buf_stream << p_flt[num_elems - 1];
-    } else {
-      LOG(FATAL) << "Only float and int are supported for now.";
+
+    // Generate the global variable for needed ndarrays
+    if (const_array_.empty()) {
+      const_array_ = "Array<NDArray> " + ext_func_id_ + "_consts;";
+      std::ostringstream buf_stream;
+      buf_stream << "CHECK(!" << ext_func_id_
+                 << "_consts.empty()) << \"C source module hasn't been initialized.\";\n";
+      ext_func_body_.insert(ext_func_body_.begin(), buf_stream.str());
     }
-    buf_stream << "};";
-    ext_func_body.insert(ext_func_body.begin(), buf_stream.str());
+
+    CHECK(dtype == "float" || dtype == "int") << "Only float and int are supported for now.";
+    output.dtype = dtype;
+
+    std::string const_var_name = ext_func_id_ + "_const_" + std::to_string(const_idx_++);

Review comment:
       As `_const_` is now a keyword for runtime to take actions, we may generate `const_var_name` in the base class to make sure users won't violate miss it.




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-tvm] tqchen commented on a change in pull request #5770: [BYOC][runtime] Separate code and metadata for CSourceModule

Posted by GitBox <gi...@apache.org>.
tqchen commented on a change in pull request #5770:
URL: https://github.com/apache/incubator-tvm/pull/5770#discussion_r439591969



##########
File path: python/tvm/runtime/module.py
##########
@@ -33,6 +33,25 @@
 ProfileResult = namedtuple("ProfileResult", ["mean", "results"])
 
 
+def ModuleInitWrapper(variables, metadata):
+    """Create a module initialization wrapper.

Review comment:
       Thanks! One thing that we want to be mindful of is to reduce the amount of concepts that an developer should learn. In most cases, we only want the developer to be aware of runtime.Module and not beyond that so the backend impl can change over time.




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-tvm] tqchen commented on a change in pull request #5770: [BYOC][runtime] Separate code and metadata for CSourceModule

Posted by GitBox <gi...@apache.org>.
tqchen commented on a change in pull request #5770:
URL: https://github.com/apache/incubator-tvm/pull/5770#discussion_r439591410



##########
File path: src/runtime/module_init_wrapper.cc
##########
@@ -0,0 +1,234 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*!
+ * \file src/runtime/module_init_wrapper.cc
+ * \brief A wrapper for initializing modules using metadata
+ */
+#include <tvm/node/container.h>
+#include <tvm/runtime/ndarray.h>
+#include <tvm/runtime/packed_func.h>
+#include <tvm/runtime/registry.h>
+
+#include <cstdint>
+#include <sstream>
+
+#include "file_util.h"
+
+namespace tvm {
+namespace runtime {
+
+using StringNDArrayMap = std::unordered_map<String, runtime::NDArray, ObjectHash, ObjectEqual>;
+
+class CSourceMetadataInitializer {
+ public:
+  explicit CSourceMetadataInitializer(const StringNDArrayMap& metadata) : metadata_(metadata) {}
+
+  template <typename T>
+  void GetElements(const std::string& var_name, const std::string& type_name,
+                   const runtime::NDArray& arr) {
+    // Get the number of elements.
+    int64_t num_elems = 1;
+    for (auto i : arr.Shape()) num_elems *= i;
+    stream_ << "static " << type_name << " " << var_name << "[" << num_elems << "] = {";
+    T* ptr = static_cast<T*>(arr->data);
+    for (int64_t i = 0; i < num_elems - 1; i++) {
+      stream_ << ptr[i] << ",";
+    }
+    if (num_elems > 0) stream_ << ptr[num_elems - 1];
+    stream_ << "};\n";
+  }
+
+  std::string Init() {
+    for (const auto& it : metadata_) {
+      std::string var_name = it.first.operator std::string();
+      runtime::NDArray data = it.second;
+      CHECK_EQ(data->dtype.lanes, 1U);
+      if (data->dtype.code == kDLFloat) {
+        if (data->dtype.bits == 32) {
+          stream_.precision(std::numeric_limits<float>::digits10 + 1);
+          GetElements<float>(var_name, "float", data);
+        } else {
+          CHECK_EQ(data->dtype.bits, 64);
+          stream_.precision(std::numeric_limits<double>::digits10 + 1);
+          GetElements<double>(var_name, "double", data);
+        }
+      } else if (data->dtype.code == kDLUInt) {
+        if (data->dtype.bits == 8) {
+          GetElements<uint8_t>(var_name, "uint8_t", data);
+        } else {
+          CHECK_EQ(data->dtype.bits, 32);
+          GetElements<uint32_t>(var_name, "uint32_t", data);
+        }
+      } else {
+        if (data->dtype.bits == 8) {
+          GetElements<int8_t>(var_name, "int8_t", data);
+        } else {
+          CHECK_EQ(data->dtype.bits, 32);
+          GetElements<int32_t>(var_name, "int32_t", data);
+        }
+      }
+    }
+    return stream_.str();
+  }
+
+ private:
+  /*! \brief The stream to print constant data. */
+  std::ostringstream stream_;
+  /*! \brief variable name to NDArray mapping. */
+  StringNDArrayMap metadata_;
+};
+
+class ModuleInitWrapper : public runtime::ModuleNode {

Review comment:
       MetadataModule sounds good




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-tvm] tqchen commented on a change in pull request #5770: [BYOC][runtime] Separate code and metadata for CSourceModule

Posted by GitBox <gi...@apache.org>.
tqchen commented on a change in pull request #5770:
URL: https://github.com/apache/incubator-tvm/pull/5770#discussion_r439504941



##########
File path: src/runtime/module_init_wrapper.cc
##########
@@ -0,0 +1,234 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*!
+ * \file src/runtime/module_init_wrapper.cc
+ * \brief A wrapper for initializing modules using metadata
+ */
+#include <tvm/node/container.h>
+#include <tvm/runtime/ndarray.h>
+#include <tvm/runtime/packed_func.h>
+#include <tvm/runtime/registry.h>
+
+#include <cstdint>
+#include <sstream>
+
+#include "file_util.h"
+
+namespace tvm {
+namespace runtime {
+
+using StringNDArrayMap = std::unordered_map<String, runtime::NDArray, ObjectHash, ObjectEqual>;
+
+class CSourceMetadataInitializer {
+ public:
+  explicit CSourceMetadataInitializer(const StringNDArrayMap& metadata) : metadata_(metadata) {}
+
+  template <typename T>
+  void GetElements(const std::string& var_name, const std::string& type_name,
+                   const runtime::NDArray& arr) {
+    // Get the number of elements.
+    int64_t num_elems = 1;
+    for (auto i : arr.Shape()) num_elems *= i;
+    stream_ << "static " << type_name << " " << var_name << "[" << num_elems << "] = {";
+    T* ptr = static_cast<T*>(arr->data);
+    for (int64_t i = 0; i < num_elems - 1; i++) {
+      stream_ << ptr[i] << ",";
+    }
+    if (num_elems > 0) stream_ << ptr[num_elems - 1];
+    stream_ << "};\n";
+  }
+
+  std::string Init() {
+    for (const auto& it : metadata_) {
+      std::string var_name = it.first.operator std::string();
+      runtime::NDArray data = it.second;
+      CHECK_EQ(data->dtype.lanes, 1U);
+      if (data->dtype.code == kDLFloat) {
+        if (data->dtype.bits == 32) {
+          stream_.precision(std::numeric_limits<float>::digits10 + 1);
+          GetElements<float>(var_name, "float", data);
+        } else {
+          CHECK_EQ(data->dtype.bits, 64);
+          stream_.precision(std::numeric_limits<double>::digits10 + 1);
+          GetElements<double>(var_name, "double", data);
+        }
+      } else if (data->dtype.code == kDLUInt) {
+        if (data->dtype.bits == 8) {
+          GetElements<uint8_t>(var_name, "uint8_t", data);
+        } else {
+          CHECK_EQ(data->dtype.bits, 32);
+          GetElements<uint32_t>(var_name, "uint32_t", data);
+        }
+      } else {
+        if (data->dtype.bits == 8) {
+          GetElements<int8_t>(var_name, "int8_t", data);
+        } else {
+          CHECK_EQ(data->dtype.bits, 32);
+          GetElements<int32_t>(var_name, "int32_t", data);
+        }
+      }
+    }
+    return stream_.str();
+  }
+
+ private:
+  /*! \brief The stream to print constant data. */
+  std::ostringstream stream_;
+  /*! \brief variable name to NDArray mapping. */
+  StringNDArrayMap metadata_;
+};
+
+class ModuleInitWrapper : public runtime::ModuleNode {

Review comment:
       Let us think a bit about the name. ModuleInitWrapper may not be the best name.

##########
File path: python/tvm/runtime/module.py
##########
@@ -33,6 +33,25 @@
 ProfileResult = namedtuple("ProfileResult", ["mean", "results"])
 
 
+def ModuleInitWrapper(variables, metadata):
+    """Create a module initialization wrapper.

Review comment:
       Do we need to expose ModuleInitWrapper in the python side? Is it possible to simply hide the Module as part of backend, instead a frontend entity?

##########
File path: src/target/source/source_module.cc
##########
@@ -152,8 +153,92 @@ runtime::Module DeviceSourceModuleCreate(
   return runtime::Module(n);
 }
 
+// A helper used to wrap different types of modules and pass through packedfunc.
+// This module will never be used for compilation and execution.
+class ModuleClassWrapperNode : public runtime::ModuleNode {
+ public:
+  ModuleClassWrapperNode() = default;
+  const char* type_key() const { return "module_class_wrapper"; }
+  PackedFunc GetFunction(const std::string& name, const ObjectPtr<Object>& sptr_to_self) final {
+    LOG(FATAL) << "Cannot execute module wrapper";
+    return PackedFunc();
+  }
+};
+
+runtime::Module ModuleClassWrapperCreate() {
+  auto n = make_object<ModuleClassWrapperNode>();
+  return runtime::Module(n);
+}
+
+// Pack the source code and metadata, where source code could be any
+// user-defined code, i.e. c source code, json graph representation, etc.
+class SourceMetadataModuleNode final : public runtime::ModuleNode {
+ public:
+  SourceMetadataModuleNode(const String& func_symbol, const String& code, const String& source_type,
+                           const Array<String>& variables, const Array<runtime::NDArray>& metadata)
+      : func_symbol_(func_symbol),
+        code_(code),
+        source_type_(source_type),
+        variables_(variables),
+        metadata_(metadata) {}
+
+  PackedFunc GetFunction(const std::string& name, const ObjectPtr<Object>& sptr_to_self) final {
+    if (name == "get_source") {
+      return PackedFunc([sptr_to_self, this](TVMArgs args, TVMRetValue* rv) { *rv = this->code_; });
+    } else if (name == "get_source_type") {
+      return PackedFunc(
+          [sptr_to_self, this](TVMArgs args, TVMRetValue* rv) { *rv = this->source_type_; });
+    } else if (name == "get_symbol") {
+      return PackedFunc(
+          [sptr_to_self, this](TVMArgs args, TVMRetValue* rv) { *rv = this->func_symbol_; });
+    } else if (name == "get_vars") {
+      return PackedFunc(
+          [sptr_to_self, this](TVMArgs args, TVMRetValue* rv) { *rv = this->variables_; });
+    } else if (name == "get_metadata") {
+      return PackedFunc(

Review comment:
       What is the relation between this class and the ModuleInitWrapper. It seems to be that at leastw can create a MetaDataModule without tying to CSourceModule(e.g. use it for LLVM target as well)

##########
File path: python/tvm/target/source_module.py
##########
@@ -0,0 +1,66 @@
+# License .to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+# pylint: disable=no-else-return, unidiomatic-typecheck, undefined-variable, invalid-name, redefined-builtin
+"""
+Helper functions and classes for handling source and metdata.
+"""
+from tvm.runtime import _ffi_api
+
+class SourceMetadataModule:
+    """The module used to wrap both source and metadata."""

Review comment:
       Let us not expose it to the python side for now, as it is totally fine to get things from the mod itself. The only reason why have the graph runtime wrapper is that it is too commonly used




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-tvm] tqchen commented on pull request #5770: [BYOC][runtime] Separate code and metadata for CSourceModule

Posted by GitBox <gi...@apache.org>.
tqchen commented on pull request #5770:
URL: https://github.com/apache/incubator-tvm/pull/5770#issuecomment-645016600


   cc @mbaret @FrozenGene @comaniac it would be great if you can help to take a look as well


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-tvm] zhiics commented on a change in pull request #5770: [BYOC][runtime] Separate code and metadata for CSourceModule

Posted by GitBox <gi...@apache.org>.
zhiics commented on a change in pull request #5770:
URL: https://github.com/apache/incubator-tvm/pull/5770#discussion_r441302269



##########
File path: python/tvm/contrib/graph_runtime.py
##########
@@ -162,7 +162,11 @@ def set_input(self, key=None, value=None, **params):
             keys = list(params.keys())
             keys.sort(key=lambda x: -np.prod(params[x].shape))
             for k in keys:
-                self._get_input(k).copyfrom(params[k])
+                # TODO(zhiics) Skip the weights for submodule in a better way.
+                # We could get all inputs required by graphruntime first,
+                # we should use MetadataModule for initialization.
+                if "_const_" not in k:

Review comment:
       Yes, the constants for graph runtime all start with "p"
   
   https://github.com/apache/incubator-tvm/blob/3e72be58f362a21dbcc1de36f9dbed216e854baf/src/relay/backend/graph_runtime_codegen.cc#L314
   
   However, as the TODO says, we should have a better way to handle this. But graph runtime current doesn't really distinguish input/weights. I think we can use this simple workaround from the frontend for now.




----------------------------------------------------------------
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:
users@infra.apache.org