-
Notifications
You must be signed in to change notification settings - Fork 555
testcase failing due to commit 7b23a1f5d87064daded4b89487dc968fd933051e #4079
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
Comments
@amd-vivekag I can't reproduce this issue. The test passed: PASSED onnx/node/generated/test_averagepool_2d_ceil/run_module_io_flags.txt::model.mlir::cpu_llvm_sync I followed the instructions here: The printout above came from this command: pytest -n auto -rA --timeout=30 --durations=20 --config-files=configs/onnx_ops_cpu_llvm_sync.json --report-log=/tmp/onnx_ops_cpu_logs.json I made sure that the torch-mlir-opt referred to is the one I just sync'ed, built and verified that has my average pooling change. $ which torch-mlir-opt Can you please let me know which steps you used to reproduce this issue? |
Hi @amd-vivekag, I have not heard back from you in a month. Are things working now? If yes, can this issue be closed? |
Hi @ivangarcia44, I'm sorry, I somehow missed the last month message. I was seeing the failure in CI with the CL mentioned above. I'll have to check if I was able to reproduce it locally (as far as I remember, I was able to reproduce it). Thanks for following up. Really appreciate it. |
Hi @ivangarcia44 , I'm able to reproduce the issue. I followed following steps:
You should see following message:
Please let me know if you need any other information from my side in this regard. |
@amd-vivekag I am able to reproduce the failure. I am taking a look at it. |
This issue is fixed here: #4144 |
…e the kernel can go out of bounds (#4144) In this pull request I added various E2E tests to cover edge cases on the average pooling torch to linalg lowering algorithm. These new tests uncovered various numerical issues that were addressed in this same PR. One of the issues is the IREE test failure found in #4079 which triggered this work. Background: In the most common case, the divisor for the average pooling is just the product of kernel dimensions. But with padding, and ceil mode options, some elements need to be discounted from the divisor computation. This change fixes two components of this: Fix the condition that determines if the divisor is just the product of kernel dimensions or the clamped divisor comptutation. Add missing isCountIncludePad logic divisor computation algorithm and reversal of kernel/stride/padding parameters element order. Both were missing from the first generalization change. --------- Co-authored-by: Ivan Garcia <[email protected]> Co-authored-by: ivangarcia44 <ivangarcia44>
The fix for this issue was merged into torch-mlir: #4144. Closing this issue. |
Hi @amd-vivekag, the fix for this issue was merged in torch-mlir: #4144. Can you please close this issue when you get a chance? I can't close it. |
The following commit (7b23a1f) has caused the testcase failure:
https://github.com/iree-org/iree-test-suites/tree/main/onnx_ops/onnx/node/generated/test_averagepool_2d_ceil/
This is blocking bump torchmlir in IREE.
Getting error:
After removing this commit, the error is not there.
The text was updated successfully, but these errors were encountered: