Utility to inline model-local functions #5105

gramalingam · 2023-04-09T04:27:43Z

Description

An initial version of utility to inline all calls to model-local functions.

Extensions to be handled in future:

Ideally, we should allow users to specify filters to indicate which functions to inline and/or which not to inline.
Doesn't yet handle opschema-defined functions.

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

jbachurski · 2023-04-09T13:44:49Z

I would propose to implement a utility to inline a given list of FunctionProtos, which is more general and already indirectly implemented here. It'll have more uses and finding all model local functions is trivial.

gramalingam · 2023-04-11T16:36:23Z

I would propose to implement a utility to inline a given list of FunctionProtos, which is more general and already indirectly implemented here. It'll have more uses and finding all model local functions is trivial.

I think that is equivalent? To be more specific, the implementation provides a core (namely inline_functions) that is generic and intended to enable other scenarios as well, and one specific scenario (namely inline_local_functions). But only the latter is currently exposed as an external API in the include file. The reason is that I think the core API can be made even more general, and so it may be good to iterate through other scenarios as well before finalizing the signature of this core API.

For example, I think it would be useful to allow the callers to specify which functions to inline or NOT inline (via the names of functions).

Anyway: I agree that other use-cases will arise and we should support them. This is just the first step.

jbachurski · 2023-04-11T16:40:27Z

Thanks for responding! I think we are on the same page then. I noticed it ends up implemented in C++ but I had only looked for a Python test/bindings.
In any case, it would be nice to have an ergonomic API for this graph transformation.

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

onnx/inliner/inliner.cc

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

onnx/inliner/inliner.cc

onnx/test/cpp/inliner_test.cc

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

justinchuby · 2023-05-04T03:50:08Z

I tested the inliner with this model, but it seemed to have returned the same model:

gpt2-dataprop.zip

gramalingam · 2023-05-04T18:27:22Z

I tested the inliner with this model, but it seemed to have returned the same model:

gpt2-dataprop.zip

I tried it. Looks like the model uses onnx opset 17, while the functions use opset 18. Currently, inliner does not inline functions when there is an opset mismatch. Changed model opset to 18, and I think it did something: looks like it inlined 24 of 26 functions, the number of nodes in main graph went up from 383 to 595. Didn't check anything more.

gramalingam · 2023-05-04T18:30:59Z

I tested the inliner with this model, but it seemed to have returned the same model:
gpt2-dataprop.zip

I tried it. Looks like the model uses onnx opset 17, while the functions use opset 18. Currently, inliner does not inline functions when there is an opset mismatch. Changed model opset to 18, and I think it did something: looks like it inlined 24 of 26 functions, the number of nodes in main graph went up from 383 to 595. Didn't check anything more.

Integrating the version-converter to automatically convert functions to model's opset is work in progress. (Even that may not always succeed for for version down-conversion; version up-conversion should be more complete.)

justinchuby · 2023-05-04T18:46:35Z

I tried it. Looks like the model uses onnx opset 17

Ah that's right. Didn't realize that. Thanks!

* Add function inliner part 1 Signed-off-by: Ganesan Ramalingam <grama@microsoft.com> * Inliner for model-local functions Signed-off-by: Ganesan Ramalingam <grama@microsoft.com> * Minor fixes Signed-off-by: Ganesan Ramalingam <grama@microsoft.com> * Export inliner to python Signed-off-by: Ganesan Ramalingam <grama@microsoft.com> * Fix emptied files Signed-off-by: Ganesan Ramalingam <grama@microsoft.com> * remove constexpr Signed-off-by: Ganesan Ramalingam <grama@microsoft.com> * Replace move by copy Signed-off-by: Ganesan Ramalingam <grama@microsoft.com> * Run formatter Signed-off-by: Ganesan Ramalingam <grama@microsoft.com> * Format file Signed-off-by: Ganesan Ramalingam <grama@microsoft.com> * Check names are unique Signed-off-by: Ganesan Ramalingam <grama@microsoft.com> * naming conventions Signed-off-by: Ganesan Ramalingam <grama@microsoft.com> * Test for opset mistmatch Signed-off-by: Ganesan Ramalingam <grama@microsoft.com> * Remove move Signed-off-by: Ganesan Ramalingam <grama@microsoft.com> * Address PR feedback Signed-off-by: Ganesan Ramalingam <grama@microsoft.com> * Add testcase with two calls to same function Signed-off-by: Ganesan Ramalingam <grama@microsoft.com> * Comment debugging code Signed-off-by: Ganesan Ramalingam <grama@microsoft.com> * Add comments about renaming Signed-off-by: Ganesan Ramalingam <grama@microsoft.com> * Run formatter Signed-off-by: Ganesan Ramalingam <grama@microsoft.com> --------- Signed-off-by: Ganesan Ramalingam <grama@microsoft.com> Signed-off-by: Aditya Goel <agoel4512@gmail.com>

