Match get_attr when compare node #91657

cccclai · 2023-01-03T20:51:03Z

Stack from ghstack (oldest at bottom):

-> Match get_attr when compare node #91657

The pattern can't be matched if one attribute is _param_constant1 and the other is _param_constant0

Large graph:

        # call_function  addmm_default      aten.addmm.default  (_param_constant1, ph_0, _tensor_constant0)  {}

Pattern graph

        # call_function  addmm_default      aten.addmm.default  (_param_constant0, ph_0, _tensor_constant0)  {}

Differential Revision: D42316574

The pattern can't be matched if one attribute is `_param_constant1` and the other is `_param_constant0` Large graph: ``` # call_function addmm_default aten.addmm.default (_param_constant1, ph_0, _tensor_constant0) {} ``` Pattern graph ``` # call_function addmm_default aten.addmm.default (_param_constant0, ph_0, _tensor_constant0) {} ``` Differential Revision: [D42316574](https://our.internmc.facebook.com/intern/diff/D42316574/) [ghstack-poisoned]

pytorch-bot · 2023-01-03T20:51:05Z

🔗 Helpful Links

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

📄 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.

❌ 1 Failures

As of commit b6f5731:

BROKEN TRUNK - The following jobs failed but were present on the merge base 6674583:

linux-focal-rocm5.3-py3.8 / test (default, 2, 2, linux.rocm.gpu)

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

The pattern can't be matched if one attribute is `_param_constant1` and the other is `_param_constant0` Large graph: ``` # call_function addmm_default aten.addmm.default (_param_constant1, ph_0, _tensor_constant0) {} ``` Pattern graph ``` # call_function addmm_default aten.addmm.default (_param_constant0, ph_0, _tensor_constant0) {} ``` Differential Revision: [D42316574](https://our.internmc.facebook.com/intern/diff/D42316574/) ghstack-source-id: 177038217 Pull Request resolved: #91657

The pattern can't be matched if one attribute is `_param_constant1` and the other is `_param_constant0` Large graph: ``` # call_function addmm_default aten.addmm.default (_param_constant1, ph_0, _tensor_constant0) {} ``` Pattern graph ``` # call_function addmm_default aten.addmm.default (_param_constant0, ph_0, _tensor_constant0) {} ``` Differential Revision: [D42316574](https://our.internmc.facebook.com/intern/diff/D42316574/) [ghstack-poisoned]

Pull Request resolved: #91657 The pattern can't be matched if one attribute is `_param_constant1` and the other is `_param_constant0` Large graph: ``` # call_function addmm_default aten.addmm.default (_param_constant1, ph_0, _tensor_constant0) {} ``` Pattern graph ``` # call_function addmm_default aten.addmm.default (_param_constant0, ph_0, _tensor_constant0) {} ``` ghstack-source-id: 177158321 Differential Revision: [D42316574](https://our.internmc.facebook.com/intern/diff/D42316574/)

cccclai · 2023-01-05T02:11:51Z

torch/fx/passes/utils/matcher_utils.py

+            elif pn.op == "get_attr":
+                pn_value = getattr(pn.graph.owning_module, pn.target)
+                gn_value = getattr(gn.graph.owning_module, gn.target)
+                return isinstance(pn_value, torch.Tensor) and isinstance (gn_value, torch.Tensor)


I feel like we don't necessarily need to compare tensor data itself, torch.linear(10, 14) and torch.linear(12, 14) will have different weight and they should be matched, thought?

Also regarding

legal object

anything else I need to check here?

The pattern can't be matched if one attribute is `_param_constant1` and the other is `_param_constant0` Large graph: ``` # call_function addmm_default aten.addmm.default (_param_constant1, ph_0, _tensor_constant0) {} ``` Pattern graph ``` # call_function addmm_default aten.addmm.default (_param_constant0, ph_0, _tensor_constant0) {} ``` Differential Revision: [D42316574](https://our.internmc.facebook.com/intern/diff/D42316574/) [ghstack-poisoned]

Pull Request resolved: #91657 The pattern can't be matched if one attribute is `_param_constant1` and the other is `_param_constant0` Large graph: ``` # call_function addmm_default aten.addmm.default (_param_constant1, ph_0, _tensor_constant0) {} ``` Pattern graph ``` # call_function addmm_default aten.addmm.default (_param_constant0, ph_0, _tensor_constant0) {} ``` ghstack-source-id: 177201189 Differential Revision: [D42316574](https://our.internmc.facebook.com/intern/diff/D42316574/)

SherlockNoMad · 2023-01-05T22:15:24Z

torch/fx/passes/utils/matcher_utils.py

+        gn_value = getattr(gn.graph.owning_module, gn.target)
+        if type(pn_value) != type(gn_value):
+            return False
+        if isinstance(pn_value, torch.Tensor):


Please comment on we don't require exact match on tensor values.

SherlockNoMad · 2023-01-05T22:16:48Z

torch/fx/passes/utils/matcher_utils.py

+            return False
+        if isinstance(pn_value, torch.Tensor):
+            return isinstance (gn_value, torch.Tensor)
+        else:


We should be able handle python literals, int/float/bool ect...

Make sense. For python literals, i'm thinking they should be pass if the type is the same maybe?

SherlockNoMad · 2023-01-05T22:17:36Z

Can we add some test?

SherlockNoMad

Per comment and lint passing.

The pattern can't be matched if one attribute is `_param_constant1` and the other is `_param_constant0` Large graph: ``` # call_function addmm_default aten.addmm.default (_param_constant1, ph_0, _tensor_constant0) {} ``` Pattern graph ``` # call_function addmm_default aten.addmm.default (_param_constant0, ph_0, _tensor_constant0) {} ``` Differential Revision: [D42316574](https://our.internmc.facebook.com/intern/diff/D42316574/) [ghstack-poisoned]

