[ONNX] Register list/tuple/dict to format_argumment and refactor fx.Node format_argument in diagnostics #105263

titaiwangms · 2023-07-15T03:25:58Z

Stack from ghstack (oldest at bottom):

-> [ONNX] Register list/tuple/dict to format_argumment and refactor fx.Node format_argument in diagnostics #105263

Previous to this PR, the SARIF reports don't have detail on torch.fx.Node (shape/dtype), and don't unpack the tuple/list/dict. This PR provides thorough information of args/kwargs from torch in fx.graph expression: f32[64, 64, 2] (dtype[shape]).

Need microsoft/onnxscript#890

…ode format_argument in diagnostics [ghstack-poisoned]

pytorch-bot · 2023-07-15T03:26:01Z

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/105263

📄 Preview Python docs built from this PR
📄 Preview C++ docs built from this PR
❓ Need help or want to give feedback on the CI? Visit the bot commands wiki or our office hours

Note: Links to docs will display an error until the docs builds have been completed.

✅ 4 Unrelated Failures

As of commit 2d6fe3b:

UNSTABLE - The following jobs failed but were likely due to flakiness present on trunk and has been marked as unstable:

This comment was automatically generated by Dr. CI and updates every 15 minutes.

…ode format_argument in diagnostics ghstack-source-id: e4a89ad6f1fe3d663ea3327a42ebc06071def218 Pull Request resolved: #105263

…factor fx.Node format_argument in diagnostics" [ghstack-poisoned]

…ode format_argument in diagnostics ghstack-source-id: ed61ca152af9cfe493ddd9ed0ef3427348a92cf8 Pull Request resolved: #105263

Due to the need of supporting None args, we need to support None (torch.OptionalType) to get shape/dtype. To support pytorch/pytorch#105263 --------- Co-authored-by: Justin Chu <justinchuby@users.noreply.github.com>

…factor fx.Node format_argument in diagnostics" Previous to this PR, the SARIF reports don't have detail on torch.fx.Node (shape/dtype), and don't unpack the tuple/list/dict. This PR provides thorough information of args/kwargs from torch in fx.graph expression: f32[64, 64, 2] (dtype[shape]). Need microsoft/onnxscript#890 ![dispatcher_sarif](https://github.com/pytorch/pytorch/assets/18010845/2567fac6-4154-4ce8-bc34-83950ef1c1d7) [ghstack-poisoned]

…ode format_argument in diagnostics ghstack-source-id: 50421618df5cd977aeadd5aa68cfc0b0bcadd2e9 Pull Request resolved: #105263

torch/onnx/_internal/fx/diagnostics.py

…factor fx.Node format_argument in diagnostics" Previous to this PR, the SARIF reports don't have detail on torch.fx.Node (shape/dtype), and don't unpack the tuple/list/dict. This PR provides thorough information of args/kwargs from torch in fx.graph expression: f32[64, 64, 2] (dtype[shape]). Need microsoft/onnxscript#890 ![dispatcher_sarif](https://github.com/pytorch/pytorch/assets/18010845/2567fac6-4154-4ce8-bc34-83950ef1c1d7) [ghstack-poisoned]

…ode format_argument in diagnostics ghstack-source-id: b6ce5444604f9bb70def741efde620bfc07c6cf1 Pull Request resolved: #105263

…factor fx.Node format_argument in diagnostics" Previous to this PR, the SARIF reports don't have detail on torch.fx.Node (shape/dtype), and don't unpack the tuple/list/dict. This PR provides thorough information of args/kwargs from torch in fx.graph expression: f32[64, 64, 2] (dtype[shape]). Need microsoft/onnxscript#890 ![dispatcher_sarif](https://github.com/pytorch/pytorch/assets/18010845/2567fac6-4154-4ce8-bc34-83950ef1c1d7) [ghstack-poisoned]

…ode format_argument in diagnostics ghstack-source-id: 879f7976faf3264d61093be1c65a819cbb83fe03 Pull Request resolved: #105263

