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 2022/02/22 16:27:36 UTC

[GitHub] [tvm] lhutton1 opened a new pull request #10348: [CMSIS-NN] Fix scalar to tensor constant pass when checked type is TupleType

lhutton1 opened a new pull request #10348:
URL: https://github.com/apache/tvm/pull/10348


   A failure is seen when compiling some networks due to assumption checked_type is a TensorTypeNode: 
   
   ```Check failed: (node != nullptr) is false: Expected type to be relay.TensorType, but get TupleType```
   
   Fixed by reshuffling some of the skip conditions and also ensures arguments are of type VarNode or ConstantNode (where necessary) before casting.
   
   cc @ashutosh-arm @Mousius @manupa-arm 
   


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



[GitHub] [tvm] lhutton1 commented on pull request #10348: [CMSIS-NN] Fix scalar to tensor constant pass when checked type is TupleType

Posted by GitBox <gi...@apache.org>.
lhutton1 commented on pull request #10348:
URL: https://github.com/apache/tvm/pull/10348#issuecomment-1049870269


   Thanks @manupa-arm, great points! Taking a second look at this I believe I need to investigate further, the behavior of my changes will mean tuples are simply skipped over, which I don't believe is correct.


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



[GitHub] [tvm] lhutton1 commented on pull request #10348: [CMSIS-NN] Fix scalar to tensor constant pass when checked type is TupleType

Posted by GitBox <gi...@apache.org>.
lhutton1 commented on pull request #10348:
URL: https://github.com/apache/tvm/pull/10348#issuecomment-1053590711


   After further investigation, the bug was found to be caused by the pass running on all functions in the input graph, not just CMSIS-NN functions. Therefore closing since the bug fix in #10375 supersedes this.


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



[GitHub] [tvm] manupa-arm commented on a change in pull request #10348: [CMSIS-NN] Fix scalar to tensor constant pass when checked type is TupleType

Posted by GitBox <gi...@apache.org>.
manupa-arm commented on a change in pull request #10348:
URL: https://github.com/apache/tvm/pull/10348#discussion_r813752267



