ns for fx: add support for subgraph matching #52130

vkuzo · 2021-02-11T05:19:09Z

Stack from ghstack:

ns for fx: update graph matching to not match nodes with equal types #52402 ns for fx: update graph matching to not match nodes with equal types
ns for fx: support linear_relu for weight matching #52395 ns for fx: support linear_relu for weight matching
ns for fx: allow graph matching of parents of cat #52368 ns for fx: allow graph matching of parents of cat
ns for fx: make unshadowed activation comparison work for N models #52357 ns for fx: make unshadowed activation comparison work for N models
ns for fx: make weights comparison work on N models #52356 ns for fx: make weights comparison work on N models
ns for fx: add support for subgraph matching #52130 ns for fx: add support for subgraph matching
NS for FX: add test for a simple sparsenn model #52092 NS for FX: add test for a simple sparsenn model

Summary:

We have patterns like (F.linear, F.relu) which need to match
to (toq.linear_relu). So, we need to match subgraphs.

This PR does the following:

defines a "subgraph" as (start_node, end_node). The current assumption
is that subgraphs are simple, there is always a path from start_node to
end_node, and we can ignore any non-input args/kwargs of these nodes
for the purposes of matching and copying things. An example one node
subgraph is (F.linear, F.linear). An example two node subgraph
is (F.linear, F.relu).
changes the matching logic to iterate over subgraphs instead of nodes
changes the NS core APIs to use subgraph pairs instead of node pairs:

for weights, we match on the start node
for unshadowed activations, we observe the end nodes
for shadowed activations, we copy the subgraph of a to graph c

TODO(before review) write up better, not ready for review yet

Test Plan:

python test/test_quantization.py TestFXNumericSuiteCoreAPIs

Reviewers:

Subscribers:

Tasks:

Tags:

Differential Revision: D26403092

Summary: We have patterns like (F.linear, F.relu) which need to match to (toq.linear_relu). So, we need to match subgraphs. This PR does the following: * defines a "subgraph" as (start_node, end_node). The current assumption is that subgraphs are simple, there is always a path from start_node to end_node, and we can ignore any non-input args/kwargs of these nodes for the purposes of matching and copying things. An example one node subgraph is (F.linear, F.linear). An example two node subgraph is (F.linear, F.relu). * changes the matching logic to iterate over subgraphs instead of nodes * changes the NS core APIs to use subgraph pairs instead of node pairs: 1. for weights, we match on the start node 2. for unshadowed activations, we observe the end nodes 3. for shadowed activations, we copy the subgraph of a to graph c TODO(before review) write up better, not ready for review yet Test Plan: TODO before land: better test plan Reviewers: Subscribers: Tasks: Tags: [ghstack-poisoned]

Summary: We have patterns like (F.linear, F.relu) which need to match to (toq.linear_relu). So, we need to match subgraphs. This PR does the following: * defines a "subgraph" as (start_node, end_node). The current assumption is that subgraphs are simple, there is always a path from start_node to end_node, and we can ignore any non-input args/kwargs of these nodes for the purposes of matching and copying things. An example one node subgraph is (F.linear, F.linear). An example two node subgraph is (F.linear, F.relu). * changes the matching logic to iterate over subgraphs instead of nodes * changes the NS core APIs to use subgraph pairs instead of node pairs: 1. for weights, we match on the start node 2. for unshadowed activations, we observe the end nodes 3. for shadowed activations, we copy the subgraph of a to graph c TODO(before review) write up better, not ready for review yet Test Plan: TODO before land: better test plan Reviewers: Subscribers: Tasks: Tags: ghstack-source-id: 862b004 Pull Request resolved: #52130

facebook-github-bot · 2021-02-11T16:35:28Z

💊 CI failures summary and remediations

As of commit e705ae6 (more details on the Dr. CI page):

1/1 failures possibly* introduced in this PR
- 1/1 non-CircleCI failure(s)

This comment was automatically generated by Dr. CI (expand for details).

Follow this link to opt-out of these comments for your Pull Requests.

Please report bugs/suggestions to the (internal) Dr. CI Users group.

