Skip to content
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

[WIP] Use IR in onnxscript #1409

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft

[WIP] Use IR in onnxscript #1409

wants to merge 5 commits into from

Conversation

justinchuby
Copy link
Collaborator

@justinchuby justinchuby commented Apr 19, 2024

Fixes #1410

self.callee.name,
[_opt_var_to_str(x) for x in self.args],
[str(x) for x in self.result],
def to_node(self, node_name: str, values: Mapping[str, ir.Value]) -> ir.Node:

Check warning

Code scanning / lintrunner

PYLINT/W0621 Warning

Redefining name 'values' from outer scope (line 19) (redefined-outer-name)
See redefined-outer-name. To disable, use # pylint: disable=redefined-outer-name
onnxscript/irbuilder.py Fixed Show fixed Hide fixed
onnxscript/irbuilder.py Fixed Show fixed Hide fixed
onnxscript/irbuilder.py Fixed Show fixed Hide fixed
onnxscript/irbuilder.py Fixed Show fixed Hide fixed
onnxscript/irbuilder.py Fixed Show fixed Hide fixed
onnxscript/irbuilder.py Fixed Show fixed Hide fixed
onnxscript/irbuilder.py Fixed Show fixed Hide fixed
onnxscript/irbuilder.py Fixed Show fixed Hide fixed
@@ -8,18 +8,18 @@
import io
import logging
import warnings
from typing import Any, Optional, Protocol, Sequence, Union
from typing import Any, Mapping, Optional, Protocol, Sequence, Union

Check warning

Code scanning / lintrunner

RUFF/F401 Warning

typing.Protocol imported but unused.
See https://docs.astral.sh/ruff/rules/unused-import
Copy link

codecov bot commented Apr 19, 2024

Codecov Report

Attention: Patch coverage is 46.80851% with 25 lines in your changes are missing coverage. Please review.

Project coverage is 51.25%. Comparing base (c979aae) to head (060009e).

Files Patch % Lines
onnxscript/irbuilder.py 42.85% 24 Missing ⚠️
onnxscript/values.py 66.66% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main    #1409       +/-   ##
===========================================
- Coverage   77.04%   51.25%   -25.80%     
===========================================
  Files         209      200        -9     
  Lines       22488    20898     -1590     
  Branches     3813     3607      -206     
===========================================
- Hits        17326    10711     -6615     
- Misses       4451     9648     +5197     
+ Partials      711      539      -172     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@justinchuby justinchuby linked an issue Apr 19, 2024 that may be closed by this pull request
self.name,
inputs=[x.name for x in self.inputs],
outputs=[y.name for y in self.outputs],
values = {}

Check warning

Code scanning / lintrunner

PYLINT/W0621 Warning

Redefining name 'values' from outer scope (line 19) (redefined-outer-name)
See redefined-outer-name. To disable, use # pylint: disable=redefined-outer-name
self.name,
inputs=[x.name for x in self.inputs],
outputs=[y.name for y in self.outputs],
values = {}

Check failure

Code scanning / lintrunner

MYPY/var-annotated Error

Need type annotation for "values" (hint: "values: Dict[, ] = ...") To disable, use # type: ignore[var-annotated]
nodes.append(node)
if node.domain not in opsets:
# FIXME(justinchuby): Node version
assert s.version is not None

Check failure

Code scanning / lintrunner

MYPY/attr-defined Error

"IRStmt" has no attribute "version" To disable, use # type: ignore[attr-defined]
if node.domain not in opsets:
# FIXME(justinchuby): Node version
assert s.version is not None
opsets[node.domain] = s.version

Check failure

Code scanning / lintrunner

MYPY/attr-defined Error

"IRStmt" has no attribute "version" To disable, use # type: ignore[attr-defined]
if output.name in function_outputs:
function_outputs[output.name] = output
inputs = [ir.Input(input.name) for input in self.inputs]
for name, output in function_outputs.items():

Check failure

Code scanning / lintrunner

MYPY/assignment Error

