[Relay] Better shape inference in TensorFlow Frontend.

[jwfromm]

In some cases it may be necessary to infer the value of a tensor to determine the shape. For example Fill, Reshape, and Resize have shape arguments that must be known. Reshape already handled this but fill and resize did not. I've added a function that infers the value of an input and sets the appropriate output shape for these classes.

Also added a DepthToSpace operation, a mapping from ResizeBicubic to _resize_bilinear (better than nothing for now), and added flexibility to bias add for non NHWC layers.

[tqchen]

@srkreddy1238 @Huyuwei @kazum please help review this PR

[srkreddy1238]

@jwfromm Can you add some test cases for new operators and also to cover the infer_value additions.

Thanks

[jwfromm]

Test cases are nearly finished, however I think I've found an unrelated Relay bug on the master branch. It looks like Relay fails to build a graph with a simple fill operation when using opt_level >= 2. Here's a snippet of code that can be run to demonstrate the error.

x = relay.op.full(relay.expr.const(1.), shape=[32], dtype='float32')
xfunc = relay.expr.Function([], x)
with relay.build_config(opt_level=2):
    graph, lib, params = relay.build(xfunc, "llvm")

This triggers the error
TVMError: Check failed: funcs.size() != 0U (0 vs. 0)

The above code works as expected with opt_level 0 or 1. Any thoughts on why this is happening? It makes it difficult to test one of the cases for infer_value.

[zhiics]

@jwfromm I think this is probably because const folding optimized the full op away.

[jwfromm]

Just pushed with added tests for depth_to_space, resize_bilinear, and fill. In the latter two tests, the output shape is based on the value of placeholders, which requires infer_value to have a proper output. At high opt_level, relay was trying to optimize the fill operation away, so I added an argument to change the opt level used during testing. Currently fill is the only test that needs this but it's a decent option to have for future tests.

[tqchen]

@srkreddy1238 @zhiics please take another look and https://docs.tvm.ai/contribute/code_review.html#approve-and-request-changes-explicitly

[zhiics]

It mostly looks good to me. Just left some minor comments. It would be good if @srkreddy1238 or @yongwww can take a look as well.

[jwfromm]

I know it's not great to add ops so late in the review, but I've also added the BroadcastTo op to the tensorflow parser and corresponding tests. This op also requires infer_value so it seems like it makes sense to include in this PR. It's actually a very common operation in tensorflow so we definitely want to include it eventually regardless.

[srkreddy1238]

We could add additional check to make sure all inputs to infer is available in params.
A utility similar to list_input_names() in nnvm.

Not sure if we have similar API in relay.
cc @zhiics @jroesch @tqchen

[zhiics]

Do you think something like this function could help?
https://github.com/dmlc/tvm/blob/master/python/tvm/relay/backend/interpreter.py#L136

[srkreddy1238]

I doubt it fits here.
What we need here is list of Var nodes current Expr is dependent on recursively.

[zhiics]

Can you just take free_vars if it is an expr? Var has name_hint.

[srkreddy1238]

free_vars should work.

@jwfromm can you cross check free_vars with params and add an assert ?

[jwfromm]

Added in the latest push.

[srkreddy1238]

Thanks @jwfromm . Good to go now.

[srkreddy1238]

@zhiics https://docs.tvm.ai/contribute/code_review.html#approve-and-request-changes-explicitly

[srkreddy1238]

Thanks @zhiics & @jwfromm . This is now merged.