[RELAY][AUTOTVM] Extract tuning tasks from Relay programs #2181

eqy · 2018-11-27T23:46:50Z

This is a copy-paste of nnvm task extraction for relay for autotvm tuning.

eqy · 2018-11-28T18:52:48Z

If someone has pointers on the current way to do relay inference, we can also add relay versions of the tune_nnvm_* scripts.

tqchen · 2018-11-28T18:56:31Z

see https://github.com/dmlc/tvm/blob/master/tests/python/frontend/mxnet/test_forward.py

eqy · 2018-11-28T20:46:48Z

I am also porting the test_autotvm_task_extraction test, but it looks like we also need to the clear the cache for relay. Is that function currently exposed? I do see a clear cache function registered.

tqchen · 2018-11-28T21:03:00Z

https://github.com/dmlc/tvm/blob/master/python/tvm/relay/backend/compile_engine.py#L107
https://github.com/dmlc/tvm/blob/master/python/tvm/relay/backend/compile_engine.py#L144

merrymercy · 2018-11-28T21:30:30Z

python/tvm/autotvm/task/relay_integration.py

+
+logger = logging.getLogger('autotvm')
+
+def serialize_args(args):


We can remove this by adding custom json encoder and decoder in https://github.com/dmlc/tvm/blob/master/python/tvm/autotvm/record.py#L54

merrymercy · 2018-11-28T21:34:21Z

python/tvm/autotvm/task/relay_integration.py

+        # NOTE: To add more ops, you only need to change the following lists
+        # relay op -> topi compute
+        self.op2topi = {
+            relay.op.nn.conv2d: [topi.nn.conv2d, topi.nn.depthwise_conv2d_nchw,


This op2topi is the only different part between TaskExtractEnv for nnvm and relay.
I think we can move TaskExtractEnv to topi_integration.py, and let nnvm and relay part pass in this argument.

The signatures are also a little different because nnvm and relay require different parameters (e.g., relay gets param dict, nnvm requires shape). How should we handle this?

I am referring to the class TaskExtractEnv, which is almost TOPI level.

Ok, good call, we can merge these.

One thing to notice is that we pass params to relay.build, which means we will do constant folding and call tvm functions.
Then the python multiprocessing package will become invalid and we cannot tune with multiprocessing. Do you notice this in your tuning? A possible workaround is to run relay.build in a separate thread.

The CI breaks now after merging in the test when nnvm is imported. Is this a circular import?

Is the issue that the CI script does not add NNVM to the pythonpath for relay tests?
https://github.com/dmlc/tvm/blob/master/tests/scripts/task_python_integration.sh

I think so. Because this is a tvm test. This's why I put the original test to nnvm folder.

Ok, the reason I put it there was because it was a "relay/autotvm" test which doesn't really have anything to do with nnvm. I can move it to the nnvm directory.

No, we should not move it to nnvm. See my comment below

merrymercy · 2018-11-30T01:45:22Z

python/tvm/autotvm/task/nnvm_integration.py

@@ -54,6 +54,7 @@ class TaskExtractEnv:
    def __init__(self):


We should do better decoupling here, as we will drop nnvm in the future. This can also solve the import problem.

Move TaskExtractEnv to topi_integration.py,
Move all import nnvm to nnvm_integration.py,
Move all import relay to relay_integration.py.

test nnvm_integration.py in nnvm/test
test relay_integration.py in tvm/test

ok, I will try to refactor for that

Actually, is there any reason we use relay/nnvm functions to specify which operators to match? Could we just use the topi functions directly or some syntax sugar otherwise?

Currently this seems to introduce some circular import problems because if we do import relay at the top level (as relay does import autotvm). We can hack this by moving the imports to within functions, but I am not sure that is ideal.

Because one operators conv can correspond to many topi functions.

We have to move import nnvm, import relay inside functions as the old code does.

jroesch · 2018-12-01T08:28:24Z

Overall this looks good to me, one question is there a reason we need to fully build the Relay function with the tracing target? isn't the set of operators just a statically inferable property?

Can we not extract the type information and call sites from the program and use that as our tuning work load?

eqy · 2018-12-02T09:07:14Z

I'm not sure about the details here, but is TOPI a blackbox from the perspective of Relay? It seems we would still need to go through TOPI's dispatch system even if we had complete information for Relay's side. But overall this is just a case of "don't fix it if ain't broke" as this implementation is basically borrowed from the NNVM version.

merrymercy · 2018-12-02T09:27:23Z

We want to keep the autotvm tuning tasks at TOPI level. Using tracing mode can make the extraction irrelevant to graph ir. So one version of code works for both nnvm and relay.

Besides, the dispatching mechanism from relay op to topi is complicated. For example, one relay op can correspond to many topi functions. (e.g., a conv op may call vanila conv, group conv, or depthwise conv in topi). On vta, one op can be run on either CPU or FPGA.

eqy · 2018-12-04T00:34:06Z

Not sure what the build error is here; do I need to do a rebase or just retrigger CI?

merrymercy · 2018-12-11T00:58:51Z

Looks good to me. Maybe I will do a follow up refactor for VTA.

merrymercy · 2018-12-11T01:00:27Z

Sorry, can you add some test cases?

yzhliu · 2018-12-11T06:51:49Z

python/tvm/autotvm/task/nnvm_integration.py

+                          topi.nn.group_conv2d_nchw],
+        nnvm.sym.conv2d_transpose: [topi.nn.conv2d_transpose_nchw],
+        nnvm.sym.dense: [topi.nn.dense],
+    }


