Use the new IR for building graphs in torchlib #1354

justinchuby · 2024-04-09T02:13:25Z

Use the new IR for building graphs in torchlib. I created a flag TORCHLIB_EXPERIMENTAL_USE_IR to control the feature. When enabled the classes in graph_building will be swapped to use the new IR. The exporter should see the same interfaces and not feel anything. After the transition period we can decide where this should live.

Set TORCHLIB_EXPERIMENTAL_USE_IR=1 to enable this feature.

Fix #997

codecov · 2024-04-09T02:17:00Z

Codecov Report

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

Project coverage is 77.17%. Comparing base (f06f303) to head (6fcd30e).

Files	Patch %	Lines
...ibs/torch_lib/graph_building/_graph_building_ir.py	67.21%	84 Missing and 16 partials ⚠️
onnxscript/ir/_naming_authority.py	30.00%	14 Missing ⚠️
onnxscript/_internal/runtime_typing.py	50.00%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1354      +/-   ##
==========================================
- Coverage   77.26%   77.17%   -0.09%     
==========================================
  Files         209      212       +3     
  Lines       22533    22867     +334     
  Branches     3806     3887      +81     
==========================================
+ Hits        17410    17648     +238     
- Misses       4418     4502      +84     
- Partials      705      717      +12

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

github-advanced-security

lintrunner found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.

github-actions · 2024-04-09T02:30:36Z

Test Results

28 files + 2 28 suites +2 3h 21m 9s ⏱️ - 14m 28s
7 135 tests + 1 300 3 974 ✅ + 17 1 972 💤 + 98 1 188 ❌ + 1 184 1 🔥 + 1
415 865 runs - 55 782 61 182 ✅ - 14 017 317 894 💤 - 78 550 36 777 ❌ +36 773 12 🔥 +12

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

Results for commit d28b8d4. ± Comparison against base commit 9ea1d1c.

This pull request removes 68 and adds 1368 tests. Note that renamed tests count towards both.

onnxscript.function_libs.torch_lib.graph_building_test.TestModelSaving ‑ test_experimental_function_value_info_are_stored_in_graph_value_info
onnxscript.function_libs.torch_lib.graph_building_test.TestModelSaving ‑ test_input_output_and_initializer_are_not_stored_in_value_info
onnxscript.function_libs.torch_lib.graph_building_test.TestModelSaving ‑ test_save_initializer_to_files_for_large_model
onnxscript.function_libs.torch_lib.graph_building_test.TestTorchScriptGraph ‑ test_add_initializer_allows_adding_the_same_tensor_twice_using_same_name
onnxscript.function_libs.torch_lib.graph_building_test.TestTorchScriptGraph ‑ test_add_initializer_raises_when_the_same_name_used_for_different_tensors
onnxscript.function_libs.torch_lib.graph_building_test.TestTorchScriptTracingEvaluator ‑ test_add_input_with_optionaltype_does_not_raise_torch_internal_error
onnxscript.function_libs.torch_lib.graph_building_test.TestTorchScriptTracingEvaluator ‑ test_model_local_function_constructed_by_traced_graph_is_same_as_compiled_graph
onnxscript.function_libs.torch_lib.graph_building_test.TestTorchScriptTracingEvaluator ‑ test_traced_constant_op_is_same_as_compiled_graph
onnxscript.function_libs.torch_lib.graph_building_test.TestTorchScriptTracingEvaluator ‑ test_traced_graph_on_single_node_is_same_as_compiled_graph
onnxscript.function_libs.torch_lib.graph_building_test.TestTorchScriptTracingEvaluator ‑ test_traced_graph_on_single_node_multi_output_is_same_as_compiled_graph
…

onnxscript.backend.onnx_export_test.TestOnnxBackEnd ‑ test_export2python_produces_correct_onnx_script_model_0000_test_abs
onnxscript.backend.onnx_export_test.TestOnnxBackEnd ‑ test_export2python_produces_correct_onnx_script_model_0001_test_acos
onnxscript.backend.onnx_export_test.TestOnnxBackEnd ‑ test_export2python_produces_correct_onnx_script_model_0002_test_acosh
onnxscript.backend.onnx_export_test.TestOnnxBackEnd ‑ test_export2python_produces_correct_onnx_script_model_0003_test_acosh_example
onnxscript.backend.onnx_export_test.TestOnnxBackEnd ‑ test_export2python_produces_correct_onnx_script_model_0004_test_acos_example
onnxscript.backend.onnx_export_test.TestOnnxBackEnd ‑ test_export2python_produces_correct_onnx_script_model_0005_test_adagrad
onnxscript.backend.onnx_export_test.TestOnnxBackEnd ‑ test_export2python_produces_correct_onnx_script_model_0006_test_adagrad_multiple
onnxscript.backend.onnx_export_test.TestOnnxBackEnd ‑ test_export2python_produces_correct_onnx_script_model_0007_test_adam
onnxscript.backend.onnx_export_test.TestOnnxBackEnd ‑ test_export2python_produces_correct_onnx_script_model_0008_test_adam_multiple
onnxscript.backend.onnx_export_test.TestOnnxBackEnd ‑ test_export2python_produces_correct_onnx_script_model_0009_test_add
…

