Skip to content

Conversation

@justinchuby
Copy link
Collaborator

@justinchuby justinchuby commented Apr 8, 2025

This pull request includes a new method to perform a topological sort on graphs and functions in the onnxscript/ir/_core.py file. The most important change is the addition of the sort method, which ensures that all graphs and functions are sorted in a stable manner.

Enhancements to graph and function sorting:

  • onnxscript/ir/_core.py: Added the sort method to perform a topological sort on all graphs and functions, ensuring stability and preserving the original order as much as possible. This method raises a ValueError if a cycle is detected in the graph.

Copy link
Contributor

Copilot AI left a 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 1 comment.

@justinchuby justinchuby added the module: IR Intermediate representation label Apr 8, 2025
@codecov
Copy link

codecov bot commented Apr 8, 2025

❌ 3 Tests Failed:

Tests completed Failed Passed Skipped
16155 3 16152 3425
View the top 3 failed test(s) by shortest run time
onnxscript.backend.onnx_export_test.TestOnnxBackEnd::test_export2python_produces_correct_onnx_script_model_0125_test_ai_onnx_ml_tree_ensemble_set_membership
Stack Traces | 0.006s run time
onnxscript/converter.py:467: in _eval_constant_expr
    return eval(cpl, self.globals, locals)  # pylint: disable=eval-used
E   NameError: name 'nan' is not defined

The above exception was the direct cause of the following exception:
..../test_ort_nightly/lib/python3.11.../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:137: in extract_functions
    mod = importlib.import_module(import_name)
.../Library/Frameworks/Python.framework/Versions/3.11/lib/python3.11/importlib/__init__.py:126: in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
<frozen importlib._bootstrap>:1204: in _gcd_import
    ???
<frozen importlib._bootstrap>:1176: in _find_and_load
    ???
<frozen importlib._bootstrap>:1147: in _find_and_load_unlocked
    ???
<frozen importlib._bootstrap>:690: in _load_unlocked
    ???
..../test_ort_nightly/lib/python3.11.../_pytest/assertion/rewrite.py:185: in exec_module
    exec(co, module.__dict__)
tests/onnx_backend_test_code/test_ai_onnx_ml_tree_ensemble_set_membership.py:9: in <module>
    @script()
onnxscript/main.py:91: in transform
    result = script_check(f_ast, opset, env, src, default_opset=default_opset)
onnxscript/main.py:35: in script_check
    return convert.translate_function_def(f)
onnxscript/converter.py:1459: in translate_function_def
    fn_ir = self._translate_function_def_common(stmt)
onnxscript/converter.py:1446: in _translate_function_def_common
    self._translate_stmt(s, index_of_stmt=i)
onnxscript/converter.py:968: in _translate_stmt
    return self._translate_assign_stmt(node)
onnxscript/converter.py:1055: in _translate_assign_stmt
    assign(lhs, rhs)
onnxscript/converter.py:999: in assign
    t = self._translate_expr(rhs, lhs).name
onnxscript/converter.py:553: in _translate_expr
    r = self._translate_call_expr(node)
