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

Spec: Clarify Transpose #4611

Open
justinchuby opened this issue Oct 20, 2022 · 5 comments
Open

Spec: Clarify Transpose #4611

justinchuby opened this issue Oct 20, 2022 · 5 comments
Labels
contributions welcome question Questions about ONNX spec clarification Clarification of the ONNX spec needed

Comments

@justinchuby
Copy link
Contributor

justinchuby commented Oct 20, 2022

Ask a Question

Question

It is unclear what types of tensors Transpose accepts from the spec. For example, does it accept 1d or 0d tensors as inputs? If so can perm be an empty list? Is perm optional since it has words like "by default"? Does len(perm) need to be rank(input)?

Further information

  • Relevant Area: operators

  • Is this issue related to a specific model?

No

Notes

@justinchuby justinchuby added the question Questions about ONNX label Oct 20, 2022
@justinchuby justinchuby changed the title Spec: Transpose Spec: Clarify Transpose Oct 20, 2022
@jcwchen
Copy link
Member

jcwchen commented Oct 25, 2022

Hi @justinchuby,
I think these corner cases might just be undefined behavior.

does it accept 1d or 0d tensors as inputs?

IIUC, at least from onnx shape inference perspective, they should be both fine case if not giving specified perm.

So can perm be an empty list?

Shape inference will stop if the given perm is an empty list according to this line.

Note that since current ONNX spec does not explicitly disallow these, probably the downstream runtime tools' behavior for these cases might also be undefined (either can work or cannot work).

Is perm optional since it has words like "by default"? Does len(perm) need to be rank(input)?

perm is optional. Definitely for this case I agree we can add as perm (optional): list of ints , although the following description has explicitly mentioned "by default".

Does len(perm) need to be rank(input)?

Currently shape inference does not forbid it, but I think we can add a check for it.

Feel free to submit a PR to resolve these ambiguity if you have bandwidth. Thanks!

@jcwchen jcwchen added the spec clarification Clarification of the ONNX spec needed label Oct 25, 2022
@justinchuby
Copy link
Contributor Author

If the input is 0d, I would expect perm to be an empty list. Would that be reasonable?

@jcwchen
Copy link
Member

jcwchen commented Oct 25, 2022

If the input is 0d, I would expect perm to be an empty list. Would that be reasonable?

Makes sense to me if we want to explicitly add the restriction: len(perm) needs to be rank(input).

@gramalingam
Copy link
Contributor

If the input is 0d, I would expect perm to be an empty list. Would that be reasonable?

Yes. I would say that the current behavior is a bug/limitation of the shape-inference code.

@gramalingam
Copy link
Contributor

I also tried out the ONNX reference implementation. It fails for a scalar (0D) input ... I assume this is a fixable limitation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contributions welcome question Questions about ONNX spec clarification Clarification of the ONNX spec needed
Projects
Development

No branches or pull requests

3 participants