can we combine these two SYMBOL2TOPI ?

Perhaps; @merrymercy should we do this? There have also been discuss board questions from people who didn't know to add conv2d_transpose to the tutorial example when tuning other networks.

@yzhliu is referring to combine L52 and L123.
This is not related to the problem in the forum.

The reason we don't combine them currently seems to be because the imports here are tricky to do outside of a function. Should we just delete the extract_from_multiple_graphs function? I do not see it used often.

I am using it. Maybe we can create another function for these lists

yzhliu · 2018-12-11T06:52:52Z

python/tvm/autotvm/task/relay_integration.py

+                                 topi.nn.group_conv2d_nchw],
+        tvm.relay.op.nn.conv2d_transpose: [topi.nn.conv2d_transpose_nchw],
+        tvm.relay.op.nn.dense: [topi.nn.dense],
+    }


so as these two OP2TOPI

tqchen · 2018-12-14T21:33:47Z

@yzhliu please take another look at the pr

tqchen · 2018-12-24T19:28:46Z

Thanks, @yzhliu @eqy @merrymercy this is merged.

eqy added 4 commits November 27, 2018 15:44

check in copy paste

94a0525

cleanup

53b877f

fix lint

ee79db7

cleanup

28e9341

tqchen assigned merrymercy Nov 28, 2018

tqchen added the status: need review label Nov 28, 2018

port nnvm tests

0ba3142

merrymercy requested changes Nov 28, 2018

View reviewed changes

eqy added 3 commits November 28, 2018 14:07

wrap build call in thread

5c162af

merge TaskExtractEnv

bba2b51

fix lint

a3774e3

merrymercy reviewed Nov 30, 2018

View reviewed changes

eqy added 2 commits November 29, 2018 17:46

try moving test to nnvm directory

022960f

refactor, fix imports

08590f8

fix lint

e902f58

eqy added 2 commits December 10, 2018 09:37

trigger CI

ddc0a5a

fix import

2248862

merrymercy approved these changes Dec 11, 2018

View reviewed changes

check in test

083cc12

yzhliu reviewed Dec 11, 2018

View reviewed changes

tqchen merged commit a006258 into apache:master Dec 24, 2018

tqchen added status: accepted and removed status: need review labels Dec 24, 2018