This pull request removes 1 skipped test and adds 335 skipped tests. Note that renamed tests count towards both.

onnxscript.function_libs.torch_lib.graph_building_test.TestTorchScriptTracingEvaluator ‑ test_traced_graph_on_single_node_multi_output_is_same_as_compiled_graph

onnxscript.backend.onnx_export_test.TestOnnxBackEnd ‑ test_export2python_produces_correct_onnx_script_model_0005_test_adagrad
onnxscript.backend.onnx_export_test.TestOnnxBackEnd ‑ test_export2python_produces_correct_onnx_script_model_0006_test_adagrad_multiple
onnxscript.backend.onnx_export_test.TestOnnxBackEnd ‑ test_export2python_produces_correct_onnx_script_model_0007_test_adam
onnxscript.backend.onnx_export_test.TestOnnxBackEnd ‑ test_export2python_produces_correct_onnx_script_model_0008_test_adam_multiple
onnxscript.backend.onnx_export_test.TestOnnxBackEnd ‑ test_export2python_produces_correct_onnx_script_model_0011_test_add_uint8
onnxscript.backend.onnx_export_test.TestOnnxBackEnd ‑ test_export2python_produces_correct_onnx_script_model_0020_test_ai_onnx_ml_array_feature_extractor
onnxscript.backend.onnx_export_test.TestOnnxBackEnd ‑ test_export2python_produces_correct_onnx_script_model_0021_test_ai_onnx_ml_binarizer
onnxscript.backend.onnx_export_test.TestOnnxBackEnd ‑ test_export2python_produces_correct_onnx_script_model_0022_test_ai_onnx_ml_label_encoder_string_int
onnxscript.backend.onnx_export_test.TestOnnxBackEnd ‑ test_export2python_produces_correct_onnx_script_model_0023_test_ai_onnx_ml_label_encoder_string_int_no_default
onnxscript.backend.onnx_export_test.TestOnnxBackEnd ‑ test_export2python_produces_correct_onnx_script_model_0024_test_ai_onnx_ml_label_encoder_tensor_mapping
…

♻️ This comment has been updated with latest results.

onnxscript/function_libs/torch_lib/graph_building/__init__.py

onnxscript/function_libs/torch_lib/graph_building/_graph_building_ir.py

+    return _ONNX_DTYPE_TO_TORCH[dtype]
+
+
+class TorchScriptTensor(ir.Value, onnxscript_tensor.Tensor):


onnxscript/function_libs/torch_lib/graph_building/_graph_building_ir.py

onnxscript/ir/_core.py

thiagocrepaldi

Overall LGTM. The TorchScript naming should go away, though :)

Maybe some docstrings for both graph_building/__init__.py and ir/__init__.py exaplining the goal of each module, the basic design idea/mechanics would help a lot the hand off to pytorch and new comers that start on the project

justinchuby · 2024-04-19T02:50:46Z

The TorchScript naming should go away

Absolutely. We need the exporter to decide how it wants to interface with the IR and move away from referencing this object. Then we can remove this class

onnxscript/function_libs/torch_lib/graph_building/_graph_building_ir.py

justinchuby · 2024-04-19T17:39:10Z

Maybe some docstrings for both graph_building/__init__.py and ir/__init__.py exaplining the goal of each module, the basic design idea/mechanics would help a lot the hand off to pytorch and new comers that start on the project

I added that to graph_building/__init__.py. I will improve docs to the ir in a separate PR.

onnxscript/function_libs/torch_lib/graph_building/_graph_building_ir.py

github-advanced-security bot found potential problems Apr 9, 2024

View reviewed changes

justinchuby added module: torchlib Related to the torch/aten function lib in development module: IR Intermediate representation labels Apr 9, 2024

justinchuby force-pushed the justinchu/ir-build-graph branch from 21b7c4e to ccdca53 Compare April 9, 2024 18:47

github-advanced-security bot found potential problems Apr 9, 2024

View reviewed changes

justinchuby added 7 commits April 9, 2024 23:55

[WIP] Use the new IR for building graphs in torchlib

d43c870

Snapshot

8c95df7

Update element type

60ae675

snap

ed1f401

Clean up

808c434

update

3c7b7d7

naming

529801f

justinchuby force-pushed the justinchu/ir-build-graph branch from 9f1f36f to 529801f Compare April 9, 2024 23:56

