[IR] Reconcile graph in Node #2183

justinchuby · 2025-04-11T14:41:41Z

Always assign a Graph object to the node's graph.

Copilot

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

onnxscript/ir/passes/common/constant_manipulation.py:54

[nitpick] The type assertion for node.graph was removed, which may allow unintended types if the invariant is not otherwise enforced. Consider documenting or validating that node.graph is always a Graph as it is set in _core.py.

assert isinstance(node.graph, ir.Graph)

onnxscript/ir/_core.py

codecov · 2025-04-11T14:45:31Z

❌ 18 Tests Failed:

Tests completed	Failed	Passed	Skipped
15423	18	15405	2898

View the top 3 failed test(s) by shortest run time

onnxscript.backend.onnx_export_test.TestOnnxBackEnd::test_export2python_produces_correct_onnx_script_model_0041_test_argmax_keepdims_example_select_last_index

Stack Traces | 0.003s run time

onnxscript\backend\onnx_export_test.py:137: in extract_functions
    mod = importlib.import_module(import_name)
C:\hostedtoolcache\windows\Python\3.10.11\x64\lib\importlib\__init__.py:126: in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
E   ModuleNotFoundError: No module named 'tests.onnx_backend_test_code.test_argmax_keepdims_example_select_last_index'

The above exception was the direct cause of the following exception:
.nox\test\lib\site-packages\parameterized\parameterized.py:620: in standalone_func
    return func(*(a + p.args), **p.kwargs, **kw)
onnxscript\backend\onnx_export_test.py:271: in test_export2python_produces_correct_onnx_script_model
    functions = extract_functions(backend_test.name, code, self.test_folder)