Pull Request resolved: #91657 The pattern can't be matched if one attribute is `_param_constant1` and the other is `_param_constant0` Large graph: ``` # call_function addmm_default aten.addmm.default (_param_constant1, ph_0, _tensor_constant0) {} ``` Pattern graph ``` # call_function addmm_default aten.addmm.default (_param_constant0, ph_0, _tensor_constant0) {} ``` ghstack-source-id: 177318763 Differential Revision: [D42316574](https://our.internmc.facebook.com/intern/diff/D42316574/) **NOTE FOR REVIEWERS**: This PR has internal Meta-specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D42316574/)!

The pattern can't be matched if one attribute is `_param_constant1` and the other is `_param_constant0` Large graph: ``` # call_function addmm_default aten.addmm.default (_param_constant1, ph_0, _tensor_constant0) {} ``` Pattern graph ``` # call_function addmm_default aten.addmm.default (_param_constant0, ph_0, _tensor_constant0) {} ``` Differential Revision: [D42316574](https://our.internmc.facebook.com/intern/diff/D42316574/) [ghstack-poisoned]

Pull Request resolved: #91657 The pattern can't be matched if one attribute is `_param_constant1` and the other is `_param_constant0` Large graph: ``` # call_function addmm_default aten.addmm.default (_param_constant1, ph_0, _tensor_constant0) {} ``` Pattern graph ``` # call_function addmm_default aten.addmm.default (_param_constant0, ph_0, _tensor_constant0) {} ``` ghstack-source-id: 177318804 Differential Revision: [D42316574](https://our.internmc.facebook.com/intern/diff/D42316574/) **NOTE FOR REVIEWERS**: This PR has internal Meta-specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D42316574/)!

The pattern can't be matched if one attribute is `_param_constant1` and the other is `_param_constant0` Large graph: ``` # call_function addmm_default aten.addmm.default (_param_constant1, ph_0, _tensor_constant0) {} ``` Pattern graph ``` # call_function addmm_default aten.addmm.default (_param_constant0, ph_0, _tensor_constant0) {} ``` Differential Revision: [D42316574](https://our.internmc.facebook.com/intern/diff/D42316574/) [ghstack-poisoned]

Pull Request resolved: #91657 The pattern can't be matched if one attribute is `_param_constant1` and the other is `_param_constant0` Large graph: ``` # call_function addmm_default aten.addmm.default (_param_constant1, ph_0, _tensor_constant0) {} ``` Pattern graph ``` # call_function addmm_default aten.addmm.default (_param_constant0, ph_0, _tensor_constant0) {} ``` ghstack-source-id: 177319081 Differential Revision: [D42316574](https://our.internmc.facebook.com/intern/diff/D42316574/) **NOTE FOR REVIEWERS**: This PR has internal Meta-specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D42316574/)!

facebook-github-bot · 2023-01-09T08:02:55Z

@pytorchbot merge

(Initiating merge automatically since Phabricator Diff has merged)

pytorchmergebot · 2023-01-09T08:10:51Z

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

The pattern can't be matched if one attribute is `_param_constant1` and the other is `_param_constant0` Large graph: ``` # call_function addmm_default aten.addmm.default (_param_constant1, ph_0, _tensor_constant0) {} ``` Pattern graph ``` # call_function addmm_default aten.addmm.default (_param_constant0, ph_0, _tensor_constant0) {} ``` Differential Revision: [D42316574](https://our.internmc.facebook.com/intern/diff/D42316574/) Pull Request resolved: #91657 Approved by: https://github.com/SherlockNoMad ghstack-source-id: b39629bdacb7b02788061579c23585253b456157

pytorch-bot bot added the release notes: fx release notes category label Jan 3, 2023

cccclai requested review from SherlockNoMad, mergennachin and angelayi January 5, 2023 01:59

cccclai commented Jan 5, 2023

View reviewed changes

SherlockNoMad reviewed Jan 5, 2023

View reviewed changes

SherlockNoMad approved these changes Jan 5, 2023

View reviewed changes

pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Jan 9, 2023

pytorchmergebot added the Merged label Jan 9, 2023

pytorchmergebot closed this in fd213c3 Jan 9, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Match get_attr when compare node #91657

Match get_attr when compare node #91657

cccclai commented Jan 3, 2023 •

edited

pytorch-bot bot commented Jan 3, 2023 •

edited

cccclai Jan 5, 2023 •

edited

SherlockNoMad Jan 5, 2023

SherlockNoMad Jan 5, 2023

cccclai Jan 5, 2023

SherlockNoMad commented Jan 5, 2023

SherlockNoMad left a comment

facebook-github-bot commented Jan 9, 2023

pytorchmergebot commented Jan 9, 2023

Match get_attr when compare node #91657

Match get_attr when compare node #91657

Conversation

cccclai commented Jan 3, 2023 • edited

pytorch-bot bot commented Jan 3, 2023 • edited

🔗 Helpful Links

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

❌ 1 Failures

cccclai Jan 5, 2023 • edited

Choose a reason for hiding this comment

SherlockNoMad Jan 5, 2023

Choose a reason for hiding this comment

SherlockNoMad Jan 5, 2023

Choose a reason for hiding this comment

cccclai Jan 5, 2023

Choose a reason for hiding this comment

SherlockNoMad commented Jan 5, 2023

SherlockNoMad left a comment

Choose a reason for hiding this comment

facebook-github-bot commented Jan 9, 2023

pytorchmergebot commented Jan 9, 2023

Merge started

cccclai commented Jan 3, 2023 •

edited

pytorch-bot bot commented Jan 3, 2023 •

edited

cccclai Jan 5, 2023 •

edited