Added pattern matching and new optimization passes #53

antonionevado · 2018-11-29T03:01:24Z

This PR simplify the current optimization mechanism.
The idea is to only have a collection of simple passes that perform some actions over every sub-graph that match some given pattern.

The benefits of this PR are:

Improve speed: previous implementation compilation time was around 80 seconds. The new implementation takes around 1~2 seconds. Thanks to Joel-san and Neil-san for finding a way of computing thresholds in constant time!
The code is very simple and easy to understand: several functions each one applying a different pass.
The code is also very short compared to the previous implementation.

ruimashita

How effective this PR when we add new operator ?

antonionevado · 2018-12-18T02:28:07Z

Adding a new operator require change much less code.
The main advantage is that the old code was very difficult to understand anyway. And very slow.

ruimashita · 2018-12-18T04:49:32Z

@antonionevado How to test this?

dlk/python/dlk/core/graph_pattern_matching.py

ruimashita · 2018-12-18T04:56:44Z

dlk/python/dlk/core/operators.py

@@ -56,6 +55,8 @@ def __init__(self,
        self._check_consistency()
        self._rank = len(shape)
        self._available_buffer = ''
+        self._visited = False


Does it need?

No, will delete.

ruimashita · 2018-12-18T04:57:06Z

dlk/python/dlk/core/operators.py

@@ -56,6 +55,8 @@ def __init__(self,
        self._check_consistency()
        self._rank = len(shape)
        self._available_buffer = ''
+        self._visited = False
+        self._prop_details = Dict


Also, Does it need ?

Probably not, old code I forgot to delete. I will delete it. Thank you!

dlk/python/dlk/core/optimizer.py

ruimashita · 2018-12-18T08:00:03Z

dlk/tests/test_optimizer.py

 from core.operators import Add, AveragePool, BatchNormalization, Constant, Conv, Identity, Input, \
    MaxPool, Operator, Output, Transpose, QTZ_binary_mean_scaling, QTZ_linear_mid_tread_half, Reshape, Softmax

 import numpy as np
-from typing import Any, Dict, List, Tuple
+from typing import Tuple


 class TestOptimizer(unittest.TestCase):


I didn't understand what is verified in this test. Is it not unit test ?
I think need to add unit tests for each passes, and some public graph matcher functions and classes.

@antonionevado @nlpng How is this?

ruimashita

It is better, The passes of changing node property like pass_compute_thresholds(), pass_propagate_quantization_details_into_conv() have document of which property change to, I think.

BTW, what is different of run() and run_forward() in Operator class ?

ruimashita · 2019-01-07T07:03:32Z

dlk/python/dlk/core/graph_pattern_matching.py

+    """
+    def __init__(self, op=str(), inputs=list()):
+        self.op = op
+        self.inputs = inputs


What case self.inputs is used ? It seems not be used.

It is necessary to support more complex patterns. For example, imagine you want to write a pass that need to process sub-graphs where you have a quantizer with an input which is a batch normalization. And that batch normalization node has an input which is a convolution node. Then you can specify the pattern:

p = Pattern('QTZ_linear_mid_tread_half', [ Pattern('BatchNormalization', [ Pattern('Conv'), Pattern('*'), Pattern('*'), Pattern('*'), Pattern('*') ]), Pattern('*'), Pattern('*'), ])

Note that the pattern * means that you don't care about that node, could be any type of operator.

Which pass is used the pattern?
I think it is no need to implement this, If the pattern is not used.
Or, should implement unit test and write document at least for future work.

ruimashita · 2019-01-07T07:07:37Z

dlk/python/dlk/core/graph_pattern_matching.py

+
+    Returns
+    -------
+    result : [NodeMatch]


Why this is a list of NodeMatch object, not list of Operator ?
If we can use list of Operator, Maybe we don't need NodeMatch class.

It is necessary to have this class because the return type is a list of sub-graphs. The NodeMatch class is a graph type (resursive) data structure.

NodeMatch is not subclass of Graph, but is it sub graph? it looks strange implementation.
Operator objects have inputs and outputs, that means you can retrieve path and nodes, it is already graph type structure.
why it is necessary?

dlk/python/dlk/templates/Makefile.tpl

dlk/python/dlk/scripts/generate_project.py

dlk/python/dlk/core/operators.py

ruimashita · 2019-01-07T09:42:18Z

dlk/python/dlk/core/operators.py

+        mean = np.float64(self._input_ops['mean'].data)
+        var = np.float64(self._input_ops['var'].data)
+
+        kwargs['nega_idx'] = [v for v in range(len(scale)) if scale[v] < 0]


nega_idx is not relate invert function of batch norm, it is related only for threshold skip. Why the nega_idx calculation is not move into pass of threshold skipping?

Since it was more straightforward to retrieve the values from the batch norm rather than the threshold skipping pass, but I agree this will go to the optimization pass.

Now the nega_idx is moved inside the optimization pass with a7f73bc

why the line remain ?

ruimashita · 2019-01-07T09:48:13Z

dlk/python/dlk/core/optimizer.py

+    p = Pattern('*')
+    matches = find_pattern(graph, p)
+
+    def find_input(node, otype):


The purpose of the find_input function is set dtype.
The find_input function name is confuse for me.

Ok, maybe rename it to dtype_changer or output_type_changer

dlk/python/dlk/core/optimizer.py

ruimashita · 2019-01-07T10:05:25Z

dlk/python/dlk/core/optimizer.py

+
+        if p[-1].op_type != 'Conv':
+            continue
+        quantizer_conv_output_node = p[0]


Is quantizer_conv_output_node activation quantization node ? confuse...

I think @nlpng already changed the name. Yes, sorry, it is a bad name.

ruimashita · 2019-01-09T18:13:31Z

dlk/python/dlk/core/graph_pattern_matching.py

+    """
+    def __init__(self, op=str(), inputs=list()):
+        self.op = op
+        self.inputs = inputs


Which pass is used the pattern?
I think it is no need to implement this, If the pattern is not used.
Or, should implement unit test and write document at least for future work.

ruimashita · 2019-01-10T01:58:56Z

dlk/python/dlk/core/graph_pattern_matching.py

+
+    Returns
+    -------
+    result : [NodeMatch]


NodeMatch is not subclass of Graph, but is it sub graph? it looks strange implementation.
Operator objects have inputs and outputs, that means you can retrieve path and nodes, it is already graph type structure.
why it is necessary?

ruimashita · 2019-01-10T01:59:43Z

dlk/python/dlk/core/operators.py

+        mean = np.float64(self._input_ops['mean'].data)
+        var = np.float64(self._input_ops['var'].data)
+
+        kwargs['nega_idx'] = [v for v in range(len(scale)) if scale[v] < 0]


why the line remain ?

dlk/python/dlk/core/optimizer.py

ruimashita · 2019-01-10T02:08:11Z

dlk/python/dlk/core/optimizer.py

+        n = 2 ** nbit - 1
+        ch = conv_node.channel
+        # assume that the threshold values will be a 13-bit signed integer
+        max_th_value = 2 ** 12 - 1


if we can change, why 12 is not variable?

ruimashita · 2019-01-10T12:10:56Z

dlk/python/dlk/core/optimizer.py

+            # get nodes to be removed after being disconnected
+            get_nodes_in_branch(m.node, None, to_be_removed)
+
+            new_constant.add_outputs({'output': m.node.output_ops.values()})


Why these several lines are in here? looks general manupulation of graph and op.
How is to implement like replace_op() in Graph class and use graph.replace_op(m.node, new_constant) ?

ruimashita · 2019-01-10T12:13:57Z

dlk/tests/test_optimizer.py

 from core.operators import Add, AveragePool, BatchNormalization, Constant, Conv, Identity, Input, \
    MaxPool, Operator, Output, Transpose, QTZ_binary_mean_scaling, QTZ_linear_mid_tread_half, Reshape, Softmax

 import numpy as np
-from typing import Any, Dict, List, Tuple
+from typing import Tuple


 class TestOptimizer(unittest.TestCase):


@antonionevado @nlpng How is this?

ruimashita · 2019-01-10T12:14:49Z

dlk/python/dlk/core/optimizer.py

+            continue
+
+        quantizer_conv_weights = conv_node.quantizer
+        quantizer_conv_weights.run_forward_no_scaling_factor()


@antonionevado @nlpng How is this ?

dlk/python/dlk/core/optimizer.py

dlk/python/dlk/core/operators.py

… out old code

…ptimizer code

ruimashita · 2019-01-25T13:39:26Z

@antonionevado @nlpng Thanks, I will review this PR next week 👍

ruimashita

@antonionevado @nlpng
Thanks, so so nice refactoring.
I wrote some nite things. Plz see it.

dlk/python/dlk/core/graph_pattern_matching.py

dlk/python/dlk/core/optimizer.py

dlk/tests/test_optimizer.py

ruimashita · 2019-01-28T12:16:53Z

dlk/python/dlk/core/optimizer.py

+            continue
+
+        quantizer_conv_weights = conv_node.quantizer
+        quantizer_conv_weights.run_forward_no_scaling_factor()


I think run_forward_no_scaling_factor() should be in scaling_factor getter.
But it's not need to change.

dlk/tests/test_optimizer.py

ruimashita

@antonionevado @nlpng
Thanks a lot 😄 😄 😄
we can get very simple and easy to understand code, and improve graph optimize speed 🥇

Added pattern matching and new optimization passes

antonionevado added the enhancement New feature or request label Nov 29, 2018

antonionevado self-assigned this Nov 29, 2018

antonionevado force-pushed the simplify_optimizer branch from a16c303 to b4ca6f7 Compare December 11, 2018 07:44

antonionevado changed the title ~~[WIP] Added pattern matching and new optimization passes~~ Added pattern matching and new optimization passes Dec 14, 2018

antonionevado requested review from leapmindadmin, ruimashita and nlpng and removed request for leapmindadmin December 17, 2018 00:54

ruimashita reviewed Dec 18, 2018

View reviewed changes

dlk/python/dlk/core/graph_pattern_matching.py Outdated Show resolved Hide resolved

ruimashita reviewed Dec 18, 2018

View reviewed changes

dlk/python/dlk/core/optimizer.py Outdated Show resolved Hide resolved

ruimashita reviewed Dec 18, 2018

View reviewed changes

ruimashita reviewed Jan 7, 2019

View reviewed changes

nlpng force-pushed the simplify_optimizer branch from 5bdce90 to 46207f1 Compare January 8, 2019 08:01

ruimashita suggested changes Jan 10, 2019

View reviewed changes

antonionevado and others added 11 commits January 11, 2019 16:19

[WIP] Added pattern matching and new optimization passes

ab184de

Fix: not necessary, axis is attribute in DLK IR

7376a12

Added a pass for computing thresholds based on binary search

8a269d5

[WIP] Visualization, weight packing and small fixes. Start to comment…

ebe9d3a

… out old code

[WIP] Add extra passes and small bugfixes. Now doesnt depend on old o…

180d156

…ptimizer code

Added base code so Neil-san can work on threshold skipping

ba900c5

changes to the threshold skipping pass

58cbe84

Temperately fix for super large threshold value over 16bit int

299fe78

Applying conditions for hq and ts

7052906

Move the axis input of split operator to attribute

a669ca5

Propagate output data type to last quantized convolution

15b9135

nlpng and others added 8 commits January 11, 2019 16:19

remove redundant method and flags

9f01c68

Implement preserve_quantization in each operator

14cf8cc

Move nega_idx aggregation into optimization pass

74da1fd

Move while loop inside of pass_precompute

8417067

Rename pass_precompute to pass_constant_folding

a0d4a33

rename find_input to output_dtype_changer

2a20dba

remove some redundant lines

fb51eee

Simplify the passes code

8dd88be

antonionevado force-pushed the simplify_optimizer branch from f9f099f to 8dd88be Compare January 11, 2019 07:24

nlpng and others added 8 commits January 16, 2019 19:31

Add new tests for optimization passes

406178d

just add more tests for more optimizatino pass

a2c51a1

Add more more tests for optimization pass

aa5bb3d

Add last test for constant folding pass

d3b496e

Add functions comments for run and de_run

ebc4679

Remove redundant codes and comments

9b5c66b

just move things around

4b006f9

Merge branch 'master' into simplify_optimizer

f515e15

ruimashita suggested changes Jan 28, 2019

View reviewed changes

antonionevado and others added 6 commits January 29, 2019 10:17

Merge branch 'master' into simplify_optimizer

8554ebc

add new assertion check for quantize convolution pass

ca65f4e

add test for graph has no kernel quantizer

39b8678

add test for graph has no weights

b5e67a8

fix the copyright year

82a72c7

take care the negative values

d216594

ruimashita approved these changes Jan 31, 2019

View reviewed changes

Merge branch 'master' into simplify_optimizer

3f4fb40

ruimashita merged commit fe7b69f into blue-oil:master Jan 31, 2019

iizukak added this to the v0.3 milestone Feb 6, 2019

ruimashita added a commit that referenced this pull request Jun 26, 2019

Merge pull request #53 from antonionevado/simplify_optimizer

fd849d7

Added pattern matching and new optimization passes

Added pattern matching and new optimization passes #53

Added pattern matching and new optimization passes #53

Conversation

antonionevado commented Nov 29, 2018 • edited Loading

ruimashita left a comment

Choose a reason for hiding this comment

antonionevado commented Dec 18, 2018

ruimashita commented Dec 18, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ruimashita left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ruimashita commented Jan 25, 2019

ruimashita left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ruimashita left a comment

Choose a reason for hiding this comment

antonionevado commented Nov 29, 2018 •

edited

Loading