Merge branch 'main' into justinchu/ir-build-graph

9bb3a5a

justinchuby changed the title ~~[WIP] Use the new IR for building graphs in torchlib~~ Use the new IR for building graphs in torchlib Apr 12, 2024

justinchuby marked this pull request as ready for review April 12, 2024 04:21

justinchuby marked this pull request as draft April 12, 2024 04:22

github-advanced-security bot found potential problems Apr 12, 2024

View reviewed changes

justinchuby added 3 commits April 12, 2024 04:51

fix tensor attr and shape

047688c

fix print

77d89c6

fix graph print

6b8f847

github-advanced-security bot found potential problems Apr 12, 2024

View reviewed changes

justinchuby added 2 commits April 12, 2024 05:14

more print fix

db482e5

Fix dtype and add_input

6c2a75d

github-advanced-security bot found potential problems Apr 12, 2024

View reviewed changes

justinchuby added 2 commits April 12, 2024 05:46

Fix node and empty shape

6317fe2

methods for dimension

a44a589

github-advanced-security bot found potential problems Apr 17, 2024

View reviewed changes

justinchuby and others added 3 commits April 17, 2024 16:39

test

f4c9f9a

test pipeline

49864c2

Merge branch 'main' into justinchu/ir-build-graph

b1dfe11

justinchuby marked this pull request as ready for review April 17, 2024 16:42

Update main.yaml

472bcce

justinchuby commented Apr 17, 2024

View reviewed changes

onnxscript/ir/_core.py Outdated Show resolved Hide resolved

Update onnxscript/ir/_core.py

2e8f7d4

justinchuby requested a review from thiagocrepaldi April 17, 2024 21:55

thiagocrepaldi approved these changes Apr 18, 2024

View reviewed changes

justinchuby added 5 commits April 19, 2024 05:08

Merge branch 'main' into justinchu/ir-build-graph

269d6f8

typing

4632cb5

mypy

fbc236c

Robust torchtensor handling

26a48f5

lint

d28b8d4

github-advanced-security bot found potential problems Apr 19, 2024

View reviewed changes

onnxscript/function_libs/torch_lib/graph_building/_graph_building_ir.py Fixed Show fixed Hide fixed

Remove value

0515ce0

_concrete_value

b70a3c0

github-advanced-security bot found potential problems Apr 19, 2024

View reviewed changes

onnxscript/function_libs/torch_lib/graph_building/_graph_building_ir.py Fixed Show fixed Hide fixed

justinchuby and others added 3 commits April 19, 2024 17:52

value

43ddafc

remove type checks

31ec8e5

Merge branch 'main' into justinchu/ir-build-graph

446468d

github-advanced-security bot found potential problems Apr 19, 2024

View reviewed changes

onnxscript/function_libs/torch_lib/graph_building/_graph_building_ir.py Fixed Show fixed Hide fixed

github-advanced-security bot found potential problems Apr 19, 2024

View reviewed changes

lint

6fcd30e

justinchuby merged commit 63f3809 into main Apr 19, 2024
35 of 41 checks passed

justinchuby deleted the justinchu/ir-build-graph branch April 19, 2024 18:37

justinchuby mentioned this pull request Apr 19, 2024

IR Integration plan #1384

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use the new IR for building graphs in torchlib #1354

Use the new IR for building graphs in torchlib #1354

justinchuby commented Apr 9, 2024 •

edited

Loading

codecov bot commented Apr 9, 2024 •

edited

Loading

github-advanced-security bot left a comment

github-actions bot commented Apr 9, 2024 •

edited

Loading

Check warning

thiagocrepaldi left a comment

justinchuby commented Apr 19, 2024 •

edited

Loading

justinchuby commented Apr 19, 2024

		return _ONNX_DTYPE_TO_TORCH[dtype]


		class TorchScriptTensor(ir.Value, onnxscript_tensor.Tensor):

Use the new IR for building graphs in torchlib #1354

Use the new IR for building graphs in torchlib #1354

Conversation

justinchuby commented Apr 9, 2024 • edited Loading

codecov bot commented Apr 9, 2024 • edited Loading

Codecov Report

github-advanced-security bot left a comment

Choose a reason for hiding this comment

github-actions bot commented Apr 9, 2024 • edited Loading

Test Results

Check warning

thiagocrepaldi left a comment

Choose a reason for hiding this comment

justinchuby commented Apr 19, 2024 • edited Loading

justinchuby commented Apr 19, 2024

justinchuby commented Apr 9, 2024 •

edited

Loading

codecov bot commented Apr 9, 2024 •

edited

Loading

github-actions bot commented Apr 9, 2024 •

edited

Loading

justinchuby commented Apr 19, 2024 •

edited

Loading