Summary: We have patterns like (F.linear, F.relu) which need to match to (toq.linear_relu). So, we need to match subgraphs. This PR does the following: * defines a "subgraph" as (start_node, end_node). The current assumption is that subgraphs are simple, there is always a path from start_node to end_node, and we can ignore any non-input args/kwargs of these nodes for the purposes of matching and copying things. An example one node subgraph is (F.linear, F.linear). An example two node subgraph is (F.linear, F.relu). * changes the matching logic to iterate over subgraphs instead of nodes * changes the NS core APIs to use subgraph pairs instead of node pairs: 1. for weights, we match on the start node 2. for unshadowed activations, we observe the end nodes 3. for shadowed activations, we copy the subgraph of a to graph c TODO(before review) write up better, not ready for review yet Test Plan: TODO before land: better test plan Reviewers: Subscribers: Tasks: Tags: Differential Revision: [D26403092](https://our.internmc.facebook.com/intern/diff/D26403092) [ghstack-poisoned]

Summary: We have patterns like (F.linear, F.relu) which need to match to (toq.linear_relu). So, we need to match subgraphs. This PR does the following: * defines a "subgraph" as (start_node, end_node). The current assumption is that subgraphs are simple, there is always a path from start_node to end_node, and we can ignore any non-input args/kwargs of these nodes for the purposes of matching and copying things. An example one node subgraph is (F.linear, F.linear). An example two node subgraph is (F.linear, F.relu). * changes the matching logic to iterate over subgraphs instead of nodes * changes the NS core APIs to use subgraph pairs instead of node pairs: 1. for weights, we match on the start node 2. for unshadowed activations, we observe the end nodes 3. for shadowed activations, we copy the subgraph of a to graph c TODO(before review) write up better, not ready for review yet Test Plan: TODO before land: better test plan Reviewers: Subscribers: Tasks: Tags: ghstack-source-id: fb8df70 Pull Request resolved: #52130

Summary: We have patterns like (F.linear, F.relu) which need to match to (toq.linear_relu). So, we need to match subgraphs. This PR does the following: * defines a "subgraph" as (start_node, end_node). The current assumption is that subgraphs are simple, there is always a path from start_node to end_node, and we can ignore any non-input args/kwargs of these nodes for the purposes of matching and copying things. An example one node subgraph is (F.linear, F.linear). An example two node subgraph is (F.linear, F.relu). * changes the matching logic to iterate over subgraphs instead of nodes * changes the NS core APIs to use subgraph pairs instead of node pairs: 1. for weights, we match on the start node 2. for unshadowed activations, we observe the end nodes 3. for shadowed activations, we copy the subgraph of a to graph c TODO(before review) write up better, not ready for review yet Test Plan: TODO before land: better test plan Reviewers: Subscribers: Tasks: Tags: Differential Revision: [D26403092](https://our.internmc.facebook.com/intern/diff/D26403092) [ghstack-poisoned]

Summary: We have patterns like (F.linear, F.relu) which need to match to (toq.linear_relu). So, we need to match subgraphs. This PR does the following: * defines a "subgraph" as (start_node, end_node). The current assumption is that subgraphs are simple, there is always a path from start_node to end_node, and we can ignore any non-input args/kwargs of these nodes for the purposes of matching and copying things. An example one node subgraph is (F.linear, F.linear). An example two node subgraph is (F.linear, F.relu). * changes the matching logic to iterate over subgraphs instead of nodes * changes the NS core APIs to use subgraph pairs instead of node pairs: 1. for weights, we match on the start node 2. for unshadowed activations, we observe the end nodes 3. for shadowed activations, we copy the subgraph of a to graph c TODO(before review) write up better, not ready for review yet Test Plan: TODO before land: better test plan Reviewers: Subscribers: Tasks: Tags: ghstack-source-id: a66f4ce Pull Request resolved: #52130

raghuramank100 · 2021-02-13T02:34:42Z

torch/quantization/ns/graph_matcher.py

+        (F.relu, F.linear),
+    ])
+
+# TODO(future PR): we should see if we can reuse quantization's fusion


Yes, this is a good idea. In general, would the entire graph_matcher logic be shared with fx quantization passes?

that would be a good long term state. I'd imagine most of the benefits to come from sharing the fusion patterns, the matching logic itself may not generalize.

raghuramank100 · 2021-02-13T02:36:15Z

torch/quantization/ns/graph_matcher.py

+            # if we match linear-relu backwards, we will always skip the
+            # relu node and attempt to match the linear node.  This can
+            # be made configurable later if needed.
+            for _reverse_fusion_ops in get_reversed_fusions():


