[Relay][Any] Add shape func for dynamic shape #3606

icemelon · 2019-07-23T22:46:24Z

This PR aims to make interpreter and VM support dynamic shape.
Dynamic shape function is responsible for calculating the output shape of an op at runtime. It can have two different semantics:

If the output shapes of an op are data dependant (op.shape_data_dependant=true), the shape function's inputs should be the same data tensors to this op;
Otherwise, the shape function's inputs should be the shape tensors of inputs to this op.

Current progress:

Interpreter can run relay program with dynamic shape as long as all ops with dynamic shape have shape function defined
Wrote shape functions for broadcast ops, arange, concatenate, and reshape using hybrid script
VM supports dynamic shapes w/o and w/ op fusion enabled.

Caveats:

Dynamic shape function doesn't support CUDA. Need to wait until VM supports heterogeneous execution
TOPI broadcast ops currently treat an Any dimension as 1, and therefore gives wrong outputs when Any dimension is not 1. For example, a : Tensor[Any] + b : Tensor[3, 2] is allowed in both Relay and TOPI. When a.shape=[1] at runtime, TOPI will give the correct result. However, if a.shape=[2], the result will be wrong. The solution to this issue needs to use auto broadcast buffer ([Codegen] Support broadcast op with symbolic shape #3389). I plan to leave this to a follow-up PR.

@jroesch @zhiics @wweic @tqchen @MarisaKirisame @junrushao1994

a1010428282 · 2019-07-29T01:10:33Z

I'm interested. Why doesn't anyone comment on it

python/tvm/hybrid/parser.py

junrushao · 2019-08-02T03:56:38Z

Is this ready for review?

icemelon · 2019-08-02T04:36:12Z

@junrushao1994 Yes, you can start to review this PR except for the VM compiler part.

were · 2019-08-02T05:18:55Z

Except WIP, this LGTM.

junrushao · 2019-08-02T08:24:16Z

src/relay/backend/vm/compiler.cc

-      Emit(alloc);
-      unpacked_arg_regs.push_back(alloc.dst);
-    }
+//    if (const TensorTypeNode* ttype = ret_type.as<TensorTypeNode>()) {


Should we remove these lines?

Yes, still WIP for this file.

junrushao · 2019-08-02T08:45:26Z

python/tvm/relay/op/_transform.py

+                "Not enough dims in input shape for -3"
+            out[dst_idx] = x[src_idx] * x[src_idx+1]
+            src_idx += 2
+            dst_idx += 1


Are those only for the very ad-hoc reshape functions in MXNet?

This might be out of the scope of this PR but can we think about removing the ad-hoc reshape from the Relay core and possibly elaborating away i.e MxNet dialect.

junrushao · 2019-08-02T09:08:55Z

First, this PR mainly focuses on operators where we are able to pre-compute output types once they know their input value (np.arange), not those that we only know the output types after computation is done (e.g. np.unique). Wouldn’t it be more informative if we come up with another name?

Second, this kind of operators occurs only when those input values are like scalars, like shape array (tuple of integers). In our previous practice, if they are known statically, we would prefer to put them into Attrs. However, in this PR, we bypass type relation and use pre-defined shape function that could be lowered to LLVM to do code generation.

Just saying, will it be possible if we have some mechanism to put those scalars back to Attrs so that we can reuse previous efforts in defining TypeRelation?

For example, imagine we have primitive types in Relay, then we allow one operator to produce tvm::Arraytvm::Integer as output. Then in the following up operator, we let the system to put these Integers as attributes, then we are able to reuse those TypeRelations we previously defined.

zhiics

LGTM, only a few questions to clarify.

src/relay/backend/vm/compiler.cc

tests/python/relay/test_any.py

src/relay/backend/compile_engine.h

src/relay/backend/compile_engine.cc

src/relay/backend/vm/compiler.cc

icemelon · 2019-08-21T17:38:17Z

@jroesch @junrushao1994 @were Could you help review this PR again?

junrushao

LGTM

jwfromm · 2019-08-22T19:24:39Z

tests/python/relay/test_any.py

+def any_dims(ndim):
+    shape = []
+    for _ in range(ndim):
+        shape.append(relay.Any())


