-
Notifications
You must be signed in to change notification settings - Fork 14
Add methods to resize inputs and outputs in Node class #244
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
Conversation
Signed-off-by: Justin Chu <[email protected]>
Signed-off-by: Justin Chu <[email protected]>
Signed-off-by: Justin Chu <[email protected]>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #244 +/- ##
==========================================
+ Coverage 76.86% 76.99% +0.13%
==========================================
Files 40 40
Lines 4997 5034 +37
Branches 996 1008 +12
==========================================
+ Hits 3841 3876 +35
- Misses 867 868 +1
- Partials 289 290 +1 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds resize_inputs and resize_outputs methods to the Node class in the ONNX IR, providing a way to dynamically adjust the number of inputs and outputs of a node.
- Adds
resize_inputs()method to adjust node input count dynamically - Adds
resize_outputs()method to adjust node output count dynamically - Updates error messages for property setters to reference the new methods
- Comprehensive test coverage for all edge cases
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/onnx_ir/_core.py | Implements resize_inputs and resize_outputs methods and updates setter error messages |
| src/onnx_ir/_core_test.py | Adds comprehensive test coverage for the new resizing methods |
Signed-off-by: Justin Chu <[email protected]>
Co-authored-by: Copilot <[email protected]> Signed-off-by: Justin Chu <[email protected]>
Co-authored-by: Copilot <[email protected]> Signed-off-by: Justin Chu <[email protected]>
|
cc @gramalingam |
Signed-off-by: Justin Chu <[email protected]>
This pull request introduces new methods to dynamically resize the inputs and outputs of a
Nodein the ONNX IR core, making graph manipulation more flexible and robust. It updates the API to support these operations and adds comprehensive tests to ensure correct behavior, including edge cases and error handling.API Enhancements for Node Inputs and Outputs
resize_inputsmethod toNode, allowing the number of inputs to be increased (by addingNonevalues) or decreased (removing extra inputs and cleaning up their uses). The setter error message forinputswas also updated to reference the new method.resize_outputsmethod toNode, enabling dynamic resizing of outputs. Outputs can be increased (newValueobjects created) or decreased, but removal is only allowed if the outputs have no uses; otherwise, aValueErroris raised. The setter error message foroutputswas updated accordingly.Comprehensive Testing
resize_inputs, covering increasing, decreasing, unchanged size, zeroing, growing from zero, and preservation ofNoneinputs.resize_outputs, including increasing, decreasing, unchanged size, zeroing, growing from zero, and error handling when attempting to remove outputs that are still in use.Why not create
popandaddto the inputs/outputs objectCurrently inputs and outputs are implemented as
tuple, which means there are a lot of methods associated to it that we get for free. Since there is not ausertupleclass in python, defining an immutable sequence that supports all of the tuple methods may be complex (have to implement the full Sequence interface). And since the current interface says Node.inputs, Node.outputs return Sequence interfaces, having selected mutable methods on them (pop, add) makes it confusing.