Do we always pick the largest fusion here?

Ideally the ordering in the default would be followed, with the ability for user to customize. It doesn't handle that yet. We'd have to add that logic as soon as we have more than one fusion.

raghuramank100 · 2021-02-13T02:37:39Z

torch/quantization/ns/graph_matcher.py

    # for now, use node name.
    # TODO(future PR): find a better solution
-    return node_b.name
+    return end_node_b.name


Should this be start_node_b.name+end_node_b.name so that it is clear that it is a sub-graph and not a single node?

Sounds good to me. In general naming of nodes is something we should talk through in detail for all of these APIs, saving that for a future PR. Thankfully changing that later should be low eng cost, so it doesn't have to be perfect now.

raghuramank100 · 2021-02-13T02:40:47Z

torch/quantization/ns/graph_matcher.py

    and continuing backwards.
-    1. Returns matchable nodes, in order
-    2. Skips over non-matchable nodes
+    1. Returns matchable subgraphs, in order. A subgraph is defined by


Sorry, dont follow this logic: Where is the sub-graph specified in the init? I dont see the end_node being defined.

this is in the __next__ function. We pop a potential end node off the stack. If the end node is matchable, we try to peek backwards to check for a potential fusion pattern. We return (end_node, end_node) if there is no fusion pattern, or (start_node, end_node) if there is a fusion pattern ending at end_node.

Summary: We have patterns like (F.linear, F.relu) which need to match to (toq.linear_relu). So, we need to match subgraphs. This PR does the following: * defines a "subgraph" as (start_node, end_node). The current assumption is that subgraphs are simple, there is always a path from start_node to end_node, and we can ignore any non-input args/kwargs of these nodes for the purposes of matching and copying things. An example one node subgraph is (F.linear, F.linear). An example two node subgraph is (F.linear, F.relu). * changes the matching logic to iterate over subgraphs instead of nodes * changes the NS core APIs to use subgraph pairs instead of node pairs: 1. for weights, we match on the start node 2. for unshadowed activations, we observe the end nodes 3. for shadowed activations, we copy the subgraph of a to graph c TODO(before review) write up better, not ready for review yet Test Plan: TODO before land: better test plan Reviewers: Subscribers: Tasks: Tags: Differential Revision: [D26403092](https://our.internmc.facebook.com/intern/diff/D26403092) [ghstack-poisoned]

Summary: We have patterns like (F.linear, F.relu) which need to match to (toq.linear_relu). So, we need to match subgraphs. This PR does the following: * defines a "subgraph" as (start_node, end_node). The current assumption is that subgraphs are simple, there is always a path from start_node to end_node, and we can ignore any non-input args/kwargs of these nodes for the purposes of matching and copying things. An example one node subgraph is (F.linear, F.linear). An example two node subgraph is (F.linear, F.relu). * changes the matching logic to iterate over subgraphs instead of nodes * changes the NS core APIs to use subgraph pairs instead of node pairs: 1. for weights, we match on the start node 2. for unshadowed activations, we observe the end nodes 3. for shadowed activations, we copy the subgraph of a to graph c TODO(before review) write up better, not ready for review yet Test Plan: TODO before land: better test plan Reviewers: Subscribers: Tasks: Tags: ghstack-source-id: 79add57 Pull Request resolved: #52130

Summary: We have patterns like (F.linear, F.relu) which need to match to (toq.linear_relu). So, we need to match subgraphs. This PR does the following: * defines a "subgraph" as (start_node, end_node). The current assumption is that subgraphs are simple, there is always a path from start_node to end_node, and we can ignore any non-input args/kwargs of these nodes for the purposes of matching and copying things. An example one node subgraph is (F.linear, F.linear). An example two node subgraph is (F.linear, F.relu). * changes the matching logic to iterate over subgraphs instead of nodes * changes the NS core APIs to use subgraph pairs instead of node pairs: 1. for weights, we match on the start node 2. for unshadowed activations, we observe the end nodes 3. for shadowed activations, we copy the subgraph of a to graph c TODO(before review) write up better, not ready for review yet Test Plan: TODO before land: better test plan Reviewers: Subscribers: Tasks: Tags: Differential Revision: [D26403092](https://our.internmc.facebook.com/intern/diff/D26403092) [ghstack-poisoned]

jerryzh168 · 2021-02-17T20:19:47Z

torch/quantization/ns/graph_matcher.py

    ])

-class _NSGraphMatchableNodesIterator:
+def get_reversed_fusions() -> Set[Tuple[Callable, Callable]]:


is this only going to support matching 2 nodes?

the main restriction is that the subgraph needs one start point and one end point. Support for a length of 3 nodes can be added here with low eng cost. This type would become Union[Tuple[Callable, Callable], Tuple[Callable, Callable, Callable]].

note: currently this is not a list because lists are not hashable. In general, I'm flexible on the exact data structure, happy to revise it if there is a better solution.

hx89 · 2021-02-17T20:13:14Z

torch/quantization/ns/graph_matcher.py

                continue
-            self.seen_nodes.add(cur_node)
+
+            # for subgraphs which are single nodes, start_node == end_node


A n00b question, how do we distinguish the subgraph of single nodes and the subgraph with two identical nodes? For example if start_node and end_node are both linear, the subgraph could be single node or it could be a subgraph with two linear nodes. Or do we assume that subgraph nodes are different, which is true for the subgraphs we want to match such as (linear, relu) ?

sure, the node here is the actual node object (not node type or name, etc). So, two distinct F.linear nodes can be used as a start point and an endpoint. For example,

linear1 -> linear2 -> linear3 # a subgraph of just linear1 (linear1, linear1) # a subgraph of 1..3 (linear1, linear3)

hx89 · 2021-02-17T20:21:39Z

torch/quantization/ns/graph_matcher.py

-    if node_b.op == 'call_module':
-        assert isinstance(node_b.target, str)
-        return node_b.target
+    if end_node_b.op == 'call_module':


Not related to this PR, just wonder why need special handling for call_module and can't get node name directly?

sounds reasonable to me, happy to update that in a separate PR. In general this part is pretty self-contained, so we can make updates to this easily.

hx89 · 2021-02-17T21:04:46Z

torch/quantization/ns/graph_matcher.py

