Skip to content

Add function-body to SoftmaxGrad#6988

Merged
gramalingam merged 6 commits into
masterfrom
rama/SoftmaxGradFunction
Mar 25, 2021
Merged

Add function-body to SoftmaxGrad#6988
gramalingam merged 6 commits into
masterfrom
rama/SoftmaxGradFunction

Conversation

@gramalingam
Copy link
Copy Markdown
Contributor

Description:
(a) Add function-body to the SoftmaxGrad schema.
(b) Add test-cases to validate the function-body by running the op and the expanded function-body using the CPU provider to establish equivalence.
(c) Add type-context when calling the function-body-builder (which will be required for other ops for typed constants).

Motivation and Context
This will enable, for example, translation of ONNX graphs containing calls to SoftmaxGrad to ONNX-MLIR using the function mechanism.

@gramalingam gramalingam requested a review from a team as a code owner March 11, 2021 22:34
input_types.emplace_back(type);
} else
input_types.emplace_back();
}
Copy link
Copy Markdown
Contributor

@ke1337 ke1337 Mar 11, 2021

Choose a reason for hiding this comment

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

nit: May use node.ForEachDef() for simplicity. It handles non-exist cases. #Resolved

Copy link
Copy Markdown
Contributor Author

@gramalingam gramalingam Mar 12, 2021

Choose a reason for hiding this comment

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

We don't want to skip the non-exist cases here: we want to push a default-value for these cases. So, I am not sure we can do that. #Resolved

for (const auto& attr : node.attributes) {
*(np->add_attribute()) = attr.proto;
}
}
Copy link
Copy Markdown
Contributor

@ke1337 ke1337 Mar 12, 2021

Choose a reason for hiding this comment

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

np->CopyFrom()? #Resolved

Copy link
Copy Markdown
Contributor Author

@gramalingam gramalingam Mar 12, 2021

Choose a reason for hiding this comment

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

The source is a NodeDef, not a NodeProto. #Resolved

Comment thread orttraining/orttraining/test/gradient/function_ops_test.cc
@souptc
Copy link
Copy Markdown
Member

souptc commented Mar 25, 2021

LGTM

@souptc souptc self-requested a review March 25, 2021 17:01
testCase.opsets[kOnnxDomain] = 13;
testCase.opsets[kMSDomain] = 1;

auto model2 = testCase.CreateModel();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

sign off

@gramalingam gramalingam merged commit cc0e7be into master Mar 25, 2021
@gramalingam gramalingam deleted the rama/SoftmaxGradFunction branch March 25, 2021 18:34
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.

3 participants