[HYBRID FRONTEND] Modify hybrid script to new interface; hybrid op supported; enable compilation_database in CMakeList.txt by were · Pull Request #1757 · apache/tvm

were · 2018-09-21T22:49:12Z

Move old hybrid script frontend

import tvm
@tvm.hybrid.script
def outer_product(a, b, c):
    for j in range(128):
        for i in range(128):
            c[i,j] = a[i] * b[j]

a = tvm.placeholder((128, ), 'float32', 'a')
b = tvm.placeholder((128, ), 'float32', 'b')
c = tvm.placeholder((128, ), 'float32', 'b')
# returns c's op
# if it is multi-output, return the tuple of output ops
op = outer_product(a, b, c)

to

import tvm
@tvm.hybrid.script
def outer_product(a, b):
    c = tvm.output((128, 128), dtype)
    for j in range(128):
        for i in range(128):
            c[i,j] = a[i] * b[j]
    return c

a = tvm.placeholder((128, ), 'float32', 'a')
b = tvm.placeholder((128, ), 'float32', 'b')
# returns an ExternOp
c = outer_product(a, b)

tqchen · 2018-09-22T17:13:21Z

Ideally, we should keep the old function parse as it is. And support a__call__ of the hybrid script that calls parse and then generate an ExternOp.

import tvm
@tvm.hybrid.script
def outer_product(a, b):
    c = tvm.output((128, 128), dtype)
    for j in range(128):
        for i in range(128):
            c[i,j] = a[i] * b[j]
    return c

a = tvm.placeholder((128, ), 'float32', 'a')
b = tvm.placeholder((128, ), 'float32', 'b')
# returns an ExternOp
c = outer_product(a, b) 
d = tvm.compute(c.shape, lambda *i: c(*i)+1)
sch = tvm.create_schedule(d.op)
...

were · 2018-09-22T19:00:45Z

OK. Then I know how an op works.

were · 2018-09-23T03:26:11Z

@tqchen but if we explicitly define the outputs, how can we do multi-output op, and how can we support in-place algorithms?

tqchen · 2018-09-23T04:27:39Z

Multi output case

import tvm
@tvm.hybrid.script
def outer_product(a, b):
    c = tvm.output((128, 128), dtype)
    d = tvm.output((128, 128), dtype)

    for j in range(128):
        for i in range(128):
            c[i,j] = a[i] * b[j]
            d[i,j] = a[i] + 1
    return c, d

a = tvm.placeholder((128, ), 'float32', 'a')
b = tvm.placeholder((128, ), 'float32', 'b')
# returns an ExternOp
c0, c1 = outer_product(a, b) 
d = tvm.compute(c.shape, lambda *i: c0(*i)+c1(*i))
sch = tvm.create_schedule(d.op)
.

Inplace Update

I agree that is harder to express an inplace update. The general design goal of the operator relation is that everything is immutable and there is no inplace update. We can still assign c and a to be the same buffer to achieve the effect of of inplace update though.

tqchen · 2018-09-23T04:29:23Z

The main reason I think the above approach is nice is that the code itself is pretty consistent with tvm.compute flow and can be mixed together with other expressions. The only drawback of inplace update can be resolved by supporting inplace update hybrid code but do not support these for extern op for now

were · 2018-09-23T08:00:51Z

I like the old one better for 2 reasons:

Calling a hybrid script becomes more like calling a TVM built module, which makes code clean when doing sw-emulation verification
In-place algorithm support will be rollback, if we use the new style
Why don't we do this?

import tvm
@tvm.hybrid.script
def outer_product(a, b, c):
    for j in range(128):
        for i in range(128):
            c[i,j] = a[i] * b[j]

a = tvm.placeholder((128, ), 'float32', 'a')
b = tvm.placeholder((128, ), 'float32', 'b')
c = tvm.placeholder((128, ), 'float32', 'b')
# returns c's op
# if it is multi-output, return the tuple of output ops
op = outer_product(a, b, c)
d = tvm.compute(c.shape, lambda *i: c(*i)+1)
sch = tvm.create_schedule(d.op)

In general, it is an extension of the current hybrid module.

tqchen · 2018-10-13T16:17:06Z

@were can you update this PR and add a few examples?

were · 2018-10-14T22:18:23Z

@tqchen @kevinthesun
I am facing a problem like this:

op = _ExternOp(input_placeholders, input_buffers, output_buffers, ..., body)
ret = [op.output(i) for i in range(len(output_buffers))]
return ret[0] if len(ret) == 1 else ret

Originally, what I am supposed to do is to register an op and return its output tensors, which seems to be OK, but the problem I am facing is that all the output tensors' names are same as the op's name.

Just as it is mentioned above, return c, d gives different names to the output tensors.

Also, if I want to give a different name to the tensor, how can I specify the relationship between the tensor and op?

tqchen · 2018-10-14T22:53:43Z

The output tensor's name can be viewed as op_name.value0, opname.value1 which have different meaning. Note that under the hood tvm uses pointer uniqueness, so even if the name are the same it is fine

were · 2018-10-15T00:16:39Z

OK. I know know what you mean --- no matter how users name the output tensors, we will finally convert all the tensors to name foo.v0, foo.v1, and etc.

Then a new problem comes when doing in-place. (or more accurately, it is not a problem. I just want to make sure it is acceptable):

out = foo(a)
sch = tvm.create_schedule(a.op)
tvm.build(sch, [out])

Say, foo is an in-place function call which transforms a to out. They are essentially the same buffer with alias, but I CANNOT use a when compilation. I need to force users to pass out, or the compiler cannot find the buffer in the binds.

tqchen · 2018-10-15T00:34:16Z

For now, we could avoid supporting inplace functions, if we want to create an inplace op, we can do

out = foo(a)
sch = tvm.create_schedule(a.op)
buffer = tvm.decl_buffer(shape=out.shape)
tvm.build(sch, [buffer], binds={a:buffer, out:buffer})

were · 2018-10-15T00:48:58Z

I am refusing not to support in-place because it is some kind of "rollback" --- it is supported before but disabled just because the "glue" to the framework.

tqchen · 2018-10-15T01:10:49Z

The example I gave kinda of support inplace operations

were · 2018-10-15T01:14:44Z

For now, I think it is good enough.
My question is that we have already have buffer binds stored in Op's attributes, so can we do it spontaneously?

tqchen · 2018-10-15T01:36:04Z

It sounds good to me, as long as the behavior is well documented

were · 2018-11-15T22:57:01Z

@kevinthesun @xqdan @tqchen Can you guys take a look so that I can get it in?

xqdan · 2018-11-16T03:47:46Z

Can we update this place? Since we have an unified routine after this patch.
https://github.com/dmlc/tvm/blob/master/python/tvm/build_module.py#L343

xqdan · 2018-11-16T04:01:00Z

        self.func_name = func_name # The name of the function to be lowered
-        self.iter_axis = []
+        self.outputs = [] # Output tensors' name
+        self.side_effect = set() # Tensors with side effects


Just a question, what's side_effect for?

Later for in-place operation.

xqdan · 2018-11-16T04:10:39Z

I believe TVM has no global (inter-op) check which leverages the input/output information based on side-effect free assumption.

If 2.'s bold assumption is not true or not acceptable for further extension of this project, I will pick the second solution in 1.

I think when we need to support inline/fuse between ComputeOp and HybirdOp, the data dependence between the Ops is important.

were · 2018-11-16T04:31:15Z

@xqdan Actually, current hybrid op is the same as the external op, which cannot be splitted. Thus, I think no inline will be done?

xqdan · 2018-11-16T04:14:31Z

+  }
+  auto n = make_node<HybridOpNode>(*this);
+  ret = op::ReplaceTensor(ret, rmap);
+  ret = op::ReplaceProvideTensor(ret, rmap);


Could you put some comments to explain the logic of this function?

It is a long story. I hope you can get what I am saying inside my updated comments.

Just curious, "In hybrid script all the tensors have their own names specified by the users." Does this mean that user can write something like "out = my_out_tensor((1, 3, 224, 224), 'float')"? In this case, if out is generated by, say flatten op with name 'tensor', out has different name with flatten op. This is why we need replace tensors, and in output IR, the out tensor name won't be "my_out_tensor" any more?

Yes. Actually, you can dump both ret in cpp and op.body in python to see this.

Got it. Maybe we can document this behavior somewhere, such as tvm.hybrid.script, make it transparent to user.

xqdan · 2018-11-16T06:13:40Z

+    ret = AttrStmt::make(
+        bind_spec, attr::buffer_bind_scope,
+        Call::make(Handle(), intrinsic::tvm_tuple, tuple, Call::Intrinsic), ret);
+  };


please put comments about why we need binding here

this is just copy paste from extern_op.cc

xqdan · 2018-11-16T06:15:57Z

