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

Shapeinf for functions #3722

Merged
merged 9 commits into from Jan 5, 2022
Merged

Shapeinf for functions #3722

merged 9 commits into from Jan 5, 2022

Conversation

askhade
Copy link
Contributor

@askhade askhade commented Sep 15, 2021

Description

  • Add shape inference for model local functions. Also includes some name formatting.
    For schema less model local functions where a type and shape inference function is not available, fetch the function proto from model local functions container and traverse the function body to infer the shapes for the function node.

Motivation and Context

  • Why is this change required? What problem does it solve?
  • If it fixes an open issue, please link to the issue here.

Signed-off-by: Ashwini Khade <askhade@microsoft.com>
Signed-off-by: Ashwini Khade <askhade@microsoft.com>
Signed-off-by: Ashwini Khade <askhade@microsoft.com>
Signed-off-by: Ashwini Khade <askhade@microsoft.com>
@askhade askhade requested a review from a team as a code owner September 15, 2021 22:29
@askhade askhade added run release CIs Use this label to trigger release tests in CI shape inference Issues related to shape inference labels Sep 15, 2021
Signed-off-by: Ashwini Khade <askhade@microsoft.com>
const int ir_version_in = IR_VERSION)
const int ir_version_in = IR_VERSION,
const ModelLocalFunctionsMap& model_local_functions_in = {},
GetFunctionMapIdFn get_func_id = nullptr)
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we really need this (function to combine the domain and opname into one string) as a parameter? I understand having a single place where we can change it easily, if we want to, but unclear if we need to expose it as a parameter (that others start taking a dependence on).

Copy link
Member

Choose a reason for hiding this comment

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

+1 We should carefully review it since it's an important change that involves many functions and people will start to rely on it. It looks useful to me though. It will be more convincing if there are some real use cases for it.

check_model(model);

ShapeInferenceOptions options{true, 1, true};
ONNX_NAMESPACE::shape_inference::InferShapes(model, OpSchemaRegistry::Instance(), options);
Copy link
Contributor

Choose a reason for hiding this comment

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

Just noting for future extension (not in this PR): Ideally, we should be saving the results of shape-inference applied to functions, and right now we don't have a place to save this. (That is, the analogue of "value_info" field in a graph.)

@gramalingam
Copy link
Contributor

Wondering about the status of this PR. I guess not much change is required to merge this PR, is there? Thanks!

onnx/shape_inference/implementation.h Outdated Show resolved Hide resolved
onnx/shape_inference/implementation.h Outdated Show resolved Hide resolved
const int ir_version_in = IR_VERSION)
const int ir_version_in = IR_VERSION,
const ModelLocalFunctionsMap& model_local_functions_in = {},
GetFunctionMapIdFn get_func_id = nullptr)
Copy link
Member

Choose a reason for hiding this comment

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

+1 We should carefully review it since it's an important change that involves many functions and people will start to rely on it. It looks useful to me though. It will be more convincing if there are some real use cases for it.

@askhade
Copy link
Contributor Author

askhade commented Dec 8, 2021

Wondering about the status of this PR. I guess not much change is required to merge this PR, is there? Thanks!

No there isn't much work remaining. I will pick it up early next week and have it ready. Thanks!

Signed-off-by: Ashwini Khade <askhade@microsoft.com>
Signed-off-by: Ashwini Khade <askhade@microsoft.com>
@askhade
Copy link
Contributor Author

askhade commented Jan 4, 2022

Linux CI failures are due to an unrelated issue. #3916 should fix it.

@askhade
Copy link
Contributor Author

askhade commented Jan 5, 2022

"LinuxRelease_i686 / build (3.8, x64) (pull_request) " pipeline is failing due to an unrelated reason. #3918 should fix it. Merging this PR now.

@askhade askhade merged commit b3f5854 into onnx:master Jan 5, 2022
@askhade askhade added this to the 1.11 milestone Jan 5, 2022
liqunfu pushed a commit to liqunfu/onnx that referenced this pull request Jan 19, 2022
* add shape inference for model local functions

Signed-off-by: Ashwini Khade <askhade@microsoft.com>

* Plus tests and more changes

