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/08/23 12:50:45 UTC
[GitHub] [tvm] chengven027-intellif opened a new pull request, #12555: fix pad constant value is none
chengven027-intellif opened a new pull request, #12555:
URL: https://github.com/apache/tvm/pull/12555
hi, cc @AndrewZhaoLuo
When I import my onnx model. I found my model get a error with pad. because the `pad` constant value is none. shown as follow:
<img width="729" alt="1661258976676" src="https://user-images.githubusercontent.com/50271153/186162163-c3727f00-1725-43d9-bc0e-09d872afed08.png">
It`s just a very little bug. So I fix it and no unnit test.
--
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] chengven027-intellif commented on pull request #12555: [frontend][onnx]fix pad constant value is none
Posted by GitBox <gi...@apache.org>.
chengven027-intellif commented on PR #12555:
URL: https://github.com/apache/tvm/pull/12555#issuecomment-1244742618
Hey, @AndrewZhaoLuo Have you seen this PR, and any update with me? thanks.
--
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 diff in pull request #12555: [frontend][onnx]fix pad constant value is none
Posted by GitBox <gi...@apache.org>.
AndrewZhaoLuo commented on code in PR #12555:
URL: https://github.com/apache/tvm/pull/12555#discussion_r952897732
##########
python/tvm/relay/frontend/onnx.py:
##########
@@ -1403,7 +1403,7 @@ def _impl_v2(cls, inputs, attr, params):
@classmethod
def _impl_v11(cls, inputs, attr, params):
pads = inputs[1]
- if len(inputs) == 3:
+ if len(inputs) == 3 and input[2] is not None:
Review Comment:
Reading the docs https://github.com/onnx/onnx/blob/main/docs/Operators.md#Pad we should also be checking if input[2] is also an empty string or False.
Pythonically we can just do `and input[2]` (though I would add a comment since this handles multiple types)
--
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] chengven027-intellif commented on pull request #12555: [frontend][onnx]fix pad constant value is none
Posted by GitBox <gi...@apache.org>.
chengven027-intellif commented on PR #12555:
URL: https://github.com/apache/tvm/pull/12555#issuecomment-1226856373
Dose this PR need to be reviewed again? @AndrewZhaoLuo @leandron
--
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 #12555: [frontend][onnx]fix pad constant value is none
Posted by GitBox <gi...@apache.org>.
AndrewZhaoLuo commented on PR #12555:
URL: https://github.com/apache/tvm/pull/12555#issuecomment-1233218163
> I am sorry, I really can't create an unit test with empty string of `constant_value`. @leandron @AndrewZhaoLuo
In verify_pad_v11 I can seem to make the following:
```
graph = helper.make_graph(
[node],
"pad_test",
inputs=[
helper.make_tensor_value_info("input", TensorProto.FLOAT, list(indata.shape)),
helper.make_tensor_value_info("pads", TensorProto.INT64, (len(pads),)),
helper.make_tensor_value_info("constant_value", TensorProto.STRING, (1,)),
],
initializer=[
helper.make_tensor("pads", TensorProto.INT64, (len(pads),), pads),
helper.make_tensor("constant_value", TensorProto.STRING, (1,), [b""]),
],
outputs=[
helper.make_tensor_value_info("output", TensorProto.FLOAT, list(outdata.shape))
],
)
```
However, my version of ONNXRT can't seem to accept the string type. What have you tried?
--
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] chengven027-intellif commented on a diff in pull request #12555: [frontend][onnx]fix pad constant value is none
Posted by GitBox <gi...@apache.org>.
chengven027-intellif commented on code in PR #12555:
URL: https://github.com/apache/tvm/pull/12555#discussion_r955791479
##########
tests/python/frontend/onnx/test_forward.py:
##########
@@ -2027,6 +2027,7 @@ def verify_pad_v11(indata, pads, mode="constant", value=0.0):
verify_pad_v11(np.random.randn(2, 2).astype(np.float32), [0, 1, 0, 0], "constant", 0.0)
verify_pad_v11(np.random.randn(2, 3).astype(np.float32), [1, 0, 0, 1], "constant", 0.0)
verify_pad_v11(np.random.randn(3, 2).astype(np.float32), [0, 0, 1, 0], "constant", 5.0)
+ verify_pad_v11(np.random.randn(3, 2).astype(np.float32), [0, 0, 1, 0], "constant", False)
Review Comment:
No, But I can't add a test case with `None`.
First, In the doc `test_forward.py test_pad`, The `value` is a default number,can't set `None`.
`def verify_pad_v11(indata, pads, mode="constant", value=0.0)`
Then, the input node 'constant_value' is a `TensorProto.FLOAT`. not `None` too.
Can you give me some suggestions to add a test case with 'None'? Thanks very much.
--
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] chengven027-intellif closed pull request #12555: [frontend][onnx]fix pad constant value is none
Posted by GitBox <gi...@apache.org>.
chengven027-intellif closed pull request #12555: [frontend][onnx]fix pad constant value is none
URL: https://github.com/apache/tvm/pull/12555
--
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 #12555: [frontend][onnx]fix pad constant value is none
Posted by GitBox <gi...@apache.org>.
AndrewZhaoLuo commented on PR #12555:
URL: https://github.com/apache/tvm/pull/12555#issuecomment-1227491549
> Dose this PR need to be reviewed again? @AndrewZhaoLuo @leandron
Yeah I'll leave it to Leandron B)
--
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 #12555: [frontend][onnx]fix pad constant value is none
Posted by GitBox <gi...@apache.org>.
AndrewZhaoLuo commented on PR #12555:
URL: https://github.com/apache/tvm/pull/12555#issuecomment-1238383912
Sorry, I'll take a look tomorrow.
--
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] chengven027-intellif commented on pull request #12555: [frontend][onnx]fix pad constant value is none
Posted by GitBox <gi...@apache.org>.
chengven027-intellif commented on PR #12555:
URL: https://github.com/apache/tvm/pull/12555#issuecomment-1234999438
> Can you share your model by any chance? Can you also run your model via ONNX-RT?
[coat_model.zip](https://github.com/apache/tvm/files/9474424/coat_model.zip)
1. model have been uploaded.
2. The model can pass by ONNX-RT. And the floating point comparison between tvm and onnx-RT result can also pass.
--
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] chengven027-intellif commented on a diff in pull request #12555: [frontend][onnx]fix pad constant value is none
Posted by GitBox <gi...@apache.org>.
chengven027-intellif commented on code in PR #12555:
URL: https://github.com/apache/tvm/pull/12555#discussion_r955791479
##########
tests/python/frontend/onnx/test_forward.py:
##########
@@ -2027,6 +2027,7 @@ def verify_pad_v11(indata, pads, mode="constant", value=0.0):
verify_pad_v11(np.random.randn(2, 2).astype(np.float32), [0, 1, 0, 0], "constant", 0.0)
verify_pad_v11(np.random.randn(2, 3).astype(np.float32), [1, 0, 0, 1], "constant", 0.0)
verify_pad_v11(np.random.randn(3, 2).astype(np.float32), [0, 0, 1, 0], "constant", 5.0)
+ verify_pad_v11(np.random.randn(3, 2).astype(np.float32), [0, 0, 1, 0], "constant", False)
Review Comment:
No, But I can't add a test case with `None`.
First, In the doc `test_forward.py test_pad`, The `value` is a default number `0.0`,can't set `None`.
`def verify_pad_v11(indata, pads, mode="constant", value=0.0)`
Then, the input node `constant_value` is a `TensorProto.FLOAT`. not `None` too.
Can you give me some suggestions to add a test case with 'None'? Thanks very much.
--
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] leandron commented on a diff in pull request #12555: [frontend][onnx]fix pad constant value is none
Posted by GitBox <gi...@apache.org>.
leandron commented on code in PR #12555:
URL: https://github.com/apache/tvm/pull/12555#discussion_r955147503
##########
tests/python/frontend/onnx/test_forward.py:
##########
@@ -2027,6 +2027,7 @@ def verify_pad_v11(indata, pads, mode="constant", value=0.0):
verify_pad_v11(np.random.randn(2, 2).astype(np.float32), [0, 1, 0, 0], "constant", 0.0)
verify_pad_v11(np.random.randn(2, 3).astype(np.float32), [1, 0, 0, 1], "constant", 0.0)
verify_pad_v11(np.random.randn(3, 2).astype(np.float32), [0, 0, 1, 0], "constant", 5.0)
+ verify_pad_v11(np.random.randn(3, 2).astype(np.float32), [0, 0, 1, 0], "constant", False)
Review Comment:
Is setting `False` here (as opposed to `None`) reproducing the same issue this patch is fixing?
--
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 #12555: [frontend][onnx]fix pad constant value is none
Posted by GitBox <gi...@apache.org>.
AndrewZhaoLuo commented on PR #12555:
URL: https://github.com/apache/tvm/pull/12555#issuecomment-1234638720
Can you share your model by any chance? Can you also run your model via ONNX-RT?
--
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] chengven027-intellif commented on pull request #12555: [frontend][onnx]fix pad constant value is none
Posted by GitBox <gi...@apache.org>.
chengven027-intellif commented on PR #12555:
URL: https://github.com/apache/tvm/pull/12555#issuecomment-1231451063
I am sorry, I really can't create an unit test with empty string of `constant_value`.
@leandron @AndrewZhaoLuo
--
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] chengven027-intellif commented on pull request #12555: [frontend][onnx]fix pad constant value is none
Posted by GitBox <gi...@apache.org>.
chengven027-intellif commented on PR #12555:
URL: https://github.com/apache/tvm/pull/12555#issuecomment-1233658510
Yes, I tried many times but I got the same error as yours.
`onnxruntime.capi.onnxruntime_pybind11_state.InvalidGraph: [ONNXRuntimeError] : 10 : INVALID_GRAPH : This is an invalid model. Type Error: Type 'tensor(string)' of input parameter (constant_value) of operator (Pad) in node () is invalid.`
--
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] chengven027-intellif commented on a diff in pull request #12555: [frontend][onnx]fix pad constant value is none
Posted by GitBox <gi...@apache.org>.
chengven027-intellif commented on code in PR #12555:
URL: https://github.com/apache/tvm/pull/12555#discussion_r955791479
##########
tests/python/frontend/onnx/test_forward.py:
##########
@@ -2027,6 +2027,7 @@ def verify_pad_v11(indata, pads, mode="constant", value=0.0):
verify_pad_v11(np.random.randn(2, 2).astype(np.float32), [0, 1, 0, 0], "constant", 0.0)
verify_pad_v11(np.random.randn(2, 3).astype(np.float32), [1, 0, 0, 1], "constant", 0.0)
verify_pad_v11(np.random.randn(3, 2).astype(np.float32), [0, 0, 1, 0], "constant", 5.0)
+ verify_pad_v11(np.random.randn(3, 2).astype(np.float32), [0, 0, 1, 0], "constant", False)
Review Comment:
No, But I can't add a test case with `None`.
First, In the doc `test_forward.py test_pad`, The `value` is a default number `0.0`,can't set `None`.
`def verify_pad_v11(indata, pads, mode="constant", value=0.0)`
Then, the input node 'constant_value' is a `TensorProto.FLOAT`. not `None` too.
Can you give me some suggestions to add a test case with 'None'? Thanks very much.
--
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 #12555: [frontend][onnx]fix pad constant value is none
Posted by GitBox <gi...@apache.org>.
AndrewZhaoLuo commented on PR #12555:
URL: https://github.com/apache/tvm/pull/12555#issuecomment-1248946899
Oops, I am so sorry, I have been very busy lately. I will try to get to this this weekend
--
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