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

Add strict_shape_type_inference config option #11081

Merged
merged 15 commits into from
Apr 21, 2022

Conversation

garymm
Copy link
Contributor

@garymm garymm commented Apr 1, 2022

Prior to this, certain shape and type errors were surfaced only when
the model was using the latest known op set version.

Providing users an explicit option allows for better testing of code
that produces models, which includes unit tests within this repo and
other repos such as the TF-ONNX and PT-ONNX converters.

Remove the previous behavior which seems quite counter-intuitive:
an otherwise identical model with a later op set version should be treated
identically in this regard.

The option defaults to false to avoid causing errors for users that
rely on the previous permissive behavior.

Turned on the strict enforcement by default in OpTester, which revealed a few
disagreements between ORT and ONNX on what the correct output shape should
be.

Fix shape inference bug in ReduceSumTraining with noop_with_empty_axes=1
which was revealed.

Fix TensorOpTest.Unsqueeze_scalar, which was testing negative axes on an
op set version where the op did not actually support negative axes.

Fixes #9506.

Prior to this, certain shape and type errors were surfaced only when
the model was using the latest known op set version.

Providing users an explicit option allows for better testing of code
that produces models, which includes unit tests within this repo and
other repos such as the TF-ONNX and PT-ONNX converters.

The option defaults to false to avoid causing errors for users that
rely on the previous permissive behavior.

Fixes microsoft#9506.
@garymm garymm marked this pull request as ready for review April 5, 2022 02:56
@garymm garymm requested a review from pranavsharma April 5, 2022 02:57
@garymm
Copy link
Contributor Author

garymm commented Apr 5, 2022

@pranavsharma could you please review or find someone else who can review? I'm not very familiar with who would be a good reviewer.

@garymm garymm changed the title Add strict_shape_type_inference session option Add strict_shape_type_inference config option Apr 12, 2022
@garymm
Copy link
Contributor Author

garymm commented Apr 15, 2022

@skottmckay I'd appreciate a review before the weekend if possible. Thanks!

Copy link
Contributor

@skottmckay skottmckay left a comment

Choose a reason for hiding this comment

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

:shipit:

@garymm
Copy link
Contributor Author

garymm commented Apr 19, 2022

@pranavsharma the web CI pipelines seem to be timing out consistently. Any way to skip them?
Is anyone working on fixing it? Should I file a GitHub issue?

@pranavsharma
Copy link
Contributor

@pranavsharma the web CI pipelines seem to be timing out consistently. Any way to skip them? Is anyone working on fixing it? Should I file a GitHub issue?

@fs-eire - can you please take a look?

@garymm garymm merged commit 7aa4af2 into microsoft:master Apr 21, 2022
@garymm garymm deleted the strict-shape-inference-option branch April 21, 2022 15:32
seddonm1 pushed a commit to seddonm1/onnxruntime that referenced this pull request May 15, 2022
Prior to this, certain shape and type errors were surfaced only when
the model was using the latest known op set version.

Providing users an explicit option allows for better testing of code
that produces models, which includes unit tests within this repo and
other repos such as the TF-ONNX and PT-ONNX converters.

Remove the previous behavior which seems quite counter-intuitive:
an otherwise identical model with a later op set version should be treated
identically in this regard.

The option defaults to false to avoid causing errors for users that
rely on the previous permissive behavior.

Turned on the strict enforcement by default in OpTester, which revealed a few
disagreements between ORT and ONNX on what the correct output shape should
be.

Fix shape inference bug in ReduceSumTraining with noop_with_empty_axes=1
which was revealed.

Fix TensorOpTest.Unsqueeze_scalar, which was testing negative axes on an
op set version where the op did not actually support negative axes.

Fixes microsoft#9506.
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.

Expose option to strictly enforce shape and type correctness
4 participants