It would be more convenient to have dimensions with -1 internally map to relay.Any rather than requiring it to be explicit. This would match the syntax of all other frameworks when it comes to dynamic shape.

I see the argument for having convenient ways to specify dynamic shapes from the frontends, but for analysis and transformation having dynamic shapes be treated specially is important instead of overloading integers. Annoyingly some of the Relay operators have the semantics of MxNet's operators baked in right now. My 2c is that we should make greater use of dialects of Relay in which the operators macro expand to the ones that are easy to operate and analyze.

IMO we should remove those ad-hoc semantics like those in reshape.

include/tvm/relay/op.h

jroesch · 2019-08-23T07:09:04Z

src/relay/backend/compile_engine.cc

@@ -283,6 +305,248 @@ class ScheduleGetter :
  Array<Operation> scalars_;
 };

+// The getter to get shape function from functor.


Can we possibly use a better name here? ShapeFuncGetter is not very descriptive imo, MakeShapeFunc, GetShapeFunc?

jroesch · 2019-08-23T07:10:31Z

src/relay/backend/interpreter.cc

@@ -310,7 +310,124 @@ class Interpreter :
    return MakeClosure(func);
  }

-  Value InvokePrimitiveOp(Function func,


Should we factor this code out somewhere?

I guess you mean set inputs and outputs part to call the packed function? It has some difference in between invoking shape function and invoking primitive ops since shape function could need either data or shape.
Further this part of code is coupled with interpreter since it involves TVMValue that only is used by interpreter.

Okay, we can revisit.

jroesch

Just left a few comments.

icemelon · 2019-08-27T20:25:02Z

@tqchen @jroesch could you take another took?

MarisaKirisame · 2019-08-28T20:42:34Z

src/relay/backend/compile_engine.cc

+        const auto *tuple_type = param->type_as<TupleTypeNode>();
+        CHECK(tuple_type);
+        for (Type field : tuple_type->fields) {
+          const auto *ttype = field.as<TensorTypeNode>();


can you support recursive tuple of tensor? It is not much work, and a lot of other piece also support that.

There're quite many places that don't support recursive tuple. I left a few TODOs in the code, though there could be more. Let's leave them in the future PR to systematically fix this issue.

MarisaKirisame · 2019-08-28T20:44:12Z

src/relay/backend/compile_engine.cc

+      CHECK(rtype);
+      for (size_t i = 0; i < rtype->fields.size(); ++i) {
+        auto ttype = rtype->fields[i].as<TensorTypeNode>();
+        CHECK(ttype);


allow recursive tuple

MarisaKirisame · 2019-08-28T20:45:33Z

src/relay/backend/interpreter.cc

+      << "Shape function input sizes mismatch";
+
+    auto fset_shape_output = [&](size_t i, Type val_type) {
+        const TensorTypeNode* rtype = val_type.as<TensorTypeNode>();


recursive. see gradient for how to do recursive tuple.

Tianqi on vacation

* init shape func in interpreter and vm compiler * Update interpreter * fix * lint * lint * fix * remove hack * update * fix * fix * update * address comments & update for shape_of * fix lint * update * fix hybrid * lint * fix bug & add take shape func * lint * lint * update * fix flaky test * add todo

icemelon changed the title ~~[Any] Add shape func for dynamic shape~~ [Relay][Any] Add shape func for dynamic shape Jul 23, 2019

were suggested changes Aug 1, 2019

View reviewed changes

python/tvm/hybrid/parser.py Outdated Show resolved Hide resolved

were approved these changes Aug 2, 2019

View reviewed changes

junrushao reviewed Aug 2, 2019

View reviewed changes

junrushao approved these changes Aug 2, 2019

View reviewed changes

junrushao reviewed Aug 2, 2019

View reviewed changes

mingwayzhang mentioned this pull request Aug 6, 2019

[TOPI][FIX] Typo in schedule template for conv2d_direct (#3648) #3717

Merged

icemelon force-pushed the any-sf branch from db1a635 to 2053229 Compare August 16, 2019 21:16

icemelon marked this pull request as ready for review August 16, 2019 21:17

zhiics approved these changes Aug 20, 2019

View reviewed changes

src/relay/backend/vm/compiler.cc Show resolved Hide resolved

tests/python/relay/test_any.py Outdated Show resolved Hide resolved

wweic requested changes Aug 20, 2019

View reviewed changes

icemelon added 14 commits August 20, 2019 22:34

init shape func in interpreter and vm compiler

b86f976

Update interpreter

7984fd1

fix

8a2671c

lint

8060a15

lint

c06c2db

fix

602f442

remove hack

9d13b28

update

eb6a477

fix

275ef47

fix

4cd74ac

update

dbb2e0e

address comments & update for shape_of

3776513

fix lint

5b9bc06

update

be8f109

fix hybrid

93f06d5

icemelon force-pushed the any-sf branch from ed8e697 to 93f06d5 Compare August 21, 2019 17:35

lint

ce91252

junrushao approved these changes Aug 22, 2019

View reviewed changes

fix bug & add take shape func

05ee9be

jwfromm reviewed Aug 22, 2019

View reviewed changes

icemelon added 2 commits August 22, 2019 13:48

lint

7a2d8c7

lint

a8afb65

tqchen previously requested changes Aug 22, 2019

View reviewed changes

include/tvm/relay/op.h Outdated Show resolved Hide resolved

wweic approved these changes Aug 23, 2019

View reviewed changes

jroesch reviewed Aug 23, 2019

View reviewed changes

jroesch requested changes Aug 23, 2019

View reviewed changes

wweic mentioned this pull request Aug 23, 2019

[Relay][Frontend][TF] Add tensor array ops #3798

Merged

11 tasks

icemelon added 2 commits August 26, 2019 11:32

update

3f621ed

fix flaky test

4ac1f58

wweic approved these changes Aug 27, 2019

View reviewed changes

jroesch approved these changes Aug 28, 2019

View reviewed changes

MarisaKirisame reviewed Aug 28, 2019

View reviewed changes

add todo

a27ec96

jroesch approved these changes Sep 1, 2019

View reviewed changes

jroesch merged commit eef35a5 into apache:master Sep 1, 2019

icemelon deleted the any-sf branch September 6, 2019 18:52

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][Any] Add shape func for dynamic shape #3606

[Relay][Any] Add shape func for dynamic shape #3606

icemelon commented Jul 23, 2019 •

edited

Loading

a1010428282 commented Jul 29, 2019

junrushao commented Aug 2, 2019

icemelon commented Aug 2, 2019

were commented Aug 2, 2019

junrushao Aug 2, 2019

icemelon Aug 2, 2019

junrushao Aug 2, 2019

icemelon Aug 2, 2019

jroesch Aug 23, 2019

junrushao commented Aug 2, 2019 •

edited

Loading

zhiics left a comment

icemelon commented Aug 21, 2019

junrushao left a comment

jwfromm Aug 22, 2019

jroesch Aug 23, 2019

junrushao Aug 23, 2019

jroesch Aug 23, 2019

icemelon Aug 26, 2019

jroesch Aug 23, 2019

icemelon Aug 26, 2019

jroesch Aug 28, 2019

jroesch left a comment

icemelon commented Aug 27, 2019

MarisaKirisame Aug 28, 2019

icemelon Aug 28, 2019

MarisaKirisame Aug 28, 2019

MarisaKirisame Aug 28, 2019

[Relay][Any] Add shape func for dynamic shape #3606

[Relay][Any] Add shape func for dynamic shape #3606

Conversation

icemelon commented Jul 23, 2019 • edited Loading

a1010428282 commented Jul 29, 2019

junrushao commented Aug 2, 2019

icemelon commented Aug 2, 2019

were commented Aug 2, 2019

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

junrushao commented Aug 2, 2019 • edited Loading

zhiics left a comment

Choose a reason for hiding this comment

icemelon commented Aug 21, 2019

junrushao 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

jroesch left a comment

Choose a reason for hiding this comment

icemelon commented Aug 27, 2019

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

icemelon commented Jul 23, 2019 •

edited

Loading

junrushao commented Aug 2, 2019 •

edited

Loading