Skip to content
This repository has been archived by the owner on Oct 13, 2021. It is now read-only.

Adjust input output sizes when any dim is None #480

Merged
merged 1 commit into from
May 8, 2020

Conversation

buddhapuneeth
Copy link
Contributor

Whenever inputs and outputs have None as dim (except for batch size), after conversion they will have 0 as the dimension.
Batch size will be converted to 'N'.
input(None, None) -> input(N, 0)

With this fix even other dims will have variables like M1, M2.
input(None, None) -> input(N, M1)

Whenever inputs and outputs have None as dim (except for batch size), after conversion they will have 0 as the dimension.
Batch size will be converted to 'N'.
input(None, None) -> input(N, 0)

With this fix even other dims will have variables like M1, M2.
input(None, None) -> input(N, M1)
@CLAassistant
Copy link

CLAassistant commented May 8, 2020

CLA assistant check
All committers have signed the CLA.

Copy link
Collaborator

@jiafatom jiafatom left a comment

Choose a reason for hiding this comment

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

Thanks for your efforts. The concern here is the benefit of showing symbolic dim in inputs. If the model shows M1, M2 as input dimension, it does not propagate to the output size, so it looks like an isolated dim. Batch size dim is different, since the output dim also shows N, so we know the corresponding dim in outputs.

@buddhapuneeth
Copy link
Contributor Author

@jiafatom currently if we don't make it M1, M2.. the inputs and outputs where dim = None will be converted to 0. I know the parameterized dimension won't be propagated over the layers. I mean in some cases input's M1 might be same as output's M2. But without this fix, on the inference side we are having issues as it is expecting dim as 0.
Simple example (embedded layer) to illustrate this:

without this fix:
Embedding_withoutfix

with fix:
Embedding_withfix

Please provide your feedback/suggestion on this.

@wenbingl
Copy link
Member

wenbingl commented May 8, 2020

Thanks a lot for your contribution!

@wenbingl wenbingl merged commit 89b0c45 into onnx:master May 8, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants