[GitHub] [tvm] ashutosh-arm commented on a diff in pull request #12970: [ETHOSN] Transpose fully connected weights

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

ashutosh-arm commented on code in PR #12970:
URL: https://github.com/apache/tvm/pull/12970#discussion_r985981792


##########
src/relay/backend/contrib/ethosn/ethosn_api.cc:
##########
@@ -213,27 +227,29 @@ EthosnError EthosnAPI::QnnFullyConnected(const Expr& expr, FullyConnectedParams*
                                       data_data_type, sl::DataFormat::NHWC, data_q_info);
 
   // Create weights info
-  const auto* weights_dtype = dense->args[1]->checked_type().as<TensorTypeNode>();
+  Constant weights_data = Downcast<Constant>(dense->args[1]);
+  weights_data = TransposeWeights(weights_data, "OI", "IO");

Review Comment:
   nit: I am in favor of full 4 dim conversion just to be consistent with the Relay supported kernel layouts.



##########
tests/python/contrib/test_ethosn/test_fullyconnected.py:
##########
@@ -30,7 +32,11 @@ def _get_model(
 ):
     """Return a model an any parameters it may have"""
     a = relay.var("a", shape=shape, dtype=dtype)
-    weights_array = tvm.nd.array(np.ones(weight_shape, dtype))

Review Comment:
   Huh :wolf: 



##########
tests/python/contrib/test_ethosn/test_fullyconnected.py:
##########
@@ -30,7 +32,11 @@ def _get_model(
 ):
     """Return a model an any parameters it may have"""
     a = relay.var("a", shape=shape, dtype=dtype)
-    weights_array = tvm.nd.array(np.ones(weight_shape, dtype))
+    weights_array = tvm.nd.array(
+        np.random.randint(
+            np.iinfo(dtype).min, high=np.iinfo(dtype).max, size=weight_shape, dtype=dtype

Review Comment:
   Would it be better to have a separate function providing range based on dtype? We do have something similar is microNPU?



##########
src/relay/backend/contrib/ethosn/ethosn_api.cc:
##########
@@ -171,6 +171,19 @@ EthosnError EthosnAPI::QnnConv2d(const Expr& expr, ConvolutionParams* params) {
   return err;
 }
 
+Constant TransposeWeights(const Constant& data, const std::string& input_layout,

Review Comment:
   Just a thought: Does it make sense to have it common for contrib or available for other passes?
   One other use case: https://github.com/apache/tvm/blob/fa17da22c73fb9e95c27e4c28130835b628caf6b/src/relay/backend/contrib/cmsisnn/generate_constants.cc#L74



##########
src/relay/backend/contrib/ethosn/ethosn_api.cc:
##########
@@ -197,7 +210,8 @@ EthosnError EthosnAPI::QnnFullyConnected(const Expr& expr, FullyConnectedParams*
   sl::QuantizationInfo output_q_info;
   err += Tvm2Npu(input_zero_point, input_scale, &data_q_info);
   err += Tvm2Npu(kernel_zero_point, kernel_scale, &weights_q_info);
-  err += Tvm2Npu(0, data_q_info.GetScale() * weights_q_info.GetScale(), &bias_q_info);
+  std::valarray<float> bias = data_q_info.GetScale() * weights_q_info.GetScales();
+  err += Tvm2Npu(0, bias, 3, &bias_q_info);

Review Comment:
   Nit: maybe a comment or a separate variable for 3 :thinking: ?
   



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