gramalingam added 5 commits April 7, 2023 10:48

Add function inliner part 1

8384923

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

Inliner for model-local functions

19299dd

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

Minor fixes

da37f8f

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

Export inliner to python

0172ef7

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

Fix emptied files

47f963d

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

gramalingam requested a review from a team as a code owner April 9, 2023 04:27

gramalingam added 7 commits April 11, 2023 10:50

remove constexpr

d35f589

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

Replace move by copy

660520c

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

Run formatter

4a4baed

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

Format file

3447a26

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

Check names are unique

4f82c17

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

naming conventions

402083f

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

Test for opset mistmatch

bbd10ff

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

gramalingam changed the title ~~Utility to inline model-local functions (Draft)~~ Utility to inline model-local functions Apr 13, 2023

Remove move

b9d1e30

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

justinchuby reviewed Apr 15, 2023

View reviewed changes

onnx/inliner/inliner.cc Show resolved Hide resolved

jcwchen added the module: utility Helper modules label Apr 20, 2023

JakubBachurskiQC mentioned this pull request Apr 21, 2023

Reinforce the implementation of inline and fix adapting bug Quantco/spox#75

Merged

gramalingam added 2 commits April 25, 2023 13:52

Address PR feedback

8b2e282

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

Merge branch 'main' into inliner

cf6369a

xadupre reviewed Apr 28, 2023

View reviewed changes

onnx/inliner/inliner.cc Show resolved Hide resolved

xadupre reviewed Apr 28, 2023

View reviewed changes

onnx/test/cpp/inliner_test.cc Outdated Show resolved Hide resolved

xadupre reviewed Apr 28, 2023

View reviewed changes

onnx/test/cpp/inliner_test.cc Show resolved Hide resolved

gramalingam added 3 commits April 28, 2023 10:22

Add testcase with two calls to same function

5d733cc

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

Comment debugging code

cb2c768

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

Add comments about renaming

879daca

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

xadupre approved these changes Apr 28, 2023

View reviewed changes

gramalingam added 2 commits April 28, 2023 11:08

Run formatter

5af837c

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

Merge branch 'main' into inliner

5c70d25

gramalingam enabled auto-merge (squash) April 28, 2023 18:09

Merge branch 'main' into inliner

a991350

gramalingam merged commit 65e8f81 into onnx:main May 4, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Utility to inline model-local functions #5105

Utility to inline model-local functions #5105

Uh oh!

gramalingam commented Apr 9, 2023 •

edited

Loading

Uh oh!

jbachurski commented Apr 9, 2023 •

edited

Loading

Uh oh!

gramalingam commented Apr 11, 2023

Uh oh!

jbachurski commented Apr 11, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

justinchuby commented May 4, 2023 •

edited

Loading

Uh oh!

gramalingam commented May 4, 2023

Uh oh!

gramalingam commented May 4, 2023

Uh oh!

justinchuby commented May 4, 2023

Uh oh!

Uh oh!

Utility to inline model-local functions #5105

Utility to inline model-local functions #5105

Uh oh!

Conversation

gramalingam commented Apr 9, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Uh oh!

jbachurski commented Apr 9, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gramalingam commented Apr 11, 2023

Uh oh!

jbachurski commented Apr 11, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

justinchuby commented May 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gramalingam commented May 4, 2023

Uh oh!

gramalingam commented May 4, 2023

Uh oh!

justinchuby commented May 4, 2023

Uh oh!

Uh oh!

gramalingam commented Apr 9, 2023 •

edited

Loading

jbachurski commented Apr 9, 2023 •

edited

Loading

justinchuby commented May 4, 2023 •

edited

Loading