onnxscript/converter.py:832: in _translate_call_expr
    attrs = [
onnxscript/converter.py:833: in <listcomp>
    self._translate_attr(x, y, callee.op_schema.attributes[x])
onnxscript/converter.py:517: in _translate_attr
    val = self._eval_constant_expr(expr)
onnxscript/converter.py:469: in _eval_constant_expr
    raise NameError(
E   NameError: ERROR: Missing names, globals contains ['__name__', '__doc__', '__package__', '__loader__', '__spec__', '__file__', '__cached__', '__builtins__', '@py_builtins', '@pytest_ar', 'numpy', 'TensorProto', 'make_tensor', 'script', 'external_tensor', 'Opset', 'FLOAT', 'ai_onnx_ml5'], locals [].
E   at: Function 'bck_test_ai_onnx_ml_tree_ensemble_set_membership', line 3
E       Y = ai_onnx_ml5.TreeEnsemble(X, aggregate_function=1, leaf_targetids=[0, 1, 2, 3], leaf_weights=make_tensor("value", 1, dims=[4], vals=[1.0, 10.0, 1000.0, 100.0]), membership_values=make_tensor("value", 1, dims=[8], vals=[1.2000000476837158, 3.700000047683716, 8.0, 9.0, nan, 12.0, 7.0, nan]), n_targets=4, nodes_falseleafs=[1, 0, 1], nodes_falsenodeids=[2, 2, 3], nodes_featureids=[0, 0, 0], nodes_modes=make_tensor("value", 2, dims=[3], vals=[0, 6, 6]), nodes_splits=make_tensor("value", 1, dims=[3], vals=[11.0, 232344.0, nan]), nodes_trueleafs=[0, 1, 1], nodes_truenodeids=[1, 0, 1], post_transform=0, tree_roots=[0])
E                                                                                                                                                                                             ^
onnxscript.backend.onnx_export_test.TestOnnxBackEnd::test_double_attr_val_promotion
Stack Traces | 0.008s 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)
<frozen importlib._bootstrap>:1204: in _gcd_import
    ???
<frozen importlib._bootstrap>:1176: in _find_and_load
    ???
<frozen importlib._bootstrap>:1140: in _find_and_load_unlocked
    ???
E   ModuleNotFoundError: No module named 'tests.export.fun_with_double_attr_promotion'

The above exception was the direct cause of the following exception:
onnxscript\backend\onnx_export_test.py:195: in test_double_attr_val_promotion
    self._round_trip_check(fun_with_double_attr_promotion)
onnxscript\backend\onnx_export_test.py:172: in _round_trip_check
    map = extract_functions(proto.name, code, TestOnnxBackEnd.temp_folder)
onnxscript\backend\onnx_export_test.py:139: in extract_functions
    raise AssertionError(
E   AssertionError: Unable to import 'tests.export.fun_with_double_attr_promotion' (e=No module named 'tests.export.fun_with_double_attr_promotion') (file: 'D:\\a\\onnxscript\\onnxscript\\tests\\export\\fun_with_double_attr_promotion.py', absolute path: 'D:\\a\\onnxscript\\onnxscript\\tests\\export\\fun_with_double_attr_promotion.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   
E   from onnxscript.onnx_opset import opset17
E   this1 = Opset('this', 1)
E   
E   @script(this1)
E   def fun_with_double_attr_promotion(X, dtype: int):
E       dtype_1 = opset17.Constant(value_int=dtype)
E       dtype_cast = opset17.CastLike(dtype_1, X)
E       Y = opset17.Add(X, dtype_cast)
E       dtype_0 = opset17.Constant(value_int=dtype)
E       dtype_0_cast = opset17.CastLike(dtype_0, Y)
E       Z = opset17.Add(Y, dtype_0_cast)
E       return Z
onnxscript.backend.onnx_export_test.TestOnnxBackEnd::test_export2python_produces_correct_onnx_script_model_0026_test_ai_onnx_ml_tree_ensemble_set_membership
Stack Traces | 0.017s run time
onnxscript\converter.py:467: in _eval_constant_expr
    return eval(cpl, self.globals, locals)  # pylint: disable=eval-used
E   NameError: name 'nan' is not defined

The above exception was the direct cause of the following exception:
.nox\test_ort_nightly\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: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)
<frozen importlib._bootstrap>:1204: in _gcd_import
    ???
<frozen importlib._bootstrap>:1176: in _find_and_load
    ???
<frozen importlib._bootstrap>:1147: in _find_and_load_unlocked
    ???
<frozen importlib._bootstrap>:690: in _load_unlocked
    ???
.nox\test_ort_nightly\Lib\site-packages\_pytest\assertion\rewrite.py:185: in exec_module
    exec(co, module.__dict__)
tests\onnx_backend_test_code\test_ai_onnx_ml_tree_ensemble_set_membership.py:9: in <module>
    @script()
onnxscript\main.py:91: in transform
    result = script_check(f_ast, opset, env, src, default_opset=default_opset)
onnxscript\main.py:35: in script_check
    return convert.translate_function_def(f)
onnxscript\converter.py:1459: in translate_function_def
    fn_ir = self._translate_function_def_common(stmt)
onnxscript\converter.py:1446: in _translate_function_def_common
    self._translate_stmt(s, index_of_stmt=i)
onnxscript\converter.py:968: in _translate_stmt
    return self._translate_assign_stmt(node)
onnxscript\converter.py:1055: in _translate_assign_stmt
    assign(lhs, rhs)
onnxscript\converter.py:999: in assign
    t = self._translate_expr(rhs, lhs).name
onnxscript\converter.py:553: in _translate_expr
    r = self._translate_call_expr(node)
onnxscript\converter.py:832: in _translate_call_expr
    attrs = [
onnxscript\converter.py:833: in <listcomp>
    self._translate_attr(x, y, callee.op_schema.attributes[x])
onnxscript\converter.py:517: in _translate_attr
    val = self._eval_constant_expr(expr)
onnxscript\converter.py:469: in _eval_constant_expr
    raise NameError(
E   NameError: ERROR: Missing names, globals contains ['__name__', '__doc__', '__package__', '__loader__', '__spec__', '__file__', '__cached__', '__builtins__', '@py_builtins', '@pytest_ar', 'numpy', 'TensorProto', 'make_tensor', 'script', 'external_tensor', 'Opset', 'FLOAT', 'ai_onnx_ml5'], locals [].
E   at: Function 'bck_test_ai_onnx_ml_tree_ensemble_set_membership', line 3
E       Y = ai_onnx_ml5.TreeEnsemble(X, aggregate_function=1, leaf_targetids=[0, 1, 2, 3], leaf_weights=make_tensor("value", 1, dims=[4], vals=[1.0, 10.0, 1000.0, 100.0]), membership_values=make_tensor("value", 1, dims=[8], vals=[1.2000000476837158, 3.700000047683716, 8.0, 9.0, nan, 12.0, 7.0, nan]), n_targets=4, nodes_falseleafs=[1, 0, 1], nodes_falsenodeids=[2, 2, 3], nodes_featureids=[0, 0, 0], nodes_modes=make_tensor("value", 2, dims=[3], vals=[0, 6, 6]), nodes_splits=make_tensor("value", 1, dims=[3], vals=[11.0, 232344.0, nan]), nodes_trueleafs=[0, 1, 1], nodes_truenodeids=[1, 0, 1], post_transform=0, tree_roots=[0])
E                                                                                                                                                                                             ^

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

graph={textwrap.indent(repr(self.graph), " " * 4).strip()}
)"""

def sort(self) -> None:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

where would this be called?

graph={textwrap.indent(repr(self.graph), " " * 4).strip()}
)"""

def sort(self) -> None:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a unit test for this?

@justinchuby
Copy link
Collaborator Author

Replaced by the topological sort pass

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

module: IR Intermediate representation

Projects

Development

Successfully merging this pull request may close these issues.

3 participants