FrozenGene pushed a commit to FrozenGene/tvm that referenced this pull request Dec 27, 2018

[RELAY][AUTOTVM] Extract tuning tasks from Relay programs (apache#2181)

7d4ea4d

wweic pushed a commit to neo-ai/tvm that referenced this pull request Feb 20, 2019

[RELAY][AUTOTVM] Extract tuning tasks from Relay programs (apache#2181)

47919b7

wweic pushed a commit to neo-ai/tvm that referenced this pull request Feb 20, 2019

[RELAY][AUTOTVM] Extract tuning tasks from Relay programs (apache#2181)

497bc3d

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[RELAY][AUTOTVM] Extract tuning tasks from Relay programs #2181

[RELAY][AUTOTVM] Extract tuning tasks from Relay programs #2181

eqy commented Nov 27, 2018

eqy commented Nov 28, 2018 •

edited

Loading

tqchen commented Nov 28, 2018

eqy commented Nov 28, 2018

tqchen commented Nov 28, 2018 •

edited

Loading

merrymercy Nov 28, 2018

merrymercy Nov 28, 2018

eqy Nov 28, 2018

merrymercy Nov 28, 2018

eqy Nov 28, 2018

merrymercy Nov 28, 2018 •

edited

Loading

eqy Nov 28, 2018

eqy Nov 29, 2018

merrymercy Nov 30, 2018 •

edited

Loading

eqy Nov 30, 2018

merrymercy Nov 30, 2018

merrymercy Nov 30, 2018 •

edited

Loading

eqy Nov 30, 2018

eqy Nov 30, 2018

merrymercy Nov 30, 2018 •

edited

Loading

jroesch commented Dec 1, 2018

eqy commented Dec 2, 2018

merrymercy commented Dec 2, 2018

eqy commented Dec 4, 2018

merrymercy commented Dec 11, 2018 •

edited

Loading

merrymercy commented Dec 11, 2018 •

edited

Loading

yzhliu Dec 11, 2018

eqy Dec 11, 2018

merrymercy Dec 14, 2018 •

edited

Loading

eqy Dec 14, 2018

merrymercy Dec 14, 2018

yzhliu Dec 11, 2018

tqchen commented Dec 14, 2018

tqchen commented Dec 24, 2018


		logger = logging.getLogger('autotvm')

		def serialize_args(args):

[RELAY][AUTOTVM] Extract tuning tasks from Relay programs #2181

[RELAY][AUTOTVM] Extract tuning tasks from Relay programs #2181

Conversation

eqy commented Nov 27, 2018

eqy commented Nov 28, 2018 • edited Loading

tqchen commented Nov 28, 2018

eqy commented Nov 28, 2018

tqchen commented Nov 28, 2018 • edited Loading

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

merrymercy Nov 28, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

merrymercy Nov 30, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

merrymercy Nov 30, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

merrymercy Nov 30, 2018 • edited Loading

Choose a reason for hiding this comment

jroesch commented Dec 1, 2018

eqy commented Dec 2, 2018

merrymercy commented Dec 2, 2018

eqy commented Dec 4, 2018

merrymercy commented Dec 11, 2018 • edited Loading

merrymercy commented Dec 11, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

merrymercy Dec 14, 2018 • edited Loading

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

tqchen commented Dec 14, 2018

tqchen commented Dec 24, 2018

eqy commented Nov 28, 2018 •

edited

Loading

tqchen commented Nov 28, 2018 •

edited

Loading

merrymercy Nov 28, 2018 •

edited

Loading

merrymercy Nov 30, 2018 •

edited

Loading

merrymercy Nov 30, 2018 •

edited

Loading

merrymercy Nov 30, 2018 •

edited

Loading

merrymercy commented Dec 11, 2018 •

edited

Loading

merrymercy commented Dec 11, 2018 •

edited

Loading

merrymercy Dec 14, 2018 •

edited

Loading