##########
File path: src/relay/backend/contrib/cmsisnn/scalar_to_tensor_constant.cc
##########
@@ -117,21 +117,19 @@ class ScalarToTensorConstantMutator : public MixedModeMutator {
     for (uint32_t i = 0; i < call->args.size(); ++i) {
       Expr arg = call->args[i];
       new_args.push_back(arg);
-      if (!arg->checked_type_.defined()) {
-        continue;
-      }
-      auto* arg_type = arg->type_as<TensorTypeNode>();
-      if (arg_type->shape.size() != 0 || arg.as<ConstantNode>()) {
+      const auto* arg_var = arg.as<VarNode>();
+      const auto* arg_type = arg->checked_type_.as<TensorTypeNode>();
+      if (!arg_var || !arg_type || arg_type->shape.size() != 0) {

Review comment:
       nit : could use "->IsInstance<VarNode>()" if arg_var and arg_type is not used anymore.

##########
File path: src/relay/backend/contrib/cmsisnn/scalar_to_tensor_constant.cc
##########
@@ -142,19 +140,18 @@ class ScalarToTensorConstantMutator : public MixedModeMutator {
     for (uint32_t i = 0; i < call->args.size(); ++i) {
       new_args.push_back(call->args[i]);
       Expr scalar_arg = call->args[i];
-      if (!scalar_arg->checked_type_.defined()) {
-        continue;
-      }
-      Array<PrimExpr> scalar_shape = scalar_arg->type_as<TensorTypeNode>()->shape;
-      if (scalar_shape.size() != 0 || scalar_arg.as<ConstantNode>() == nullptr) {
+      const auto* scalar_const = scalar_arg.as<ConstantNode>();
+      const auto* scalar_type = scalar_arg->checked_type_.as<TensorTypeNode>();

Review comment:
       same nit as above

##########
File path: src/relay/backend/contrib/cmsisnn/scalar_to_tensor_constant.cc
##########
@@ -142,19 +140,18 @@ class ScalarToTensorConstantMutator : public MixedModeMutator {
     for (uint32_t i = 0; i < call->args.size(); ++i) {
       new_args.push_back(call->args[i]);
       Expr scalar_arg = call->args[i];
-      if (!scalar_arg->checked_type_.defined()) {
-        continue;
-      }
-      Array<PrimExpr> scalar_shape = scalar_arg->type_as<TensorTypeNode>()->shape;
-      if (scalar_shape.size() != 0 || scalar_arg.as<ConstantNode>() == nullptr) {
+      const auto* scalar_const = scalar_arg.as<ConstantNode>();
+      const auto* scalar_type = scalar_arg->checked_type_.as<TensorTypeNode>();
+      if (!scalar_const || !scalar_type || scalar_type->shape.size() != 0) {
         continue;
       }
       int tensor_arg_id = (i + 1) % 2;
       Expr tensor_arg = call->args[tensor_arg_id];
-      if (!tensor_arg->checked_type_.defined()) {
+      const auto* tensor_type_node = tensor_arg->checked_type_.as<TensorTypeNode>();
+      if (!tensor_type_node) {
         continue;
       }
-      TensorType tensor_type = GetRef<TensorType>(tensor_arg->type_as<TensorTypeNode>());
+      TensorType tensor_type = GetRef<TensorType>(tensor_type_node);

Review comment:
       same nit as above

##########
File path: src/relay/backend/contrib/cmsisnn/scalar_to_tensor_constant.cc
##########
@@ -117,21 +117,19 @@ class ScalarToTensorConstantMutator : public MixedModeMutator {
     for (uint32_t i = 0; i < call->args.size(); ++i) {
       Expr arg = call->args[i];
       new_args.push_back(arg);
-      if (!arg->checked_type_.defined()) {
-        continue;
-      }
-      auto* arg_type = arg->type_as<TensorTypeNode>();
-      if (arg_type->shape.size() != 0 || arg.as<ConstantNode>()) {
+      const auto* arg_var = arg.as<VarNode>();
+      const auto* arg_type = arg->checked_type_.as<TensorTypeNode>();
+      if (!arg_var || !arg_type || arg_type->shape.size() != 0) {
         continue;
       }
       String arg_name = arg.as<VarNode>()->name_hint();
       int tensor_arg_id = (i + 1) % 2;
       Expr tensor_arg = call->args[tensor_arg_id];
-      if (!tensor_arg->checked_type_.defined()) {
+      const auto* tensor_type = tensor_arg->checked_type_.as<TensorTypeNode>();
+      if (!tensor_type) {
         continue;
       }
-      TensorType tensor_type = GetRef<TensorType>(tensor_arg->type_as<TensorTypeNode>());
-      new_args.Set(i, Var(arg_name, tensor_type));
+      new_args.Set(i, Var(arg_name, GetRef<TensorType>(tensor_type)));

Review comment:
       nit : same here we dont need to extract the node just to covert it back to Ref




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



[GitHub] [tvm] manupa-arm commented on a change in pull request #10348: [CMSIS-NN] Fix scalar to tensor constant pass when checked type is TupleType

Posted by GitBox <gi...@apache.org>.
manupa-arm commented on a change in pull request #10348:
URL: https://github.com/apache/tvm/pull/10348#discussion_r813752267



##########
File path: src/relay/backend/contrib/cmsisnn/scalar_to_tensor_constant.cc
##########
@@ -117,21 +117,19 @@ class ScalarToTensorConstantMutator : public MixedModeMutator {
     for (uint32_t i = 0; i < call->args.size(); ++i) {
       Expr arg = call->args[i];
       new_args.push_back(arg);
-      if (!arg->checked_type_.defined()) {
-        continue;
-      }
-      auto* arg_type = arg->type_as<TensorTypeNode>();
-      if (arg_type->shape.size() != 0 || arg.as<ConstantNode>()) {
+      const auto* arg_var = arg.as<VarNode>();
+      const auto* arg_type = arg->checked_type_.as<TensorTypeNode>();
+      if (!arg_var || !arg_type || arg_type->shape.size() != 0) {

Review comment:
       nit : could use "->IsInstance\<VarNode\>() if arg_var and arg_type is not used anymore.




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



[GitHub] [tvm] lhutton1 closed pull request #10348: [CMSIS-NN] Fix scalar to tensor constant pass when checked type is TupleType

Posted by GitBox <gi...@apache.org>.
lhutton1 closed pull request #10348:
URL: https://github.com/apache/tvm/pull/10348


   


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