Incompatible types in assignment (expression has type "Value | None", variable has type "Value") To disable, use # type: ignore[assignment]
return IRAttributeValue(proto)
def make_attr_ref(self, attrname: str, refname: str, pytype: type) -> ir.RefAttr:
return ir.RefAttr(
attrname, refname, ta.pytype_to_attrtype(pytype)

Check failure

Code scanning / lintrunner

MYPY/arg-type Error

Argument 3 to "RefAttr" has incompatible type "onnx.onnx_ml_pb2.AttributeProto.AttributeType | None"; expected "onnxscript.ir._enums.AttributeType" To disable, use # type: ignore[arg-type]
@justinchuby
Copy link
Collaborator Author

Should stmt subclass node or own a node?

Copy link

github-actions bot commented Apr 19, 2024

Test Results

    30 files  +      2     30 suites  +2   15m 37s ⏱️ - 3h 27m 15s
   415 tests  -   5 433    334 ✅  -  3 630   13 💤  -   1 864     50 ❌ +   43     18 🔥 +   18 
11 451 runs   - 497 576  8 016 ✅  - 72 582  312 💤  - 428 086  1 200 ❌ +1 169  1 923 🔥 +1 923 

For more details on these failures and errors, see this check.

Results for commit 060009e. ± Comparison against base commit c979aae.

This pull request removes 5451 and adds 18 tests. Note that renamed tests count towards both.
docs.test.test_documentation_examples.TestDocumentationExample ‑ test_documentation_examples
onnxscript.backend.onnx_export_test.TestOnnxBackEnd ‑ test_attr_ref
onnxscript.backend.onnx_export_test.TestOnnxBackEnd ‑ test_double_attr_val_promotion
onnxscript.backend.onnx_export_test.TestOnnxBackEnd ‑ test_export2python
onnxscript.backend.onnx_export_test.TestOnnxBackEnd ‑ test_export2python_produces_correct_onnx_script_model_0000_test_convinteger_without_padding
onnxscript.backend.onnx_export_test.TestOnnxBackEnd ‑ test_export2python_produces_correct_onnx_script_model_0001_test_sqrt
onnxscript.backend.onnx_export_test.TestOnnxBackEnd ‑ test_export2python_produces_correct_onnx_script_model_0002_test_min_int32
onnxscript.backend.onnx_export_test.TestOnnxBackEnd ‑ test_export2python_produces_correct_onnx_script_model_0003_test_col2im
onnxscript.backend.onnx_export_test.TestOnnxBackEnd ‑ test_export2python_produces_correct_onnx_script_model_0004_test_reduce_sum_square_do_not_keepdims_example_expanded
onnxscript.backend.onnx_export_test.TestOnnxBackEnd ‑ test_export2python_produces_correct_onnx_script_model_0005_test_mod_mixed_sign_int8
…
onnxscript.backend.onnx_export_test
onnxscript.function_libs.tools.torch_lib.deduce_type_constraints_test
onnxscript.function_libs.torch_lib.graph_building.__init__
onnxscript.function_libs.torch_lib.graph_building._graph_building_ir
onnxscript.function_libs.torch_lib.graph_building._graph_building_torch
onnxscript.function_libs.torch_lib.graph_building.graph_building_test
onnxscript.function_libs.torch_lib.ops.__init__
onnxscript.function_libs.torch_lib.ops.common
onnxscript.function_libs.torch_lib.ops.core
onnxscript.function_libs.torch_lib.ops.fft
…

♻️ This comment has been updated with latest results.

default_value: int | float | str | None,
) -> None:
fn.add_attr_parameter(IRAttributeParameter(varname, attribute_type, default_value))
fn.add_attr_parameter(ir_convenience.convert_attribute(varname, default_value, attribute_type))

Check failure

Code scanning / lintrunner

MYPY/arg-type Error

Argument 1 to "add_attr_parameter" of "IRFunction" has incompatible type "Attr | RefAttr"; expected "Attr" To disable, use # type: ignore[arg-type]
@gramalingam
Copy link
Collaborator

I think this is outdated and can be closed, right @justinchuby ?

@justinchuby
Copy link
Collaborator Author

I will come back to this after exporter. I think some of the code is still relevant

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

Use IR in ONNX Script converter
2 participants