onnxscript\backend\onnx_export_test.py:139: in extract_functions
    raise AssertionError(
E   AssertionError: Unable to import 'tests.onnx_backend_test_code.test_argmax_keepdims_example_select_last_index' (e=No module named 'tests.onnx_backend_test_code.test_argmax_keepdims_example_select_last_index') (file: 'D:\\a\\onnxscript\\onnxscript\\tests\\onnx_backend_test_code\\test_argmax_keepdims_example_select_last_index.py', absolute path: 'D:\\a\\onnxscript\\onnxscript\\tests\\onnx_backend_test_code\\test_argmax_keepdims_example_select_last_index.py', current folder: D:\a\onnxscript\onnxscript
E   ---- CONTENT --
E   import numpy
E   from onnx import TensorProto
E   from onnx.helper import make_tensor
E   from onnxscript import script, external_tensor
E   from onnxscript.values import Opset
E   from onnxscript.onnx_types import FLOAT, INT64
E   from onnxscript.onnx_opset import opset13
E   
E   @script()
E   def bck_test_argmax_keepdims_example_select_last_index(data: FLOAT[2,2]) -> (INT64[2,1]):
E       result = opset13.ArgMax(data, axis=1, keepdims=1, select_last_index=1)
E       return result

onnxscript.backend.onnx_export_test.TestOnnxBackEnd::test_export2python_produces_correct_onnx_script_model_0173_test_cast_DOUBLE_to_FLOAT

Stack Traces | 0.003s run time

onnxscript\backend\onnx_export_test.py:137: in extract_functions
    mod = importlib.import_module(import_name)
C:\hostedtoolcache\windows\Python\3.10.11\x64\lib\importlib\__init__.py:126: in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
E   ModuleNotFoundError: No module named 'tests.onnx_backend_test_code.test_cast_DOUBLE_to_FLOAT'

The above exception was the direct cause of the following exception:
.nox\test\lib\site-packages\parameterized\parameterized.py:620: in standalone_func
    return func(*(a + p.args), **p.kwargs, **kw)
onnxscript\backend\onnx_export_test.py:271: in test_export2python_produces_correct_onnx_script_model
    functions = extract_functions(backend_test.name, code, self.test_folder)
onnxscript\backend\onnx_export_test.py:139: in extract_functions
    raise AssertionError(
E   AssertionError: Unable to import 'tests.onnx_backend_test_code.test_cast_DOUBLE_to_FLOAT' (e=No module named 'tests.onnx_backend_test_code.test_cast_DOUBLE_to_FLOAT') (file: 'D:\\a\\onnxscript\\onnxscript\\tests\\onnx_backend_test_code\\test_cast_DOUBLE_to_FLOAT.py', absolute path: 'D:\\a\\onnxscript\\onnxscript\\tests\\onnx_backend_test_code\\test_cast_DOUBLE_to_FLOAT.py', current folder: D:\a\onnxscript\onnxscript
E   ---- CONTENT --
E   import numpy
E   from onnx import TensorProto
E   from onnx.helper import make_tensor
E   from onnxscript import script, external_tensor
E   from onnxscript.values import Opset
E   from onnxscript.onnx_types import DOUBLE, FLOAT
E   from onnxscript.onnx_opset import opset21
E   
E   @script()
E   def bck_test_cast_DOUBLE_to_FLOAT(input: DOUBLE[3,4]) -> (FLOAT[3,4]):
E       output = opset21.Cast(input, to=1)
E       return output

onnxscript.backend.onnx_export_test.TestOnnxBackEnd::test_export2python_produces_correct_onnx_script_model_0586_test_log_example

Stack Traces | 0.003s run time

onnxscript\backend\onnx_export_test.py:137: in extract_functions
    mod = importlib.import_module(import_name)
C:\hostedtoolcache\windows\Python\3.11.9\x64\Lib\importlib\__init__.py:126: in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
E   ModuleNotFoundError: No module named 'tests.onnx_backend_test_code.test_log_example'

The above exception was the direct cause of the following exception:
.nox\test\Lib\site-packages\parameterized\parameterized.py:620: in standalone_func
    return func(*(a + p.args), **p.kwargs, **kw)
onnxscript\backend\onnx_export_test.py:271: in test_export2python_produces_correct_onnx_script_model
    functions = extract_functions(backend_test.name, code, self.test_folder)
onnxscript\backend\onnx_export_test.py:139: in extract_functions
    raise AssertionError(
E   AssertionError: Unable to import 'tests.onnx_backend_test_code.test_log_example' (e=No module named 'tests.onnx_backend_test_code.test_log_example') (file: 'D:\\a\\onnxscript\\onnxscript\\tests\\onnx_backend_test_code\\test_log_example.py', absolute path: 'D:\\a\\onnxscript\\onnxscript\\tests\\onnx_backend_test_code\\test_log_example.py', current folder: D:\a\onnxscript\onnxscript
E   ---- CONTENT --
E   import numpy
E   from onnx import TensorProto
E   from onnx.helper import make_tensor
E   from onnxscript import script, external_tensor
E   from onnxscript.values import Opset
E   from onnxscript.onnx_types import FLOAT
E   from onnxscript.onnx_opset import opset13
E   
E   @script()
E   def bck_test_log_example(x: FLOAT[2]) -> (FLOAT[2]):
E       y = opset13.Log(x)
E       return y

To view more test analytics, go to the Test Analytics Dashboard
_{📋 Got 3 mins? Take this short survey to help us improve Test Analytics.}

onnxscript/ir/_core.py

titaiwangms · 2025-04-14T21:34:30Z

onnxscript/ir/_core.py

        self.doc_string = doc_string

        # Add the node as a use of the inputs
        for i, input_value in enumerate(self._inputs):
            if input_value is not None:
                input_value._add_usage(self, i)  # pylint: disable=protected-access

-        # Add the node to the graph if graph is specified
-        if self._graph is not None:
-            self._graph.append(self)


I don't understand how moving these up make any difference?

I moved it closer to were graph is created so it is more readable.

Implement model.graphs() as a way to retrieve the main graph and all subgraphs of it in the model. Given (1) how useful the method is (2) I couldn't find an appropriate name for it in `traversal.py` (3) Users familiar with onnxruntime optimization tools expect this method. In PyTorch a similar `modules()` method exists. I created this method as a core method instead of an iterator in `traversal.py`. Depends on #2183

[IR] Reconcile graph in Node

180c314

github-project-automation bot moved this to Todo in ONNX Script Review Board Apr 11, 2025

github-project-automation bot added this to ONNX Script Review Board Apr 11, 2025

justinchuby mentioned this pull request Apr 11, 2025

[Pass] Support lift constants to initializers pass #2160

Merged

justinchuby requested review from titaiwangms and Copilot and removed request for titaiwangms April 11, 2025 14:42

justinchuby added the module: IR Intermediate representation label Apr 11, 2025

Copilot AI reviewed Apr 11, 2025

View reviewed changes

onnxscript/ir/_core.py Show resolved Hide resolved

xadupre reviewed Apr 11, 2025

View reviewed changes

onnxscript/ir/_core.py Show resolved Hide resolved

gramalingam reviewed Apr 11, 2025

View reviewed changes

onnxscript/ir/_core.py Show resolved Hide resolved

Merge branch 'main' into justinchu/node-graph

24b7ecd

justinchuby requested a review from gramalingam April 13, 2025 00:56

justinchuby enabled auto-merge (squash) April 14, 2025 16:55

justinchuby mentioned this pull request Apr 14, 2025

[IR] Implement model.graphs() #2200

Merged

Merge branch 'main' into justinchu/node-graph

5453385

titaiwangms reviewed Apr 14, 2025

View reviewed changes

titaiwangms approved these changes Apr 14, 2025

View reviewed changes

justinchuby merged commit ef8e889 into main Apr 14, 2025
20 of 27 checks passed

justinchuby deleted the justinchu/node-graph branch April 14, 2025 22:17

github-project-automation bot moved this from Todo to Done in ONNX Script Review Board Apr 14, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[IR] Reconcile graph in Node #2183

[IR] Reconcile graph in Node #2183

justinchuby commented Apr 11, 2025 •

edited

Loading

Copilot AI left a comment

codecov bot commented Apr 11, 2025 •

edited

Loading

titaiwangms Apr 14, 2025

justinchuby Apr 14, 2025

[IR] Reconcile graph in Node #2183

[IR] Reconcile graph in Node #2183

Conversation

justinchuby commented Apr 11, 2025 • edited Loading

Copilot AI left a comment

Choose a reason for hiding this comment

codecov bot commented Apr 11, 2025 • edited Loading

❌ 18 Tests Failed:

titaiwangms Apr 14, 2025

Choose a reason for hiding this comment

justinchuby Apr 14, 2025

Choose a reason for hiding this comment

justinchuby commented Apr 11, 2025 •

edited

Loading

codecov bot commented Apr 11, 2025 •

edited

Loading