+            # note: relatedness is checked on the start node, i.e.
+            # if a linear-relu pattern is checked, we would check for relatedness
+            # of the linear
+            if not _node_a_related_to_b(cur_start_node_a, cur_start_node_b,


Since subgraph matching is introduced to graph matcher, would it be natural to extend the relatedness checking to support subgraph also, which might be more general?

Sure, that could make sense. Is there a use case in mind?

Since this is a private API, the eng cost of changing this in the future would be low.

Yeah we can think about it in later PR. I don't have specific use case in mind, just thinking if we can relate for example the subgraph (linear, relu) to linear_relu directly, then it would be self-explanatory and we may not need to note "relatedness is checked on the start node".

raghuramank100 · 2021-02-17T23:10:15Z

torch/quantization/ns/graph_passes.py

-    nodes_to_instrument_b_to_a = {}
-    for match_name, (node_a, node_b) in matched_node_pairs.items():
-        nodes_to_instrument_b_to_a[node_b] = node_a
+    node_b_to_matched_subgraph_a = {}


It is quite useful to insert shadow at multiple levels. Assuming in a later PR we will support shadowing a sub-graph instead of a single node.

sounds reasonable

raghuramank100 · 2021-02-17T23:12:04Z

torch/quantization/ns/graph_passes.py

            # subgraph so far:
            #
-            #       dtype_cast_node --> node_a_copy
+            #       dtype_cast_node --> subgraph_a_copy


Should the cast be at the input of node_c?

correct. The code does this, but the comment is wrong, thanks for catching. Will update before landing.

Summary: We have patterns like (F.linear, F.relu) which need to match to (toq.linear_relu). So, we need to match subgraphs. This PR does the following: * defines a "subgraph" as (start_node, end_node). The current assumption is that subgraphs are simple, there is always a path from start_node to end_node, and we can ignore any non-input args/kwargs of these nodes for the purposes of matching and copying things. An example one node subgraph is (F.linear, F.linear). An example two node subgraph is (F.linear, F.relu). * changes the matching logic to iterate over subgraphs instead of nodes * changes the NS core APIs to use subgraph pairs instead of node pairs: 1. for weights, we match on the start node 2. for unshadowed activations, we observe the end nodes 3. for shadowed activations, we copy the subgraph of a to graph c TODO(before review) write up better, not ready for review yet Test Plan: ``` python test/test_quantization.py TestFXNumericSuiteCoreAPIs ``` Reviewers: Subscribers: Tasks: Tags: Differential Revision: [D26403092](https://our.internmc.facebook.com/intern/diff/D26403092) [ghstack-poisoned]

codecov · 2021-02-18T07:54:58Z

Codecov Report

Merging #52130 (e705ae6) into gh/vkuzo/225/base (16f0cdf) will increase coverage by 0.00%.
The diff coverage is 99.14%.

@@                Coverage Diff                 @@
##           gh/vkuzo/225/base   #52130   +/-   ##
==================================================
  Coverage              80.33%   80.34%           
==================================================
  Files                   1967     1967           
  Lines                 215670   215722   +52     
==================================================
+ Hits                  173262   173313   +51     
- Misses                 42408    42409    +1

facebook-github-bot · 2021-02-18T16:20:46Z

This pull request has been merged in d903106.

Summary: Pull Request resolved: pytorch#52130 We have patterns like (F.linear, F.relu) which need to match to (toq.linear_relu). So, we need to match subgraphs. This PR does the following: * defines a "subgraph" as (start_node, end_node). The current assumption is that subgraphs are simple, there is always a path from start_node to end_node, and we can ignore any non-input args/kwargs of these nodes for the purposes of matching and copying things. An example one node subgraph is (F.linear, F.linear). An example two node subgraph is (F.linear, F.relu). * changes the matching logic to iterate over subgraphs instead of nodes * changes the NS core APIs to use subgraph pairs instead of node pairs: 1. for weights, we match on the start node 2. for unshadowed activations, we observe the end nodes 3. for shadowed activations, we copy the subgraph of a to graph c TODO(before review) write up better, not ready for review yet Test Plan: TODO before land: better test plan Imported from OSS Reviewed By: raghuramank100 Differential Revision: D26403092 fbshipit-source-id: e49aaad4b02b8d60589435848bee422b8f41937a

This was referenced Feb 11, 2021

Early version of fx graph matcher for NS #51588

Closed

ns for fx - stubs of the three APIs (compare weights, activations, activations with shadow) #51669

Closed

NS for FX: add test for a simple sparsenn model #52092

Closed

facebook-github-bot added the cla signed label Feb 11, 2021

raghuramank100 reviewed Feb 13, 2021

View reviewed changes

vkuzo mentioned this pull request Feb 16, 2021

reland - ns for fx - stubs of the three APIs (compare weights, activations, activations with shadow) #52302

Closed

This was referenced Feb 17, 2021

ns for fx: make weights comparison work on N models #52356

Closed

ns for fx: make unshadowed activation comparison work for N models #52357

Closed

vkuzo mentioned this pull request Feb 17, 2021

ns for fx: allow graph matching of parents of cat #52368

Closed

jerryzh168 reviewed Feb 17, 2021

View reviewed changes

hx89 reviewed Feb 17, 2021

View reviewed changes

vkuzo changed the title ~~[wip] ns for fx: add support for subgraph matching~~ ns for fx: add support for subgraph matching Feb 17, 2021

vkuzo mentioned this pull request Feb 17, 2021

ns for fx: support linear_relu for weight matching #52395

Closed

raghuramank100 reviewed Feb 17, 2021

View reviewed changes

raghuramank100 approved these changes Feb 17, 2021

View reviewed changes

vkuzo mentioned this pull request Feb 17, 2021

ns for fx: update graph matching to not match nodes with equal types #52402

Closed

vkuzo added 2 commits February 17, 2021 16:24

facebook-github-bot closed this in d903106 Feb 18, 2021

facebook-github-bot added the Merged label Feb 18, 2021

facebook-github-bot deleted the gh/vkuzo/225/head branch February 22, 2021 15:17

ns for fx: add support for subgraph matching #52130

ns for fx: add support for subgraph matching #52130

Uh oh!

Conversation

vkuzo commented Feb 11, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

facebook-github-bot commented Feb 11, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

💊 CI failures summary and remediations

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Feb 18, 2021

Codecov Report

Uh oh!

facebook-github-bot commented Feb 18, 2021

Uh oh!

Uh oh!

vkuzo commented Feb 11, 2021 •

edited

Loading

facebook-github-bot commented Feb 11, 2021 •

edited

Loading