Conversation
| namespace onnxruntime { | ||
|
|
||
| // EmbedLayerNorm supports limited data types. | ||
| static std::vector<std::string> supported_data_types{"tensor(float16)", "tensor(float)", "tensor(int32)"}; |
There was a problem hiding this comment.
Array can be initialized at compile time but vector can't.
| // Get input "input_ids" from node. | ||
| NodeArg* input_ids = nullptr; | ||
| if (!graph_utils::NodeArgIsConstant(graph, *(word_gather_node.MutableInputDefs()[1])) && | ||
| !graph_utils::IsGraphInput(graph, word_gather_node.MutableInputDefs()[1])) { |
There was a problem hiding this comment.
Shall we add check of first input for 3 Gather nodes? For example, the input has the initializer of 2D tensor.
| continue; | ||
| } | ||
| Node& reduce_sum_node = *graph.GetNode(edges[0]->GetNode().Index()); | ||
| if (reduce_sum_node.GetOutputEdgesCount() != 1) { |
There was a problem hiding this comment.
Reduce_sum could have multiple output edges. For Bert model with 12 layers, there are 12 Attention nodes so output edge count could be 12. I think it is fine to remove the check of output edge count since those edges will linked to the mask index output of embed layer when fusion is done.
| continue; | ||
| } | ||
|
|
||
| std::vector<graph_utils::EdgeEndToMatch> position_embedding_path_symbolic{ |
There was a problem hiding this comment.
If second input of position gather is constant initializer, it is better to add code the check that the const like like [1, 2, 3, ...].
There was a problem hiding this comment.
How about using logic like:
If gather input is (constant) initializer {
check initializer
}
else {
match two paths:
expand -- shape --input_ids
expand -- ... --shape--input_ids
if not matched return.
}
| {0, 0, "Unsqueeze", {1, 11}, kOnnxDomain}, | ||
| {0, 0, "Gather", {1, 11}, kOnnxDomain}, | ||
| {0, 0, "Shape", {1}, kOnnxDomain}, | ||
| }; |
There was a problem hiding this comment.
Also check that shape's parent is input_ids. The edges of ? has not been checked in your code:
input_ids -?- Shape -?- Expand -- Gather
|?
+-Shape--(this Path to Expand)
tianleiwu
left a comment
There was a problem hiding this comment.
The improvements can be in another PR. Approve it to check in so that other changes depending on it could continue.
tianleiwu
left a comment
There was a problem hiding this comment.
Add logic to check the shapes of three inputs (input_ids, segment_ids and mask): dimenion 1 is known and all have same value.
Description:
Motivation and Context
It is used to optimize Bert model performance.