Skip to content

Integrate function-inlining with version-conversion #5211

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

Merged
merged 34 commits into from
May 16, 2023

Conversation

gramalingam
Copy link
Contributor

This extends the function-inliner to take advantage of the version-converter to inline functions targetting ONNX opset X into a model targetting ONNX opset Y (when conversion of the function from version X to version Y is successful).

The two are coupled because the existing version-converter does not support functions. It is not easy to extend the version-converter to handle functions because some of the version-conversion rules depend on context-information (such as the types of inputs or values of attributes) that may not be available in a function body. However, in the context of inlining a function into a callsite in a model, the necessary context information can be obtained, enabling the conversion to proceed.

The functionality is complete. But there are a couple of pending improvements, which can be delayed to a later PR

(i) It may be useful to expose an option to the caller to indicate if we should attempt version-conversion or not.

(ii) Version-conversion requires us to update type information for the model after one or more inlining steps, in order to apply version-conversion to nested function-calls. The current implementation uses a somewhat naive strategy to do this. We can improve this by using an incremental type-inference implementation.

Signed-off-by: Ganesan Ramalingam <grama@microsoft.com>
Signed-off-by: Ganesan Ramalingam <grama@microsoft.com>
Signed-off-by: Ganesan Ramalingam <grama@microsoft.com>
Signed-off-by: Ganesan Ramalingam <grama@microsoft.com>
Signed-off-by: Ganesan Ramalingam <grama@microsoft.com>
Signed-off-by: Ganesan Ramalingam <grama@microsoft.com>
Signed-off-by: Ganesan Ramalingam <grama@microsoft.com>
Signed-off-by: Ganesan Ramalingam <grama@microsoft.com>
Signed-off-by: Ganesan Ramalingam <grama@microsoft.com>
Signed-off-by: Ganesan Ramalingam <grama@microsoft.com>
Signed-off-by: Ganesan Ramalingam <grama@microsoft.com>
Signed-off-by: Ganesan Ramalingam <grama@microsoft.com>
Signed-off-by: Ganesan Ramalingam <grama@microsoft.com>
Signed-off-by: Ganesan Ramalingam <grama@microsoft.com>
Signed-off-by: Ganesan Ramalingam <grama@microsoft.com>
Signed-off-by: Ganesan Ramalingam <grama@microsoft.com>
Signed-off-by: Ganesan Ramalingam <grama@microsoft.com>
Signed-off-by: Ganesan Ramalingam <grama@microsoft.com>
Signed-off-by: Ganesan Ramalingam <grama@microsoft.com>
@gramalingam gramalingam requested a review from a team as a code owner May 8, 2023 21:34
@xadupre
Copy link
Contributor

xadupre commented May 9, 2023

Would it be possible to add a section in the python documentation as well about the inliner (https://github.com/onnx/onnx/tree/main/docs/docsgen/source/api)?

gramalingam added 10 commits May 9, 2023 10:46
Signed-off-by: Ganesan Ramalingam <grama@microsoft.com>
Signed-off-by: Ganesan Ramalingam <grama@microsoft.com>
Signed-off-by: Ganesan Ramalingam <grama@microsoft.com>
Signed-off-by: Ganesan Ramalingam <grama@microsoft.com>
Signed-off-by: Ganesan Ramalingam <grama@microsoft.com>
Signed-off-by: Ganesan Ramalingam <grama@microsoft.com>
Signed-off-by: Ganesan Ramalingam <grama@microsoft.com>
@gramalingam
Copy link
Contributor Author

Would it be possible to add a section in the python documentation as well about the inliner (https://github.com/onnx/onnx/tree/main/docs/docsgen/source/api)?

Done

@gramalingam gramalingam enabled auto-merge (squash) May 16, 2023 16:37
@gramalingam gramalingam merged commit 1d17a40 into onnx:main May 16, 2023
q-ycong-p pushed a commit to q-ycong-p/onnx that referenced this pull request May 18, 2023
Signed-off-by: Ganesan Ramalingam <grama@microsoft.com>
Signed-off-by: Yu Cong <congyc@amazon.com>
adityagoel4512 pushed a commit to adityagoel4512/onnx that referenced this pull request Jul 28, 2023
Signed-off-by: Ganesan Ramalingam <grama@microsoft.com>
Signed-off-by: Aditya Goel <agoel4512@gmail.com>
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.

3 participants