Signed-off-by: Ashwini Khade <askhade@microsoft.com>

* fix typo

Signed-off-by: Ashwini Khade <askhade@microsoft.com>

* Plus updates

Signed-off-by: Ashwini Khade <askhade@microsoft.com>

* Plus updates per review

Signed-off-by: Ashwini Khade <askhade@microsoft.com>

* plus updates

Signed-off-by: Ashwini Khade <askhade@microsoft.com>
Signed-off-by: Liqun Fu <liqfu@microsoft.com>
askhade added a commit that referenced this pull request Jan 19, 2022
#3902)

* Fix typos (#3894)

Signed-off-by: Lewis Tunstall <lewis.c.tunstall@gmail.com>
Signed-off-by: Liqun Fu <liqfu@microsoft.com>

* Honor existing dim_param in shape inference (#3896)

* Honor existing dim_param

Before this change, existing dim_param was overwritten by generated
symbols like `unk__`. `NonZero` is used to test the behavior.

Signed-off-by: Shinichiro Hamaji <shinichiro.hamaji@gmail.com>

* Fix test expectation of symbolic_shape_test.py

Signed-off-by: Shinichiro Hamaji <shinichiro.hamaji@gmail.com>

* Keep the original check as suggested in review

Signed-off-by: Shinichiro Hamaji <shinichiro.hamaji@gmail.com>

* Simplify the logic of mergeShapesAndTypes

Signed-off-by: Shinichiro Hamaji <shinichiro.hamaji@gmail.com>

Co-authored-by: Ashwini Khade <askhade@microsoft.com>
Signed-off-by: Liqun Fu <liqfu@microsoft.com>

* for issue 3849 to confirm that type check is performed during checker.check_model call

Signed-off-by: Liqun Fu <liqfu@microsoft.com>

* fix path to InferenceError

Signed-off-by: Liqun Fu <liqfu@microsoft.com>

* replace whitelist by safelist (#3900)

Signed-off-by: Alexandre Eichenberger <alexe@us.ibm.com>
Signed-off-by: Liqun Fu <liqfu@microsoft.com>

* add checker test case for input type mismatch conflict

Signed-off-by: Liqun Fu <liqfu@microsoft.com>

* remove old numpy in ipynb (#3916)

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>
Signed-off-by: Liqun Fu <liqfu@microsoft.com>

* Shapeinf for functions (#3722)

* add shape inference for model local functions

Signed-off-by: Ashwini Khade <askhade@microsoft.com>

* Plus tests and more changes

Signed-off-by: Ashwini Khade <askhade@microsoft.com>

* fix typo

Signed-off-by: Ashwini Khade <askhade@microsoft.com>

* Plus updates

Signed-off-by: Ashwini Khade <askhade@microsoft.com>

* Plus updates per review

Signed-off-by: Ashwini Khade <askhade@microsoft.com>

* plus updates

Signed-off-by: Ashwini Khade <askhade@microsoft.com>
Signed-off-by: Liqun Fu <liqfu@microsoft.com>

* Fix old ConvTranspose shape inference and softmax upgrader (#3893)

* Fixed ConvTranspose-1 shape inference

Brings change from #3188 (for ConvTranspose-11) into old shape inference
Signed-off-by: Matteo Salvarezza <matteo.salvarezza@gmail.com>

* Fix softmax adapter

Signed-off-by: Matteo Salvarezza <matteo.salvarezza@gmail.com>

Co-authored-by: G. Ramalingam <grama@microsoft.com>
Signed-off-by: Liqun Fu <liqfu@microsoft.com>

* Fix Linux i686 Release CI failure due to the latest NumPy (#3918)

* 3.8 and 3.9 use numpy 1.21.5

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>

* 1.21.4

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>

* 6be8011a073feeca08d256ac64335a19fc8eee4c6312668b6781ace71db0de20

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>

* 2021-12-19-a4d7f5a

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>

* 1.21.5

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>

* 1.16.6

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>

* 1.16.6

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>

* do not check generated

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>

* remove unrelated numpy

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>

* if

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>

* remove generation

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>

* name

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>

* add back

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>

* for testing

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>

* no 1.16.6

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>
Signed-off-by: Liqun Fu <liqfu@microsoft.com>

* Remind release manager to remove old onnx-weekly packages after release (#3923)

* remind release manager to remove old onnx-weekly packages

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>

* add steps

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>
Signed-off-by: Liqun Fu <liqfu@microsoft.com>

* Simplify function definition of context-dependent functions (#3882)

* Simplify function definition of context-dependent functions

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

* Add missing parenthesis

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

* Fix errors in function defs

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

* Eliminate unused variable

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

* Add int64 type specifier to literal

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

Co-authored-by: Ashwini Khade <askhade@microsoft.com>
Signed-off-by: Liqun Fu <liqfu@microsoft.com>

* Migration to using main branch (#3925)

* add warning announce

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>

* Rename to main branch globally

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>
Signed-off-by: Liqun Fu <liqfu@microsoft.com>

* Fix the bug of shape in docs (#3927)

* fix the bug of shape

Signed-off-by: namasikanam <namasikanam@gmail.com>

* fix the bug of shape

Signed-off-by: namasikanam <namasikanam@gmail.com>

Co-authored-by: Ashwini Khade <askhade@microsoft.com>
Signed-off-by: Liqun Fu <liqfu@microsoft.com>

* Append dim even both dim value and param are not set (#3828)

* Append dim even both dim value and param are not set

Signed-off-by: Joe <joe@preferred.jp>

* Copy dim

Signed-off-by: Joe <joe@preferred.jp>

* Revert "Copy dim"

This reverts commit 6623505.

Signed-off-by: Joe <joe@preferred.jp>

* Simplify code

Signed-off-by: Joe <joe@preferred.jp>

Co-authored-by: Ashwini Khade <askhade@microsoft.com>
Signed-off-by: Liqun Fu <liqfu@microsoft.com>

* bump to 10.15 in azp (#3941)

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>
Signed-off-by: Liqun Fu <liqfu@microsoft.com>

* Add explanation to run gtest (#3933)

Signed-off-by: Joe <joe@preferred.jp>

Co-authored-by: Ashwini Khade <askhade@microsoft.com>
Signed-off-by: Liqun Fu <liqfu@microsoft.com>

* Update TreeEnsembleClassifier and TreeEnsembleRegressor to support tensor as attributes (#3897)

* update TreeEnsembleClassifier and TreeEnsembleRegressor

Signed-off-by: xavier dupré <xavier.dupre@gmail.com>

* change the type of another attribute

Signed-off-by: xavier dupré <xavier.dupre@gmail.com>

* add missing file

Signed-off-by: xavier dupré <xavier.dupre@gmail.com>

* update documentation

Signed-off-by: xavier dupré <xavier.dupre@gmail.com>

* eol

Signed-off-by: xavier dupré <xavier.dupre@gmail.com>

* add field with _as_tensor

Signed-off-by: xavier dupré <xavier.dupre@gmail.com>

* fix error messages

Signed-off-by: xavier dupré <xavier.dupre@gmail.com>

* fix error message

Signed-off-by: xavier dupré <xavier.dupre@gmail.com>

* fix missing change

Signed-off-by: xavier dupré <xavier.dupre@gmail.com>

Co-authored-by: xavier dupré <xavier.dupre@gmail.com>
Co-authored-by: G. Ramalingam <grama@microsoft.com>
Signed-off-by: Liqun Fu <liqfu@microsoft.com>

Co-authored-by: lewtun <lewis.c.tunstall@gmail.com>
Co-authored-by: Shinichiro Hamaji <shinichiro.hamaji@gmail.com>
Co-authored-by: Ashwini Khade <askhade@microsoft.com>
Co-authored-by: Alexandre Eichenberger <alexe@us.ibm.com>
Co-authored-by: Chun-Wei Chen <jacky82226@gmail.com>
Co-authored-by: Matteo Salvarezza <matteo.salvarezza@gmail.com>
Co-authored-by: G. Ramalingam <grama@microsoft.com>
Co-authored-by: Xingyu Xie <namasikanam@gmail.com>
Co-authored-by: Joe <joe@preferred.jp>
Co-authored-by: Xavier Dupré <xadupre@users.noreply.github.com>
Co-authored-by: xavier dupré <xavier.dupre@gmail.com>
liqunfu added a commit to liqunfu/onnx that referenced this pull request Jan 24, 2022
onnx#3902)

* Fix typos (onnx#3894)

Signed-off-by: Lewis Tunstall <lewis.c.tunstall@gmail.com>
Signed-off-by: Liqun Fu <liqfu@microsoft.com>

* Honor existing dim_param in shape inference (onnx#3896)

* Honor existing dim_param

Before this change, existing dim_param was overwritten by generated
symbols like `unk__`. `NonZero` is used to test the behavior.

Signed-off-by: Shinichiro Hamaji <shinichiro.hamaji@gmail.com>

* Fix test expectation of symbolic_shape_test.py

Signed-off-by: Shinichiro Hamaji <shinichiro.hamaji@gmail.com>

* Keep the original check as suggested in review

Signed-off-by: Shinichiro Hamaji <shinichiro.hamaji@gmail.com>

* Simplify the logic of mergeShapesAndTypes

Signed-off-by: Shinichiro Hamaji <shinichiro.hamaji@gmail.com>

Co-authored-by: Ashwini Khade <askhade@microsoft.com>
Signed-off-by: Liqun Fu <liqfu@microsoft.com>

* for issue 3849 to confirm that type check is performed during checker.check_model call

Signed-off-by: Liqun Fu <liqfu@microsoft.com>

* fix path to InferenceError

Signed-off-by: Liqun Fu <liqfu@microsoft.com>

* replace whitelist by safelist (onnx#3900)

Signed-off-by: Alexandre Eichenberger <alexe@us.ibm.com>
Signed-off-by: Liqun Fu <liqfu@microsoft.com>

* add checker test case for input type mismatch conflict

Signed-off-by: Liqun Fu <liqfu@microsoft.com>

* remove old numpy in ipynb (onnx#3916)

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>
Signed-off-by: Liqun Fu <liqfu@microsoft.com>

* Shapeinf for functions (onnx#3722)

* add shape inference for model local functions

Signed-off-by: Ashwini Khade <askhade@microsoft.com>

* Plus tests and more changes

Signed-off-by: Ashwini Khade <askhade@microsoft.com>

* fix typo

Signed-off-by: Ashwini Khade <askhade@microsoft.com>

* Plus updates

Signed-off-by: Ashwini Khade <askhade@microsoft.com>

* Plus updates per review

Signed-off-by: Ashwini Khade <askhade@microsoft.com>

* plus updates

Signed-off-by: Ashwini Khade <askhade@microsoft.com>
Signed-off-by: Liqun Fu <liqfu@microsoft.com>

* Fix old ConvTranspose shape inference and softmax upgrader (onnx#3893)

* Fixed ConvTranspose-1 shape inference

Brings change from onnx#3188 (for ConvTranspose-11) into old shape inference
Signed-off-by: Matteo Salvarezza <matteo.salvarezza@gmail.com>

* Fix softmax adapter

Signed-off-by: Matteo Salvarezza <matteo.salvarezza@gmail.com>

Co-authored-by: G. Ramalingam <grama@microsoft.com>
Signed-off-by: Liqun Fu <liqfu@microsoft.com>

* Fix Linux i686 Release CI failure due to the latest NumPy (onnx#3918)

* 3.8 and 3.9 use numpy 1.21.5

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>

* 1.21.4

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>

* 6be8011a073feeca08d256ac64335a19fc8eee4c6312668b6781ace71db0de20

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>

* 2021-12-19-a4d7f5a

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>

* 1.21.5

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>

* 1.16.6

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>

* 1.16.6

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>

* do not check generated

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>

* remove unrelated numpy

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>

* if

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>

* remove generation

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>

* name

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>

* add back

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>

* for testing

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>

* no 1.16.6

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>
Signed-off-by: Liqun Fu <liqfu@microsoft.com>

* Remind release manager to remove old onnx-weekly packages after release (onnx#3923)

* remind release manager to remove old onnx-weekly packages

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>

* add steps

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>
Signed-off-by: Liqun Fu <liqfu@microsoft.com>

* Simplify function definition of context-dependent functions (onnx#3882)

* Simplify function definition of context-dependent functions

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

* Add missing parenthesis

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

* Fix errors in function defs

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

* Eliminate unused variable

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

* Add int64 type specifier to literal

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

Co-authored-by: Ashwini Khade <askhade@microsoft.com>
Signed-off-by: Liqun Fu <liqfu@microsoft.com>

* Migration to using main branch (onnx#3925)

* add warning announce

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>

* Rename to main branch globally

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>
Signed-off-by: Liqun Fu <liqfu@microsoft.com>

* Fix the bug of shape in docs (onnx#3927)

* fix the bug of shape

Signed-off-by: namasikanam <namasikanam@gmail.com>

* fix the bug of shape

Signed-off-by: namasikanam <namasikanam@gmail.com>

Co-authored-by: Ashwini Khade <askhade@microsoft.com>
Signed-off-by: Liqun Fu <liqfu@microsoft.com>

* Append dim even both dim value and param are not set (onnx#3828)

* Append dim even both dim value and param are not set

Signed-off-by: Joe <joe@preferred.jp>

* Copy dim

Signed-off-by: Joe <joe@preferred.jp>

* Revert "Copy dim"

This reverts commit 6623505.

Signed-off-by: Joe <joe@preferred.jp>

* Simplify code

Signed-off-by: Joe <joe@preferred.jp>

Co-authored-by: Ashwini Khade <askhade@microsoft.com>
Signed-off-by: Liqun Fu <liqfu@microsoft.com>

* bump to 10.15 in azp (onnx#3941)

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>
Signed-off-by: Liqun Fu <liqfu@microsoft.com>

* Add explanation to run gtest (onnx#3933)

Signed-off-by: Joe <joe@preferred.jp>

Co-authored-by: Ashwini Khade <askhade@microsoft.com>
Signed-off-by: Liqun Fu <liqfu@microsoft.com>

* Update TreeEnsembleClassifier and TreeEnsembleRegressor to support tensor as attributes (onnx#3897)

* update TreeEnsembleClassifier and TreeEnsembleRegressor

Signed-off-by: xavier dupré <xavier.dupre@gmail.com>

* change the type of another attribute

Signed-off-by: xavier dupré <xavier.dupre@gmail.com>

* add missing file

Signed-off-by: xavier dupré <xavier.dupre@gmail.com>

* update documentation

Signed-off-by: xavier dupré <xavier.dupre@gmail.com>

* eol

Signed-off-by: xavier dupré <xavier.dupre@gmail.com>

* add field with _as_tensor

Signed-off-by: xavier dupré <xavier.dupre@gmail.com>

* fix error messages

Signed-off-by: xavier dupré <xavier.dupre@gmail.com>

* fix error message

Signed-off-by: xavier dupré <xavier.dupre@gmail.com>

* fix missing change

Signed-off-by: xavier dupré <xavier.dupre@gmail.com>

Co-authored-by: xavier dupré <xavier.dupre@gmail.com>
Co-authored-by: G. Ramalingam <grama@microsoft.com>
Signed-off-by: Liqun Fu <liqfu@microsoft.com>

Co-authored-by: lewtun <lewis.c.tunstall@gmail.com>
Co-authored-by: Shinichiro Hamaji <shinichiro.hamaji@gmail.com>
Co-authored-by: Ashwini Khade <askhade@microsoft.com>
Co-authored-by: Alexandre Eichenberger <alexe@us.ibm.com>
Co-authored-by: Chun-Wei Chen <jacky82226@gmail.com>
Co-authored-by: Matteo Salvarezza <matteo.salvarezza@gmail.com>
Co-authored-by: G. Ramalingam <grama@microsoft.com>
Co-authored-by: Xingyu Xie <namasikanam@gmail.com>
Co-authored-by: Joe <joe@preferred.jp>
Co-authored-by: Xavier Dupré <xadupre@users.noreply.github.com>
Co-authored-by: xavier dupré <xavier.dupre@gmail.com>
Signed-off-by: Liqun Fu <liqfu@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
run release CIs Use this label to trigger release tests in CI shape inference Issues related to shape inference
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants