[RELAY][ONNX] Support Deriving channels when it is not provided in AlterLayout. #2972

cbalint13 · 2019-04-05T17:07:08Z

Dear All,

I propose this patch that address issue #2941 and all related to subject of altering conv2d layout.

Patch was tested with THIS on: arm, mali, x86 and cuda.
It is based on already implemented infer_channels().

tqchen · 2019-04-05T22:27:48Z

Thanks for the contribution, please add a regression test case as per https://docs.tvm.ai/contribute/code_review.html#ensure-test-coverage

cbalint13 · 2019-04-09T00:20:00Z

@tqchen & All,

Added testcases, with some comments:

The test case goes through all known alter_conv2d variants by iterating on target contexts and try to count the mull-accumulate amount after the possible optimization.
Currently on intel-cpu and intel-gpu the mac_count() returns zero (i think it's a bug, will try fix it in other PR) so the extra assert check is skipped for these cases.
The optimize will fail if one particular alter_conv2d is doing something odd so asserts on mac-accumulate counts are just bonus double check.

topi/python/topi/arm_cpu/conv2d.py

cbalint13 · 2019-04-10T14:32:27Z

@merrymercy , @tqchen ,

Regarding testcase:

Initial thought was to count MAC, but realized tvm can count only for nn.conv2d
For now I added an assertion that graph should be different, MAC comparasion is a ToDo

topi/python/topi/cuda/conv2d_winograd.py

topi/python/topi/intel_graphics/conv2d.py

topi/python/topi/x86/conv2d.py

merrymercy · 2019-04-17T09:10:01Z

LGTM. Only some nits. You can batch my suggestions and commit them.

cbalint13 · 2019-04-17T10:05:40Z

LGTM. Only some nits. You can batch my suggestions and commit them.

It is done, committed the suggestion comment even in +one more place, now all are covered.

merrymercy · 2019-04-17T14:20:58Z

@cbalint13 @tqchen Thanks. It is merged!

apivovarov · 2019-04-17T23:06:03Z

topi/python/topi/x86/conv2d.py

@@ -327,11 +327,16 @@ def _alter_conv2d_layout(attrs, inputs, tinfo, F):

    copy_inputs = [s for s in inputs]
    new_attrs = {k : attrs[k] for k in attrs.keys()}
+
+    if F == tvm.relay.op:


@merrymercy @cbalint13 This call fails if user uses nnvm.compiler.build. You need to import relay from tvm first. It fails only for nnvm.compiler.build, not for relay.build

Compile using nnvm... Traceback (most recent call last): File "./compile.py", line 62, in <module> graph, lib, params = nnvm.compiler.build(sym, target, shape={"data": data_shape}, dtype=dtype, params=params, target_host=target_host) File "/usr/local/lib/python3.6/dist-packages/nnvm-0.8.0-py3.6.egg/nnvm/compiler/build_module.py", line 297, in build graph = optimize(graph, shape, dtype, layout) File "/usr/local/lib/python3.6/dist-packages/nnvm-0.8.0-py3.6.egg/nnvm/compiler/build_module.py", line 186, in optimize graph = graph.apply(["InferShape", "InferType", "AlterOpLayout"]) File "/usr/local/lib/python3.6/dist-packages/nnvm-0.8.0-py3.6.egg/nnvm/graph.py", line 250, in apply check_call(_LIB.NNGraphApplyPasses(self.handle, npass, cpass, ctypes.byref(ghandle))) File "/usr/local/lib/python3.6/dist-packages/nnvm-0.8.0-py3.6.egg/nnvm/_base.py", line 91, in check_call raise NNVMError(py_str(_LIB.NNGetLastError())) nnvm._base.NNVMError: AttributeError: module 'tvm' has no attribute 'relay' Stack trace: if F == tvm.relay.op: File "/usr/local/lib/python3.6/dist-packages/topi-0.6.dev0-py3.6.egg/topi/x86/conv2d.py", line 331, in _alter_conv2d_layout return dispatch_dict[k](*args, **kwargs) File "/usr/local/lib/python3.6/dist-packages/tvm-0.6.dev0-py3.6-linux-x86_64.egg/tvm/target.py", line 372, in dispatch_func File "</usr/local/lib/python3.6/dist-packages/decorator.py:decorator-gen-18>", line 2, in conv2d_alter_layout return topi.nn.conv2d_alter_layout(attrs, inputs, tinfos, sym) File "/usr/local/lib/python3.6/dist-packages/nnvm-0.8.0-py3.6.egg/nnvm/top/nn.py", line 194, in alter_conv2d_layout File "tvm/_ffi/_cython/./function.pxi", line 55, in tvm._ffi._cy3.core.tvm_callback Stack trace: [bt] (0) /usr/local/lib/python3.6/dist-packages/tvm-0.6.dev0-py3.6-linux-x86_64.egg/tvm/libtvm.so(+0x8e7f8b) [0x7f2ecdc9af8b] [bt] (1) /usr/local/lib/python3.6/dist-packages/nnvm-0.8.0-py3.6.egg/nnvm/libnnvm_compiler.so(+0x12ab65) [0x7f2ec8404b65] [bt] (2) /usr/local/lib/python3.6/dist-packages/nnvm-0.8.0-py3.6.egg/nnvm/libnnvm_compiler.so(+0xe5623) [0x7f2ec83bf623] [bt] (3) /usr/local/lib/python3.6/dist-packages/nnvm-0.8.0-py3.6.egg/nnvm/libnnvm_compiler.so(+0xe7c93) [0x7f2ec83c1c93] [bt] (4) /usr/local/lib/python3.6/dist-packages/nnvm-0.8.0-py3.6.egg/nnvm/libnnvm_compiler.so(+0xea0c9) [0x7f2ec83c40c9] [bt] (5) /usr/local/lib/python3.6/dist-packages/nnvm-0.8.0-py3.6.egg/nnvm/libnnvm_compiler.so(+0xc0800) [0x7f2ec839a800] [bt] (6) /usr/local/lib/python3.6/dist-packages/nnvm-0.8.0-py3.6.egg/nnvm/libnnvm_compiler.so(+0x85a11) [0x7f2ec835fa11] [bt] (7) /usr/local/lib/python3.6/dist-packages/nnvm-0.8.0-py3.6.egg/nnvm/libnnvm_compiler.so(NNGraphApplyPasses+0x320) [0x7f2ec833df00] [bt] (8) /usr/lib/x86_64-linux-gnu/libffi.so.6(ffi_call_unix64+0x4c) [0x7f2eee480dae]

@apivovarov ,

Could make it in a new PR ?

If we had nnvm compilation tests then your PR would not pass CI tests and you needed to fix your code.

@apivovarov ,

Agree.

Will add as a new PR today, with testcase using nnvm symbols instead of relay.op

I'll keep you on Cc

@cbalint13 I'm not sure we want to import relay if user decided to use nnvm.compiler.build.
Can we use string comparison here? e.g.

if F.__name__ == 'tvm.relay.op':

continue discussion in
#3044

…ache#2972)

tqchen added the status: need test case need test cases to cover the change label Apr 5, 2019

tqchen added the status: need review label Apr 9, 2019

merrymercy reviewed Apr 10, 2019

View reviewed changes

topi/python/topi/arm_cpu/conv2d.py Outdated Show resolved Hide resolved

tqchen assigned merrymercy Apr 16, 2019

merrymercy approved these changes Apr 17, 2019

View reviewed changes

merrymercy requested changes Apr 17, 2019

View reviewed changes

topi/python/topi/cuda/conv2d_winograd.py Show resolved Hide resolved

topi/python/topi/intel_graphics/conv2d.py Show resolved Hide resolved

topi/python/topi/x86/conv2d.py Show resolved Hide resolved

merrymercy removed the status: need test case need test cases to cover the change label Apr 17, 2019

merrymercy removed the status: need review label Apr 17, 2019

Support Deriving channels when it is not provided in AlterLayout.

0a5c992

merrymercy approved these changes Apr 17, 2019

View reviewed changes

merrymercy merged commit c91f714 into apache:master Apr 17, 2019

apivovarov reviewed Apr 17, 2019

View reviewed changes

cbalint13 mentioned this pull request Apr 18, 2019

Additional fix for PR#2972 (case of missing import) #3044

Merged

larroy pushed a commit to larroy/tvm that referenced this pull request Apr 19, 2019

Support Deriving channels when it is not provided in AlterLayout. (ap…

8e9c0e0

…ache#2972)

merrymercy mentioned this pull request Apr 19, 2019

[RELAY][ONNX] Support Deriving channels when it is not provided in AlterLayout #2941

Closed

wweic pushed a commit to wweic/tvm that referenced this pull request May 13, 2019

Support Deriving channels when it is not provided in AlterLayout. (ap…

0f66c3c

…ache#2972)

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

Support Deriving channels when it is not provided in AlterLayout. (ap…

c809095

…ache#2972)

tqchen mentioned this pull request Nov 8, 2019

[RELEASE][DRAFT] TVM v0.6 Release candidate #4259

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[RELAY][ONNX] Support Deriving channels when it is not provided in AlterLayout. #2972

[RELAY][ONNX] Support Deriving channels when it is not provided in AlterLayout. #2972

cbalint13 commented Apr 5, 2019

tqchen commented Apr 5, 2019

cbalint13 commented Apr 9, 2019

cbalint13 commented Apr 10, 2019

merrymercy commented Apr 17, 2019 •

edited

Loading

cbalint13 commented Apr 17, 2019

merrymercy commented Apr 17, 2019

apivovarov Apr 17, 2019 •

edited

Loading

cbalint13 Apr 18, 2019

apivovarov Apr 18, 2019

cbalint13 Apr 18, 2019

apivovarov Apr 18, 2019

apivovarov Apr 18, 2019

[RELAY][ONNX] Support Deriving channels when it is not provided in AlterLayout. #2972

[RELAY][ONNX] Support Deriving channels when it is not provided in AlterLayout. #2972

Conversation

cbalint13 commented Apr 5, 2019

tqchen commented Apr 5, 2019

cbalint13 commented Apr 9, 2019

cbalint13 commented Apr 10, 2019

merrymercy commented Apr 17, 2019 • edited Loading

cbalint13 commented Apr 17, 2019

merrymercy commented Apr 17, 2019

apivovarov Apr 17, 2019 • edited Loading

Choose a reason for hiding this comment

cbalint13 Apr 18, 2019

Choose a reason for hiding this comment

apivovarov Apr 18, 2019

Choose a reason for hiding this comment

cbalint13 Apr 18, 2019

Choose a reason for hiding this comment

apivovarov Apr 18, 2019

Choose a reason for hiding this comment

apivovarov Apr 18, 2019

Choose a reason for hiding this comment

merrymercy commented Apr 17, 2019 •

edited

Loading

apivovarov Apr 17, 2019 •

edited

Loading