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

[FIX] symbolic shape infer error with onnx-1.11.0 #10674

Merged
merged 4 commits into from
Mar 17, 2022

Conversation

PeixuanZuo
Copy link
Contributor

@PeixuanZuo PeixuanZuo commented Feb 25, 2022

When the input shape of the op is uncertain, the inferred output shape is uncertain.
Before onnx1.11.0, the uncertain shape dim is represented by None. In onnx 1.11.0, the uncertain shape is represented by "unk__#index". The method of verifying whether the output shape is certained invalid.
For example, a Where op has three inputs[1,1,min(1024,dim0), min(1024,dim1)],[1,12,dim0,dim1],[], the infered output has shape [1,12,None,None] in onnx1.10 but has shape[1,12,"unk__0",'unk__1'] in onnx1.11.

It's caused by https://github.com/onnx/onnx/blob/main/onnx/shape_inference/implementation.cc#L437, the symbolic shape generated before it's assign to output.

  1. Add the logic to distinguish if a symbolic dim represent None.
  2. Update the changed onnx model : gpt2_megatron_opt.onnx

Fixes #10761

@jywu-msft
Copy link
Member

Can you re-enable tests for symbolic shape infer that were disabled by https://github.com/microsoft/onnxruntime/pull/10441/files#diff-95f6d8002a2b02785895316d5a8268de674a67e7ed3fc0bae0e7274a762a144b
to test this change?
+@liqunfu FYI , can you also help validate?

@thiagocrepaldi
Copy link
Contributor

@baijumeswani That reminds me of the issue you were seeing

Copy link
Contributor

@liqunfu liqunfu 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 this fix. Please remove this line in the test to enable the test:

test_skip_due_to_onnx_1_11_shape_inference_change = ["GPT2", "GPT2_LM_HEAD", "test_GPT2"]

@PeixuanZuo PeixuanZuo force-pushed the peixuanzuo/symbolic_shape_infer_for_onnx1110 branch from 653b927 to 3678858 Compare March 10, 2022 05:48
@PeixuanZuo
Copy link
Contributor Author

Can you re-enable tests for symbolic shape infer that were disabled by https://github.com/microsoft/onnxruntime/pull/10441/files#diff-95f6d8002a2b02785895316d5a8268de674a67e7ed3fc0bae0e7274a762a144b to test this change? +@liqunfu FYI , can you also help validate?

I enable tests which were disabled and pass the unit test.

@PeixuanZuo PeixuanZuo requested review from liqunfu, jcwchen and jywu-msft and removed request for pengwa March 16, 2022 03:16
@PeixuanZuo PeixuanZuo merged commit 463fac6 into master Mar 17, 2022
@PeixuanZuo PeixuanZuo deleted the peixuanzuo/symbolic_shape_infer_for_onnx1110 branch March 17, 2022 05:47
lavanyax pushed a commit to intel/onnxruntime that referenced this pull request Mar 29, 2022
* [FIX] symbolic shape infer error with onnx-1.11.0

* [FIX] consider inputs name contains 'unk__'

* [TEST] enable gpt2 test

* [FIX] gpt2_megatron_opt.onnx graph
chilo-ms pushed a commit that referenced this pull request Apr 15, 2022
* [FIX] symbolic shape infer error with onnx-1.11.0

* [FIX] consider inputs name contains 'unk__'

* [TEST] enable gpt2 test

* [FIX] gpt2_megatron_opt.onnx graph
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.

Shape Inference does not work with onnx1.11.0 due to onnx shape inference enchancement
7 participants