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

DML EP add einsum MatMul NHCW ops #13440

Merged
merged 3 commits into from Oct 26, 2022

Conversation

PatriceVignola
Copy link
Contributor

Description

This adds the "NHCW" format support for einsum MatMul. The logic is basically a merge of the existing Transpose and MatMul Einsum implementations.

Motivation and Context

Some transformer models that I'm tracking use Einsum quite often during a single inference, and about half of those were "NHCW" MatMul Einsums. Supporting them will reduce the number of copies to the CPU.

@fdwr fdwr requested a review from sumitsays October 25, 2022 19:24
@@ -176,7 +252,7 @@ void CALLBACK QueryEinSum(IMLOperatorSupportQueryContextPrivate* context, bool*
EinSumHelper helper(attributes);
auto recognizedOperatorType = helper.GetRecognizedOperatorType();
Copy link
Contributor

@fdwr fdwr Oct 25, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I forget if we have shape information at this time, but it would be useful to check if available. There's a comment above saying we don't support 4D in the DML EP, which means hypothetically an existing 5D EinSum that used to run (albeit falling back to CPU) will now fail to run. #Resolved

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, IIRC, the shape of the tensor must be compatible in the expression, and so we do already have enough info to proceed safely, and the existing code should work as-is.

@fdwr fdwr changed the title Add einsum MatMul NHCW ops for DML DML EP add einsum MatMul NHCW ops Oct 25, 2022
Copy link
Contributor

@fdwr fdwr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@PatriceVignola PatriceVignola merged commit ac48bde into main Oct 26, 2022
@PatriceVignola PatriceVignola deleted the user/pavignol/add-einsum-matmul-nhcw branch October 26, 2022 06:09
linnealovespie pushed a commit that referenced this pull request Oct 28, 2022
### Description
This adds the "NHCW" format support for einsum MatMul. The logic is
basically a merge of the existing Transpose and MatMul Einsum
implementations.



### Motivation and Context
Some transformer models that I'm tracking use Einsum quite often during
a single inference, and about half of those were "NHCW" MatMul Einsums.
Supporting them will reduce the number of copies to the CPU.
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

Successfully merging this pull request may close these issues.

None yet

2 participants