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 2021/09/01 23:54:25 UTC
[GitHub] [tvm] anwang2009 opened a new pull request #8907: Add support for QLinearConcat contrib op
anwang2009 opened a new pull request #8907:
URL: https://github.com/apache/tvm/pull/8907
- Moves `get_scalar` to common point where other Q op impls can reach it.
- Adds QLinearConcat
- Note the Lowering function within `_qnn.op.concatenate` will requantize as necessary, so there is nothing special necessary that needs to be done here for this Q op.
--
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] masahi merged pull request #8907: [ONNX] Add support for QLinearConcat contrib op
Posted by GitBox <gi...@apache.org>.
masahi merged pull request #8907:
URL: https://github.com/apache/tvm/pull/8907
--
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] AndrewZhaoLuo commented on a change in pull request #8907: Add support for QLinearConcat contrib op
Posted by GitBox <gi...@apache.org>.
AndrewZhaoLuo commented on a change in pull request #8907:
URL: https://github.com/apache/tvm/pull/8907#discussion_r701330456
##########
File path: python/tvm/relay/frontend/onnx.py
##########
@@ -3306,6 +3290,32 @@ def get_scalar(x, dtype="float32"):
return _qnn.op.quantize(out, y_scale, y_zero_point, out_dtype=dtype)
+class QLinearConcat(OnnxOpConverter):
+ """Operator converter for QLinearConcat from Microsoft onnxruntime contrib opset."""
+
+ @classmethod
+ def _impl_v1(cls, inputs, attr, params):
+ # which axis to concat on
+ axis = attr["axis"]
+
+ y_scale = fold_constant(get_scalar(inputs[0], params))
+ y_zero_point = get_scalar(inputs[1], params, "int32")
+
+ # input tensors, scales, zero_points
+ assert (
+ len(inputs) % 3 == 2
+ ), "Additional input count must be a multiple of 3 -- tensor/scale/zero_point tuples"
+ tensors = []
+ scales = []
+ zero_points = []
+ for i in range(2, len(inputs), 3):
Review comment:
Cool, documentation is wrong then. Not the first time :P
--
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] AndrewZhaoLuo commented on a change in pull request #8907: Add support for QLinearConcat contrib op
Posted by GitBox <gi...@apache.org>.
AndrewZhaoLuo commented on a change in pull request #8907:
URL: https://github.com/apache/tvm/pull/8907#discussion_r701310382
##########
File path: python/tvm/relay/frontend/onnx.py
##########
@@ -195,6 +195,17 @@ def _dim_check(attrs):
return _dim_check, "Only 1d, 2d and 3d kernel supported."
+def get_scalar(x, params, dtype="float32"):
+ """Helper to get a scalar value for Quantized operators."""
+ if isinstance(x, _expr.Var) and x.name_hint in params:
Review comment:
Should we throw an error if it's a var we can't look up in params
##########
File path: tests/python/frontend/onnx/test_forward.py
##########
@@ -5266,6 +5266,39 @@ def repeat(N, D):
)
+@tvm.testing.parametrize_targets
+def test_qlinearconcat(target, dev):
+ def verify_qlinearconcat(shapes, out_shape, axis=None):
+ input_names = []
+ input_values = []
+ input_nodes = []
+ for i in range(len(shapes)):
+ tensor_name = str(chr(ord("a") + i))
Review comment:
nit: I believe `chr` returns a string so no need to wrap it with `str`
##########
File path: python/tvm/relay/frontend/onnx.py
##########
@@ -3306,6 +3290,32 @@ def get_scalar(x, dtype="float32"):
return _qnn.op.quantize(out, y_scale, y_zero_point, out_dtype=dtype)
+class QLinearConcat(OnnxOpConverter):
+ """Operator converter for QLinearConcat from Microsoft onnxruntime contrib opset."""
+
+ @classmethod
+ def _impl_v1(cls, inputs, attr, params):
+ # which axis to concat on
+ axis = attr["axis"]
+
+ y_scale = fold_constant(get_scalar(inputs[0], params))
+ y_zero_point = get_scalar(inputs[1], params, "int32")
+
+ # input tensors, scales, zero_points
+ assert (
+ len(inputs) % 3 == 2
+ ), "Additional input count must be a multiple of 3 -- tensor/scale/zero_point tuples"
+ tensors = []
+ scales = []
+ zero_points = []
+ for i in range(2, len(inputs), 3):
Review comment:
Though it looks like your tests pass so idk.
##########
File path: python/tvm/relay/frontend/onnx.py
##########
@@ -3306,6 +3290,32 @@ def get_scalar(x, dtype="float32"):
return _qnn.op.quantize(out, y_scale, y_zero_point, out_dtype=dtype)
+class QLinearConcat(OnnxOpConverter):
+ """Operator converter for QLinearConcat from Microsoft onnxruntime contrib opset."""
+
+ @classmethod
+ def _impl_v1(cls, inputs, attr, params):
+ # which axis to concat on
+ axis = attr["axis"]
+
+ y_scale = fold_constant(get_scalar(inputs[0], params))
+ y_zero_point = get_scalar(inputs[1], params, "int32")
+
+ # input tensors, scales, zero_points
+ assert (
+ len(inputs) % 3 == 2
+ ), "Additional input count must be a multiple of 3 -- tensor/scale/zero_point tuples"
+ tensors = []
+ scales = []
+ zero_points = []
+ for i in range(2, len(inputs), 3):
Review comment:
Based on the spec it seems like these things should be packaged as tuples https://github.com/microsoft/onnxruntime/blob/master/docs/ContribOperators.md#com.microsoft.QLinearConcat
--
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] tmoreau89 commented on pull request #8907: Add support for QLinearConcat contrib op
Posted by GitBox <gi...@apache.org>.
tmoreau89 commented on pull request #8907:
URL: https://github.com/apache/tvm/pull/8907#issuecomment-912847440
Awesome to see this thank you @anwang2009 ! Could you add the [ONNX] tag to the PR? Thank you!
--
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] anwang2009 commented on a change in pull request #8907: Add support for QLinearConcat contrib op
Posted by GitBox <gi...@apache.org>.
anwang2009 commented on a change in pull request #8907:
URL: https://github.com/apache/tvm/pull/8907#discussion_r701329324
##########
File path: python/tvm/relay/frontend/onnx.py
##########
@@ -3306,6 +3290,32 @@ def get_scalar(x, dtype="float32"):
return _qnn.op.quantize(out, y_scale, y_zero_point, out_dtype=dtype)
+class QLinearConcat(OnnxOpConverter):
+ """Operator converter for QLinearConcat from Microsoft onnxruntime contrib opset."""
+
+ @classmethod
+ def _impl_v1(cls, inputs, attr, params):
+ # which axis to concat on
+ axis = attr["axis"]
+
+ y_scale = fold_constant(get_scalar(inputs[0], params))
+ y_zero_point = get_scalar(inputs[1], params, "int32")
+
+ # input tensors, scales, zero_points
+ assert (
+ len(inputs) % 3 == 2
+ ), "Additional input count must be a multiple of 3 -- tensor/scale/zero_point tuples"
+ tensors = []
+ scales = []
+ zero_points = []
+ for i in range(2, len(inputs), 3):
Review comment:
yeah, the onnxruntime implementation expects a sequence split across inputs as well, not tuples
--
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] AndrewZhaoLuo commented on pull request #8907: [ONNX] Add support for QLinearConcat contrib op
Posted by GitBox <gi...@apache.org>.
AndrewZhaoLuo commented on pull request #8907:
URL: https://github.com/apache/tvm/pull/8907#issuecomment-914529143
> It seems we have a CI error which I'm not sure relates to the PR at all:
>
> ```
> java.io.IOException: java.lang.RuntimeException: Failed to serialize hudson.model.Actionable#actions for class org.jenkinsci.plugins.workflow.job.WorkflowRun
> ```
If you jostle ci via `git commit -m "jostle ci"` --allow-empty` and push it should restart it. It's jsut something flaky with CI
--
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] anwang2009 commented on a change in pull request #8907: Add support for QLinearConcat contrib op
Posted by GitBox <gi...@apache.org>.
anwang2009 commented on a change in pull request #8907:
URL: https://github.com/apache/tvm/pull/8907#discussion_r702152497
##########
File path: python/tvm/relay/frontend/onnx.py
##########
@@ -195,6 +195,17 @@ def _dim_check(attrs):
return _dim_check, "Only 1d, 2d and 3d kernel supported."
+def get_scalar(x, params, dtype="float32"):
+ """Helper to get a scalar value for Quantized operators."""
+ if isinstance(x, _expr.Var) and x.name_hint in params:
Review comment:
QLinearConv depends on falling back to `op.cast` in the case that x.name_hint is not in params, not throwing error.
--
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] anwang2009 commented on pull request #8907: Add support for QLinearConcat contrib op
Posted by GitBox <gi...@apache.org>.
anwang2009 commented on pull request #8907:
URL: https://github.com/apache/tvm/pull/8907#issuecomment-910918400
cc @AndrewZhaoLuo @mbrookhart
--
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] tmoreau89 commented on pull request #8907: [ONNX] Add support for QLinearConcat contrib op
Posted by GitBox <gi...@apache.org>.
tmoreau89 commented on pull request #8907:
URL: https://github.com/apache/tvm/pull/8907#issuecomment-914526952
It seems we have a CI error which I'm not sure relates to the PR at all:
```
java.io.IOException: java.lang.RuntimeException: Failed to serialize hudson.model.Actionable#actions for class org.jenkinsci.plugins.workflow.job.WorkflowRun
```
--
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