-
Notifications
You must be signed in to change notification settings - Fork 516
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
[MHLO] Init end-to-end unit tests #1223
Conversation
84d938d
to
23e3836
Compare
As @silvasean mentioned #1025 (comment) before. This PR adds the MHLO end-to-end unit tests to CI. It lowers MHLO to Linalg and run it on Linalg-On-Tensors backend. Please review for me @silvasean @ZihengJiang @Vremold |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
23e3836
to
a509961
Compare
@silvasean I can't reproduce the CI failure locally with the following environments:
My testing script is:
|
@fortianyou . I'm about to send a PR that dockerizes CI. This should help with local reproducers. Once that's out, could you try to rebase on that and then run these tests locally? It should hopefully eliminate any environmental issues and enable a robust reproducer. |
@fortianyou Here it is: #1225. Please LMK once you rebase if you are able to repro locally. |
@sjain-stanford Thanks! I would love to do that. |
The CI is failing because one of the e2e tests is failing on an assertion. For some reason this causes a cascade of failures in the CI, which can make it hard to tell what is going wrong. (See the assertion error here: https://github.com/llvm/torch-mlir/runs/7840097389?check_suite_focus=true#step:12:9) If you run the tests sequentially, you should also see the assertion error locally, and it should crash the entire program, allowing you to debug further. When I run locally
Here is the relevant backtrace
Note: There are other tests also causing assertion errors. If you run the tests in parallel, you should see the assertion error messages print out before the results are printed out. Let me know if you're able to reproduce things. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome!!!
I've seen better failures when disabling multiprocessing (with the |
(apologies for the accidental closing with a comment :P ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work! Thanks @fortianyou
a509961
to
355720c
Compare
355720c
to
09ee945
Compare
Signed-off-by: Kevin O'Brien <[email protected]>
See RFC #999
Co-authored-by: Bairen Yi [email protected]
Co-authored-by: Jiawei Wu [email protected]
Co-authored-by: Tianyou Guo [email protected]
Co-authored-by: Xu Yan [email protected]
Co-authored-by: Ziheng Jiang [email protected]