+  Stmt ret = repl.Mutate(stmt);
+  return repl.found ? ret : stmt;
+}
+Expr ReplaceProvideTensor(Expr expr,


Only for Stmt is enough

Jesus, let me fix it!

xqdan · 2018-11-16T06:24:34Z

@xqdan Actually, current hybrid op is the same as the external op, which cannot be splitted. Thus, I think no inline will be done?

The difference is that, with hybird op, we can have the whole ir context, so it's possible to do some schedule, but external op is just a black box.

were · 2018-11-16T06:51:36Z

@xqdan That's why I opened a new op node. Later more features will be supported and more attributes will be added.

kevinthesun · 2018-11-17T23:11:10Z

Update tutorial as well?

xqdan

LGTM now

tqchen · 2018-11-19T06:52:33Z

Thanks @xqdan @kevinthesun for reviews, Thanks @were this is merged!

junrushao

LGTM

…pported; enable compilation_database in CMakeList.txt (apache#1757)

tqchen self-assigned this Sep 22, 2018

tqchen added the status: need review label Sep 22, 2018

tqchen added status: need update need update based on feedbacks and removed status: need review labels Sep 29, 2018

were changed the title ~~[HYBRID FRONTEND] Support hybrid op.~~ [HYBRID FRONTEND] Modify hybrid script to new interface. Oct 3, 2018

were force-pushed the hybrid-op branch from 1547048 to f88a346 Compare October 3, 2018 23:08

were force-pushed the hybrid-op branch from d702f91 to 712fe59 Compare November 11, 2018 06:56

Jian Weng added 8 commits November 10, 2018 22:56

move callee parser to explicit call parser

0754fd1

a hacky hybridop is done

8f65112

fix commit

ae5135a

on the way to refactor

cd6547a

fix typo

eab1bd6

fix outer prod first

764abc2

get rid of var_buffer

5c34dbe

simpliy raise ValueError to _internal_assert

30491a3

fix lint

042034b

were changed the title ~~[HYBRID FRONTEND] Modify hybrid script to new interface.~~ [HYBRID FRONTEND] Modify hybrid script to new interface; hybrid op supported; enable compilation_database in CMakeList.txt Nov 15, 2018

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

xqdan reviewed Nov 16, 2018

View reviewed changes

get rid of stmt build legacy

909ef13

xqdan reviewed Nov 16, 2018

View reviewed changes

remove unnecessary Expr overload. add some comments

e586b1e

update langref

c9a90e8

kevinthesun approved these changes Nov 19, 2018

View reviewed changes

xqdan approved these changes Nov 19, 2018

View reviewed changes

document the subtle body problem in hybridop header comments

3b71c01

tqchen approved these changes Nov 19, 2018

View reviewed changes

tqchen merged commit 7a01476 into apache:master Nov 19, 2018

tqchen added status: accepted and removed status: need review status: need update need update based on feedbacks labels Nov 19, 2018

junrushao reviewed Nov 19, 2018

View reviewed changes

tqchen mentioned this pull request Nov 19, 2018

[TOPI] Migrate Vision Kernel from IRBuilder to Hybrid Script #1322

Closed

2 tasks

were deleted the hybrid-op branch December 14, 2018 04:00

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

[HYBRID FRONTEND] Modify hybrid script to new interface; hybrid op su…

49bd3b0

…pported; enable compilation_database in CMakeList.txt (apache#1757)

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

[HYBRID FRONTEND] Modify hybrid script to new interface; hybrid op su…

324a21d

…pported; enable compilation_database in CMakeList.txt (apache#1757)

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

[HYBRID FRONTEND] Modify hybrid script to new interface; hybrid op su…

c8fc120

…pported; enable compilation_database in CMakeList.txt (apache#1757)

Conversation

were commented Sep 21, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tqchen commented Sep 22, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

were commented Sep 22, 2018

Uh oh!

were commented Sep 23, 2018

Uh oh!

tqchen commented Sep 23, 2018

Multi output case

Inplace Update

Uh oh!

tqchen commented Sep 23, 2018

Uh oh!

were commented Sep 23, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tqchen commented Oct 13, 2018

Uh oh!

were commented Oct 14, 2018

Uh oh!

tqchen commented Oct 14, 2018

Uh oh!

were commented Oct 15, 2018

Uh oh!

tqchen commented Oct 15, 2018

Uh oh!

were commented Oct 15, 2018

Uh oh!

tqchen commented Oct 15, 2018

Uh oh!

were commented Oct 15, 2018

Uh oh!

tqchen commented Oct 15, 2018

Uh oh!

were commented Nov 15, 2018

Uh oh!

xqdan commented Nov 16, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xqdan commented Nov 16, 2018

Uh oh!

were commented Nov 16, 2018

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!

kevinthesun Nov 17, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

xqdan commented Nov 16, 2018

Uh oh!

were commented Nov 16, 2018

Uh oh!

kevinthesun commented Nov 17, 2018

Uh oh!

xqdan left a comment

Choose a reason for hiding this comment

were commented Sep 21, 2018 •

edited

Loading

tqchen commented Sep 22, 2018 •

edited

Loading

were commented Sep 23, 2018 •

edited

Loading

kevinthesun Nov 17, 2018 •

edited

Loading