-
Notifications
You must be signed in to change notification settings - Fork 3.8k
[TOPI] Implement Einsum with reduction axes #12913
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
Conversation
include/tvm/topi/einsum.h
Outdated
| // Neglecting oshape assign check temporally | ||
| return oshape; | ||
| } | ||
| Array<PrimExpr> NumpyEinsumShape(const std::string subscripts, |
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.
Just curious: what does the “Numpy” prefix stand for?
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.
this is the name used previously, indeed there's no reason using numpy prefix, we can probably find a better name
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.
Sounds good. The function is just doing pure shape inference... Just some random thought, what about “InferEinsumShape”?
| TVM_REGISTER_GLOBAL("topi.einsum").set_body([](TVMArgs args, TVMRetValue* rv) { | ||
| *rv = einsum(args[0], args[1]); | ||
| }); | ||
|
|
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.
Is this no longer used?
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.
it's moved to einsum.cc file
MasterJH5574
left a comment
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.
Thanks @vinx13 for the fix!!
* [TOPI] Implement Einsum with reduction axes * address comments
The current Einsum doesn't utilize
ReduceOpfor reduction. It instead unrolls the whole reduction when defining the computation. It makes the generated result not suitable for analysis as it doesn't contain reduction information. It's also broken for large reductions. This PR provided a new implementation that generates the reduction axes correctly.Graph level optimizations might be needed for efficient Einsum. This PR only provides compute definition for further analysis and optimizations.
cc @spectrometerHBH @MasterJH5574 @junrushao @masahi