Refactor Tests for Multiple ONNX Opsets #20036
Conversation
|
test_topk and test_blacklisted_ops will be failing until merged with #19294 |
|
@houseroad this PR is ready for review :) |
houseroad
left a comment
houseroad
left a comment
There was a problem hiding this comment.
I would suggest to split this into two PRs, the refactor part, and the top-k changes. This may help us merge the first part in quicker.
|
Could you rebase again? |
|
<test_pytorch_onnx_caffe2_opset9.TestCaffe2BackendEmbed_opset9 testMethod=test_c2_generate_proposals> failed. |
zou3519
added
the
triaged
|
rebase? I may take a look if the tests are still failing. |
facebook-github-bot
left a comment
facebook-github-bot
left a comment
There was a problem hiding this comment.
@houseroad has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
houseroad
left a comment
houseroad
left a comment
There was a problem hiding this comment.
Looks good. Finally we are able to land this.
| {"name": "pads", "ints": [0, 0], "type": 7}, | ||
| {"name": "strides", "ints": [1], "type": 7}]}] | ||
| ops = {9 : ops_9, 10 : ops_10} | ||
| ops = {10 : ops_10} |
There was a problem hiding this comment.
Good catch. Why did it fail before?
There was a problem hiding this comment.
check_onnx_opsets_operator() runs tests for opset versions in opset_versions, so it didn't run it for opset 9.
| return func(self) | ||
| return wrapper | ||
|
|
||
| # skips tests for all versions below min_opset_version. |
There was a problem hiding this comment.
| self.run_model_test(torch.nn.CosineSimilarity(dim=1, eps=1e-6), train=False, | ||
| input=(x, y), batch_size=BATCH_SIZE, use_gpu=False) | ||
|
|
||
| @skipIfUnsupportedOpsetVersion([10]) |
There was a problem hiding this comment.
LSTM exports with Slice ops (Slice exports with all inputs in opset 10 rather than one input and attributes, which is not the case with the caffe2 op)
There was a problem hiding this comment.
I see, make sense. I would suggest to add this context into the comments later.
|
Feel free to address the comments in the following PR. |
| dict(TestCaffe2Backend.__dict__, embed_params=True)) | ||
|
|
||
| # opset 10 tests | ||
| TestCaffe2Backend_opset10 = type(str("TestCaffe2Backend_opset10"), |
There was a problem hiding this comment.
Also I am wondering if we can renamed TestCaffe2Backend and TestCaffe2BackendEmbed to TestCaffe2Backend_opset9 and TestCaffe2BackendEmbed_opset10
If so, let's do it in the following PRs.
|
Thank you @houseroad !! |
|
@houseroad merged this pull request in 45c6fa0. |
7 participants
Refactor tests for #19294.