Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[iree_tests/onnx/node] Mass convert dims loaded from .npy files into onnx.Constant built into the model.mlir #308

Closed
renxida opened this issue Jul 29, 2024 · 3 comments

Comments

@renxida
Copy link
Contributor

renxida commented Jul 29, 2024

We have a lot of tests in SHARK-TestSuite/iree_tests/onnx/node/generated/ like https://github.com/nod-ai/SHARK-TestSuite/blob/main/iree_tests/onnx/node/generated/test_reduce_l1_do_not_keepdims_example/model.mlir that uses a dim specified as an argument to the func.func & loaded from a npy file.

Since torch_mlir requires a lot of dims to be constant in order to compile, these tests fail, causing a lot of false positives.

I wonder if there is some sort of mass solution that could convert the args to onnx.Constant like:

└──╼ $git diff ./model.mlir
diff --git a/iree_tests/onnx/node/generated/test_reduce_l1_do_not_keepdims_example/model.mlir b/iree_tests/onnx/node/generated/test_reduce_l1_do_not_keepdims_example/model.mlir
index 182a1fc6..8ee10c96 100644
--- a/iree_tests/onnx/node/generated/test_reduce_l1_do_not_keepdims_example/model.mlir
+++ b/iree_tests/onnx/node/generated/test_reduce_l1_do_not_keepdims_example/model.mlir
@@ -1,5 +1,6 @@
 module {
-  func.func @test_reduce_l1_do_not_keepdims_example(%arg0: !torch.vtensor<[3,2,2],f32>, %arg1: !torch.vtensor<[1],si64>) -> !torch.vtensor<[3,2],f32> attributes {torch.onnx_meta.ir_version = 8 : si64, torch.onnx_meta.opset_version = 18 : si64, torch.onnx_meta.producer_name = "backend-test", torch.onnx_meta.producer_version = ""} {
+  func.func @test_reduce_l1_do_not_keepdims_example(%arg0: !torch.vtensor<[3,2,2],f32>) -> !torch.vtensor<[3,2],f32> attributes {torch.onnx_meta.ir_version = 8 : si64, torch.onnx_meta.opset_version = 18 : si64, torch.onnx_meta.producer_name = "backend-test", torch.onnx_meta.producer_version = ""} {
+    %arg1 = "torch.operator"() <{name = "onnx.Constant"}> {torch.onnx.value = dense<2> : tensor<1xsi64>} : () -> !torch.vtensor<[1],si64>
     %none = torch.constant.none
     %0 = torch.operator "onnx.ReduceL1"(%arg0, %arg1) {torch.onnx.keepdims = 0 : si64} : (!torch.vtensor<[3,2,2],f32>, !torch.vtensor<[1],si64>) -> !torch.vtensor<[3,2],f32> 
     return %0 : !torch.vtensor<[3,2],f32>

spicifically, instead of taking arg1 from the func arguments, we have:

%arg1 = "torch.operator"() <{name = "onnx.Constant"}> {

which causes the test cases to pass

@ScottTodd
Copy link
Member

ScottTodd commented Jul 29, 2024

Don't change the tests to make them pass :P

Since torch_mlir requires a lot of dims to be constant in order to compile, these tests fail, causing a lot of false positives.

Those sound like true positives, not false positives? (edit: or true negatives vs false negatives? 🤔)

@renxida
Copy link
Contributor Author

renxida commented Jul 31, 2024

Yup that makes sense. Opening torch-mlir issue to fix this llvm/torch-mlir#3573

@ScottTodd
Copy link
Member

Nothing to do here, closing this issue.
(and iree_tests/onnx/node moved to https://github.com/iree-org/iree-test-suites)

@ScottTodd ScottTodd closed this as not planned Won't fix, can't repro, duplicate, stale Sep 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants