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

Extend shape op to return a slice #3580

Merged
merged 5 commits into from
Jul 20, 2021
Merged

Extend shape op to return a slice #3580

merged 5 commits into from
Jul 20, 2021

Conversation

gramalingam
Copy link
Contributor

Signed-off-by: Ganesan Ramalingam grama@microsoft.com

Description
Extend the shape op by adding optional attributes start/end so that the return value is input_tensor.shape[start:end].

Motivation and Context
Helps abbreviate a common pattern, eg., of extracting a batch-size from a tensor. Note that this extension is an alternative to introducing a separate "Dim" op (see PR: #3574 ), so only one of the two PRs is required.

Signed-off-by: Ganesan Ramalingam <grama@microsoft.com>
@gramalingam gramalingam requested review from a team as code owners July 14, 2021 20:36
Signed-off-by: Ganesan Ramalingam <grama@microsoft.com>
@gramalingam gramalingam added the operator Issues related to ONNX operators label Jul 15, 2021
@gramalingam gramalingam added this to the 1.10 milestone Jul 15, 2021
@gramalingam gramalingam mentioned this pull request Jul 15, 2021
@rajeevsrao
Copy link
Contributor

ONNX-TensorRT can support either this or the original (Dim op) proposal, with a slight preference for using this approach (using shape op and return slice).

auto* output_length = output_shape->add_dim();


if (!hasNInputShapes(ctx, 1)) {
return;
}

if (ctx.getInputType(0)->tensor_type().has_shape()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

is this if required? if (!hasNInputShapes(ctx, 1)) is checking the same thing right?

int64_t end = getAttribute(ctx, "end", rank);
if (end < 0)
end += rank;
output_length->set_dim_value(end - start);
Copy link
Contributor

Choose a reason for hiding this comment

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

validate the end-start before setting dim_value?
make sure end -start is positive > 0

Copy link
Contributor

Choose a reason for hiding this comment

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

@gramalingam @askhade : Is it worth explicitly calling out the behavior in the spec that by setting start > end (and start == end because of the exclusive nature of end) the resultant shape slice will be empty (over it being an error) ? Just by reading the spec, this portion was unclear to me until I saw the shape inference. (Sorry if I overlooked something obvious in the spec)

@askhade
Copy link
Contributor

askhade commented Jul 19, 2021

I think this approach is better over introducing Dim (PR #3574 )

Signed-off-by: Ganesan Ramalingam <grama@microsoft.com>
Signed-off-by: Ganesan Ramalingam <grama@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
operator Issues related to ONNX operators
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants