[GitHub] [tvm] jwfromm commented on a change in pull request #9811: PackedFunction to return params from the .so module, show warning when no params are set

[GitBox <gi...@apache.org>]

jwfromm commented on a change in pull request #9811:
URL: https://github.com/apache/tvm/pull/9811#discussion_r784151076



##########
File path: src/runtime/graph_executor/graph_executor_factory.cc
##########
@@ -57,6 +58,14 @@ PackedFunc GraphExecutorFactory::GetFunction(
     return PackedFunc(
         [sptr_to_self, this](TVMArgs args, TVMRetValue* rv) { *rv = this->graph_json_; });
 
+  } else if (name == "get_graph_params") {

Review comment:
       Whats the motivation behind adding this? It does seem useful so I'm all for it but we should add some tests to make sure it works as expected.

##########
File path: src/runtime/graph_executor/graph_executor.cc
##########
@@ -56,6 +56,9 @@ inline size_t GetDataAlignment(const DLTensor& arr) {
  * \brief Run all the operations one by one.
  */
 void GraphExecutor::Run() {
+  if (!params_set_) {
+    DLOG(WARNING) << "Params are not set, result may be invalid.";

Review comment:
       `params_set_` is only checking that the `set_input` method has been called at least once. This doesn't necessarily mean that all the parameters of the model have been set so this warning is a little misleading. I would recommend changing it to something like "No inputs have been provided, to get valid results call set_input before running."




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

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org