-
Notifications
You must be signed in to change notification settings - Fork 57
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
[IR] Create predecessors()
and successors()
on ir.Node
#2022
base: main
Are you sure you want to change the base?
Conversation
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.
Copilot reviewed 1 out of 1 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (5)
onnxscript/ir/_core.py:1314
- Ensure that value is not None before calling value.producer().
if value is not None and (producer := value.producer()) is not None:
onnxscript/ir/_core.py:1326
- [nitpick] Consider replacing the assert statement with a more informative error message or exception.
assert value is not None, "Bug: Output values are not expected to be None"
onnxscript/ir/_core.py:1059
- [nitpick] Consider renaming 'Usage' to 'NodeUsage' for better clarity.
class Usage(NamedTuple):
onnxscript/ir/_core.py:1309
- Ensure that the 'predecessors' method is covered by tests.
def predecessors(self) -> Sequence[Node]:
onnxscript/ir/_core.py:1321
- Ensure that the 'successors' method is covered by tests.
def successors(self) -> Sequence[Node]:
❌ 70 Tests Failed:
View the top 3 failed tests by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
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.
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (2)
onnxscript/ir/_core.py:1646
- The word 'addes' is misspelled. It should be 'adds'.
// This addes a small overhead but is better a user experience than
onnxscript/ir/_core_test.py:817
- Add a test for the
successors
method.
# TODO(justinchuby): Test all methods
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
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.
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (2)
onnxscript/ir/_core_test.py:834
- Ensure that the order of nodes in the predecessors method is deterministic and consistent.
self.assertEqual(self.node.predecessors(), ())
onnxscript/ir/_core_test.py:843
- Ensure that the order of nodes in the successors method is deterministic and consistent.
self.assertEqual(self.node.successors(), (self.node_a, self.node_b))
Usage
to a named tupleconsumers()
onValue