-
Notifications
You must be signed in to change notification settings - Fork 319
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
Einsum decomposition #1537
Einsum decomposition #1537
Conversation
Can one of the admins verify this patch? |
6dc9f50
to
3e6b9ff
Compare
Can one of the admins verify this patch? |
4 similar comments
Can one of the admins verify this patch? |
Can one of the admins verify this patch? |
Can one of the admins verify this patch? |
Can one of the admins verify this patch? |
@jenkins-droid test this please |
@sorenlassen thank you for the patch! Could you please sign DCO by following its instruction (click Details by DCO)? To avoid this, we often sign our commits by |
Can one of the admins verify this patch? |
1 similar comment
Can one of the admins verify this patch? |
04f3684
to
d4b0a4e
Compare
Can one of the admins verify this patch? |
oops, I DCO signed the last commits now I also ran clang-format just now |
I think we need to sign all the previous commits. |
I had signed the initial batch of commits yesterday but hadn't signed the follow-on commits today when I ran the DCO suggested command shall I signoff on the MHLO commit to make the DCO check happy? |
Can one of the admins verify this patch? |
@jenkins-droid test this please |
Can one of the admins verify this patch? |
492e165
to
4c37939
Compare
Can one of the admins verify this patch? |
I rebased and went ahead and signed off on other people's MHLO and SeqType/LoopOp commits, hopefully this is all right and will pass the DCO check |
Can one of the admins verify this patch? |
Signed-off-by: Soren Lassen <sorenlassen@gmail.com>
Added Einsum to OpsWithVerifier list in gen_onnx_mlir.py Ran utils/gen_onnx_mlir.py Moved utils/ONNXOps.td.inc to src/Dialect/ONNX/ Moved utils/OpBuildTable.inc to src/Builder (but was unchanged) TODO: fill in the skeleton verifier for Einsum Signed-off-by: Soren Lassen <sorenlassen@gmail.com>
Signed-off-by: Soren Lassen <sorenlassen@gmail.com>
Signed-off-by: Soren Lassen <sorenlassen@gmail.com>
Implemented for batch_diagonal, inner_prod, batch_matmul, transpose, sum Tested with TEST_VERBOSE=true \ TEST_CASE_BY_USER="test_einsum_batch_diagonal_cpu test_einsum_batch_matmul_cpu test_einsum_inner_prod_cpu test_einsum_transpose_cpu test_einsum_sum_cpu" \ PYTHONPATH=$PWD/build/test/backend/Debug \ python3 test/backend/test.py Signed-off-by: Soren Lassen <sorenlassen@gmail.com>
build and run with: cd build cmake --build . --target Debug/bin/TestONNXEinsumOp && ctest -R TestONNXEinsumOp --verbose Signed-off-by: Soren Lassen <sorenlassen@gmail.com>
Signed-off-by: Soren Lassen <sorenlassen@gmail.com>
and used it in DecomposeEinsum Signed-off-by: Soren Lassen <sorenlassen@gmail.com>
Can one of the admins verify this patch? |
@jenkins-droid test this please |
Added new matmulToMemref() with the old behavior and updated existing callers. Used matmul() without toMemref() in DecomposeEinsum. Signed-off-by: Soren Lassen <sorenlassen@gmail.com>
Can one of the admins verify this patch? |
and used it in DecomposeEinsum Signed-off-by: Soren Lassen <sorenlassen@gmail.com>
Can one of the admins verify this patch? |
@jenkins-droid test this please |
Signed-off-by: Soren Lassen <sorenlassen@gmail.com>
Can one of the admins verify this patch? |
@jenkins-droid test this please |
@sorenlassen could you please check why the lit test in |
it succeeds for me locally, as far as I can tell is the CI failing? how can I see the failure? |
Yes, can you view the CI logs when clicking Here is the log:
|
It passed for me locally but failed in the CI and I coudn't tell why. It wasn't important anyway. Signed-off-by: Soren Lassen <sorenlassen@gmail.com>
Can one of the admins verify this patch? |
@tungld thanks for the info It passed for me locally and I can't tell why it fails in the CO. It wasn't important anyway, so I removed it now. |
@jenkins-droid test this please |
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. Thank you very much!
thanks for the review! could you merge it for me? |
Yes, sure. I am waiting for comments from other reviewers if any. Otherwise, I will merge it tomorrow. |
This PR implements ONNXEinsumOp verify(), inferShapes(), and decomposes it into a set of supported ops (Constant, Squeeze, Unsqueeze, Mul, ReduceSum, Where, Transpose).
Closes #1367
It is based on a preexisting python implementation here:
https://github.com/sorenlassen/onnx-fun/blob/main/python/src/einsummer.py
Testing:
Added the onnx einsum tests to
inference_backend.py
and ran them with the command:TEST_VERBOSE=true TEST_CASE_BY_USER="test_einsum_batch_diagonal_cpu test_einsum_batch_matmul_cpu test_einsum_inner_prod_cpu test_einsum_transpose_cpu test_einsum_sum_cpu" cmake --build . --target check-onnx-backend
Added shape inference tests in
onnx_shape_inference.mlir
Added a unit test which can be run with:
ctest -R TestONNXEinsumOp --verbose