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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