thiagocrepaldi

nice improvement. I would consider covering more complex data types which combine data structs, such as dict with tuples as values, etc

.ci/docker/common/install_onnx.sh

torch/onnx/_internal/fx/diagnostics.py

…factor fx.Node format_argument in diagnostics" Previous to this PR, the SARIF reports don't have detail on torch.fx.Node (shape/dtype), and don't unpack the tuple/list/dict. This PR provides thorough information of args/kwargs from torch in fx.graph expression: f32[64, 64, 2] (dtype[shape]). Need microsoft/onnxscript#890 ![dispatcher_sarif](https://github.com/pytorch/pytorch/assets/18010845/2567fac6-4154-4ce8-bc34-83950ef1c1d7) [ghstack-poisoned]

…ode format_argument in diagnostics ghstack-source-id: 0c6507f7624380ce7983069b0cae855621a526c4 Pull Request resolved: #105263

titaiwangms · 2023-07-21T18:10:10Z

@BowenBao PTAL

BowenBao · 2023-07-21T20:19:31Z

test/onnx/test_fx_op_consistency.py

    xfail(
        "nn.functional.adaptive_avg_pool2d",
-        reason=onnx_test_common.reason_onnx_script_does_not_support("aten.index.Tensor"),
+        reason=onnx_test_common.reason_dynamo_does_not_support("RecursionError: maximum recursion depth exceeded \


decompose pass.. probably our bug not dynamo's?

I'm getting this error onnxruntime.capi.onnxruntime_pybind11_state.Fail: [ONNXRuntimeError] : 1 : FAIL : Node (AveragePool_0) Op (AveragePool) [ShapeInferenceError] Attribute strides has incorrect size ...

I just repro. Maybe try the latest commit of onnx-script. I had an update on avg_pool in torchlib.

test/onnx/test_fx_op_consistency.py

torch/onnx/_internal/fx/type_utils.py

torch/onnx/_internal/fx/diagnostics.py

BowenBao · 2023-07-21T20:25:21Z

torch/onnx/_internal/fx/diagnostics.py

+
+@_format_argument.register
+def _torch_fx_symbolic_value(
+    obj,  # NOTE: functools.singledispatch does not support Union until 3.11, so we use Any here.


I wonder if a silly approach of creating 3 different functions for it is viable :)

should then allow format_argument to be used instead of _format_nested_argument_by_dtype

BowenBao

Nice! LG w/ comment.

A few follow ups

Add format_argument for SymbolicFunction. Not sure what is the best format, but right now it is only showing class name. We might be interested in the overload, whether it is custom, and maybe param schema but that might be too verbose.
Avoid printing long tuple/list/dict. I think a few cases like decomp table, list of nodes of fx graph might fall into this category.
Add format_argument for primitive types like int. This probably should go into base format_argument inside the base diagnostic infra package.

…factor fx.Node format_argument in diagnostics" Previous to this PR, the SARIF reports don't have detail on torch.fx.Node (shape/dtype), and don't unpack the tuple/list/dict. This PR provides thorough information of args/kwargs from torch in fx.graph expression: f32[64, 64, 2] (dtype[shape]). Need microsoft/onnxscript#890 ![dispatcher_sarif](https://github.com/pytorch/pytorch/assets/18010845/2567fac6-4154-4ce8-bc34-83950ef1c1d7) [ghstack-poisoned]

…ode format_argument in diagnostics ghstack-source-id: 758c2290f185fa83d31acc7d9b82761d9c99a521 Pull Request resolved: #105263

titaiwangms · 2023-07-22T05:54:21Z

Nice! LG w/ comment.

A few follow ups

Add format_argument for SymbolicFunction. Not sure what is the best format, but right now it is only showing class name. We might be interested in the overload, whether it is custom, and maybe param schema but that might be too verbose.

Avoid printing long tuple/list/dict. I think a few cases like decomp table, list of nodes of fx graph might fall into this category.

Add format_argument for primitive types like int. This probably should go into base format_argument inside the base diagnostic infra package.

#105786

titaiwangms · 2023-07-22T05:54:56Z

@pytorchbot merge

pytorchmergebot · 2023-07-22T05:56:28Z

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging

Check the merge workflow status
here

[ONNX] Register list/tuple/dict to format_argumment and refactor fx.N…

3b9de03

…ode format_argument in diagnostics [ghstack-poisoned]

titaiwangms requested review from BowenBao, abock and thiagocrepaldi as code owners July 15, 2023 03:25

pytorch-bot bot added the release notes: onnx torch.onnx related changes that should show up in the release notes label Jul 15, 2023

titaiwangms added a commit that referenced this pull request Jul 15, 2023

[ONNX] Register list/tuple/dict to format_argumment and refactor fx.N…

fc635a2

…ode format_argument in diagnostics ghstack-source-id: e4a89ad6f1fe3d663ea3327a42ebc06071def218 Pull Request resolved: #105263

titaiwangms marked this pull request as draft July 15, 2023 03:26

pytorchbot added the open source label Jul 15, 2023

titaiwangms linked an issue Jul 17, 2023 that may be closed by this pull request

[ONNX] Add fine-grained diagnostic message in onnx dispatcher to reveal input/attribute-schema matching #105174

Closed

titaiwangms mentioned this pull request Jul 17, 2023

Support OptionalType to get shape | feat(graph_building) microsoft/onnxscript#890

Merged

Update on "[ONNX] Register list/tuple/dict to format_argumment and re…

cdfe40b

…factor fx.Node format_argument in diagnostics" [ghstack-poisoned]

titaiwangms added a commit that referenced this pull request Jul 17, 2023

[ONNX] Register list/tuple/dict to format_argumment and refactor fx.N…

0e397f8

…ode format_argument in diagnostics ghstack-source-id: ed61ca152af9cfe493ddd9ed0ef3427348a92cf8 Pull Request resolved: #105263

titaiwangms marked this pull request as ready for review July 17, 2023 23:08

titaiwangms added a commit that referenced this pull request Jul 18, 2023

[ONNX] Register list/tuple/dict to format_argumment and refactor fx.N…

6b9ce30

…ode format_argument in diagnostics ghstack-source-id: 50421618df5cd977aeadd5aa68cfc0b0bcadd2e9 Pull Request resolved: #105263

titaiwangms commented Jul 18, 2023

View reviewed changes

torch/onnx/_internal/fx/diagnostics.py Outdated Show resolved Hide resolved

titaiwangms added a commit that referenced this pull request Jul 18, 2023

[ONNX] Register list/tuple/dict to format_argumment and refactor fx.N…

ab704e5

…ode format_argument in diagnostics ghstack-source-id: b6ce5444604f9bb70def741efde620bfc07c6cf1 Pull Request resolved: #105263

titaiwangms added a commit that referenced this pull request Jul 18, 2023

[ONNX] Register list/tuple/dict to format_argumment and refactor fx.N…

00a3ed5

…ode format_argument in diagnostics ghstack-source-id: 879f7976faf3264d61093be1c65a819cbb83fe03 Pull Request resolved: #105263

thiagocrepaldi reviewed Jul 18, 2023

View reviewed changes

.ci/docker/common/install_onnx.sh Outdated Show resolved Hide resolved

torch/onnx/_internal/fx/diagnostics.py Show resolved Hide resolved

torch/onnx/_internal/fx/diagnostics.py Outdated Show resolved Hide resolved

torch/onnx/_internal/fx/diagnostics.py Outdated Show resolved Hide resolved

titaiwangms added module: onnx Related to torch.onnx onnx-triaged triaged by ONNX team topic: improvements topic category labels Jul 18, 2023

titaiwangms added a commit that referenced this pull request Jul 19, 2023

[ONNX] Register list/tuple/dict to format_argumment and refactor fx.N…

7dd9b09

…ode format_argument in diagnostics ghstack-source-id: 0c6507f7624380ce7983069b0cae855621a526c4 Pull Request resolved: #105263

BowenBao reviewed Jul 21, 2023

View reviewed changes

test/onnx/test_fx_op_consistency.py Outdated Show resolved Hide resolved

BowenBao reviewed Jul 21, 2023

View reviewed changes

torch/onnx/_internal/fx/type_utils.py Outdated Show resolved Hide resolved

BowenBao reviewed Jul 21, 2023

View reviewed changes

torch/onnx/_internal/fx/diagnostics.py Outdated Show resolved Hide resolved

BowenBao reviewed Jul 21, 2023

View reviewed changes

BowenBao approved these changes Jul 21, 2023

View reviewed changes

titaiwangms added a commit that referenced this pull request Jul 22, 2023

[ONNX] Register list/tuple/dict to format_argumment and refactor fx.N…

ec47c86

…ode format_argument in diagnostics ghstack-source-id: 758c2290f185fa83d31acc7d9b82761d9c99a521 Pull Request resolved: #105263

titaiwangms mentioned this pull request Jul 22, 2023

[ONNX] Format register primitive types, and SymbolicFunction, and ignore too long tuple in SARIF #105786

Closed

titaiwangms added the ciflow/trunk Trigger trunk jobs on your pull request label Jul 22, 2023

pytorchmergebot added the merging label Jul 22, 2023

pytorchmergebot added Merged and removed merging labels Jul 22, 2023

pytorchmergebot closed this in 20fb2ba Jul 22, 2023

facebook-github-bot deleted the gh/titaiwangms/31/head branch July 25, 2023 14:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ONNX] Register list/tuple/dict to format_argumment and refactor fx.Node format_argument in diagnostics #105263

[ONNX] Register list/tuple/dict to format_argumment and refactor fx.Node format_argument in diagnostics #105263

titaiwangms commented Jul 15, 2023 •

edited

pytorch-bot bot commented Jul 15, 2023 •

edited

thiagocrepaldi left a comment

titaiwangms commented Jul 21, 2023

BowenBao Jul 21, 2023

BowenBao Jul 21, 2023

titaiwangms Jul 21, 2023

titaiwangms Jul 22, 2023

BowenBao Jul 21, 2023

BowenBao Jul 21, 2023

titaiwangms Jul 22, 2023

BowenBao left a comment

titaiwangms commented Jul 22, 2023

titaiwangms commented Jul 22, 2023

pytorchmergebot commented Jul 22, 2023

[ONNX] Register list/tuple/dict to format_argumment and refactor fx.Node format_argument in diagnostics #105263

[ONNX] Register list/tuple/dict to format_argumment and refactor fx.Node format_argument in diagnostics #105263

Conversation

titaiwangms commented Jul 15, 2023 • edited

pytorch-bot bot commented Jul 15, 2023 • edited

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/105263

✅ 4 Unrelated Failures

thiagocrepaldi left a comment

Choose a reason for hiding this comment

titaiwangms commented Jul 21, 2023

BowenBao Jul 21, 2023

Choose a reason for hiding this comment

BowenBao Jul 21, 2023

Choose a reason for hiding this comment

titaiwangms Jul 21, 2023

Choose a reason for hiding this comment

titaiwangms Jul 22, 2023

Choose a reason for hiding this comment

BowenBao Jul 21, 2023

Choose a reason for hiding this comment

BowenBao Jul 21, 2023

Choose a reason for hiding this comment

titaiwangms Jul 22, 2023

Choose a reason for hiding this comment

BowenBao left a comment

Choose a reason for hiding this comment

titaiwangms commented Jul 22, 2023

titaiwangms commented Jul 22, 2023

pytorchmergebot commented Jul 22, 2023

Merge started

titaiwangms commented Jul 15, 2023 •

edited

pytorch-bot bot commented Jul 15, 2023 •

edited