Allow GraphBuilder to call script functions#2820
Conversation
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>
There was a problem hiding this comment.
lintrunner found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2820 +/- ##
==========================================
+ Coverage 71.44% 71.48% +0.03%
==========================================
Files 236 237 +1
Lines 28496 28645 +149
Branches 2819 2841 +22
==========================================
+ Hits 20360 20476 +116
- Misses 7171 7199 +28
- Partials 965 970 +5 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Pull request overview
This pull request enables GraphBuilder/OpBuilder to call @script-decorated ONNXScript functions, allowing seamless composition of imperative (builder) and declarative (script) code. The PR also updates the default output naming strategy to include node numbers for better uniqueness.
Changes:
- Added
call()method to GraphBuilder and OpBuilder that inlines script functions into the builder's graph - Modified converter to accept OpBuilder as a default_opset parameter, converting it to Opset internally
- Updated default output naming from
{op_type}_outputto{op_type}_n{count}_outputfor uniqueness - Created new
_inliner.pymodule with function instantiation logic
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| onnxscript/_internal/builder.py | Added call() methods and domain/version properties to OpBuilder; updated naming strategy |
| onnxscript/_internal/converter.py | Added support for OpBuilder as default_opset with automatic conversion to Opset |
| onnxscript/_internal/_inliner.py | New module providing function instantiation/inlining capabilities |
| onnxscript/_internal/builder_test.py | Comprehensive tests for call() functionality with various options (_outputs, _prefix) |
| docs/tutorial/builder/graph_builder.md | Documentation for calling script functions from OpBuilder |
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
Co-authored-by: Justin Chu <justinchuby@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.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>
TODO later: Consider integration of standard execution of script functions with execution of OpBuilder calls, as well as other extensions within script-mode to ensure both modes are seamless and uniform