Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RELAY][OP] Relay Operator Sprint #1799

Closed
66 tasks done
tqchen opened this issue Oct 3, 2018 · 52 comments
Closed
66 tasks done

[RELAY][OP] Relay Operator Sprint #1799

tqchen opened this issue Oct 3, 2018 · 52 comments

Comments

@tqchen
Copy link
Member

tqchen commented Oct 3, 2018

Now that the Relay RFC is being merged and we are stabilizing the type inference interface, we should sprint to add new operators to relay to make it on parity with NNVM.

#1798 shows an example on how to do so for conv2d operator.

General Steps of Porting

  • Implement the TypeRelation function, when necessary
    • The shapes represented by IndexExpr(symbolic integer)
      • When possible, support symbolic shape inference
      • You can, however, get the integer out from symbolic shape if it is a must, that will require the inference to work on concrete shapes.
    • User reporter->Assign to set the inferred result
    • Use reporter->AssertEQ to assert symbolic integer equivalence
      • It will return false if there is an unsatisfied constraint
  • Use tvm::Attrs to replace dmlc::Parameter
  • We switch to directly create python wrappers by calling into positional functions so that the operator signature is explicit in python

General Principles

  • Numpy consistency, always consistent with numpy
    • All binary operators broadcast
    • This means we will use add, subtract instead of broadcast_add, broadcast_sub ...
    • elemwise_add version will not be supported for now as we can just use the broadcast version
  • Consistent with nnvm when possible
  • Fields in Attrs
    • Use concrete types when possible(int, string, bool)
    • If you need None, you can use IndexExpr, which gives you that

List of Operators to be covered

Generally, we need to cover everything we have so far https://docs.tvm.ai/nnvm_top.html
Please use this issue to coordinate what you will be working on. As we expect things to move quickly, try to do "fine grained locking" and only claim things that you are working on right now and aim to get things in a few days.

The List

Level 1: Common Basic Ops

Enough to get MLP

  • nn.dense
  • nn.relu
  • tanh
  • sigmoid
  • exp
  • log
  • sqrt
  • add
  • subtract
  • multiply
  • divide
  • mod
  • nn.batch_flatten
  • concatenate
  • nn.softmax
  • nn.log_softmax
  • nn.batch_norm
  • nn.dropout
  • expand_dims

Level 2: Convolutions

Enough to get convnet

  • nn.conv2d
  • nn.conv2d_transpose
  • nn.max_pool2d
  • nn.avg_pool2d
  • nn.global_max_pool2d
  • nn.global_avg_pool2d
  • nn.pad
  • nn.lrn

Level 3: Additional Math And Transform Operators

  • reshape
  • copy
  • negative
  • floor
  • ceil
  • round
  • trunc
  • clip
  • abs
  • leaky_relu
  • tranpose
  • split
  • squeeze
  • take
  • full
  • zeros
  • ones
  • transpose
  • zeros_like
  • ones_like

Level 4: All broadcast and reduction functions that are not in previous level

  • pow
  • less
  • greater
  • less_than
  • greater_than
  • right_shift
  • left_shift
  • maximum
  • minimum
  • sum
  • max
  • prod
  • argmax, argmin
  • strided_slice
  • broadcast_to
  • where

Level 5: Vision Operators

  • image.resize
  • vision.multibox_prior
  • vision.nms

Level 10: Backend Operators

Operators necessary as intermediate stage of optimizations, or gradient, can be influx

@MarisaKirisame
Copy link
Contributor

I had 'implemented' some elem wise function which I need for ad (negative, multiplication, division)
('implemented' because it isnt possible to lower code right now)
I can take some operators, and I dont have any particular preference.

@srkreddy1238
Copy link
Contributor

I could get through resize operator (will PR once #1798 is merged) to start with and proceeding with transforms.

@junrushao
Copy link
Member

junrushao commented Oct 3, 2018

I strongly agree with the numpy consistency, e.g. nnvm.symbol.flatten should be renamed.

A good example could be that TensorFlow's API uses batch_xxx for batched operators.

@junrushao
Copy link
Member

I can work on some shape-related APIs.

@srkreddy1238
Copy link
Contributor

@junrushao1994
Can you share the ops you are working on to avoid the duplication?

@srkreddy1238
Copy link
Contributor

@tqchen
How about relaying an assert on a condition which involve two variables ?
Ref. Transposed convolution where channels should divide groups and both are variables.

@junrushao
Copy link
Member

junrushao commented Oct 4, 2018

@srkreddy1238
I am starting with expand_dims, the easiest one
#1819

@tqchen
Copy link
Member Author

tqchen commented Oct 4, 2018

One note about int32 vs int64 when constructing constant was raised by @junrushao1994 @srkreddy1238 . This is an issue we should think about now. int32 will likely cause some regression on large arrays which need to be fixed. I think we should prefer int64 when possible for constants, and let the compiler to automatically detect and downgrade to int32.

A temporary workaround is always to keep the inferred shape type consistent with the input shape type, and we can make the switch more easily in one place later

@junrushao
Copy link
Member

Another thing I am concerning is user friendliness.

First, examples provided by Python API docs should be at least runnable by copy-pasting, like PyTorch (https://pytorch.org/docs/stable/tensors.html) or NumPy (https://docs.scipy.org/doc/numpy/reference/generated/numpy.expand_dims.html).

Second, Python API docs should be self-contained, at least those designed for DL practitioners who may not take a good look at the C++ code.

It does not seem to be a big deal for now, but we should put more effort into it in the future.

@tqchen
Copy link
Member Author

tqchen commented Oct 4, 2018

+1 for API docs friendless, I would recommend we do it now than later. Maybe I am having a bad lead example in the conv2d docs as it was pretty minimum, I will send an updated PR to update that, and let us make sure the new ops are being well documented with examples, especially non-trival ones

@srkreddy1238
Copy link
Contributor

Expr like below is not getting simplified !!

TensorTypeNode(float32, [n, c, int32((float32(100)*2.000000f)), int32((float32(200)*2.000000f))])

any idea ?

@tqchen
Copy link
Member Author

tqchen commented Oct 4, 2018

The eager CSE is done among integer expression only so far. For floating points, we still need to call explicitly simplification, or use as_const_int to get out and explicit simplify

@junrushao
Copy link
Member

junrushao commented Oct 5, 2018

I am going to grab transpose (update: no, it does not exist in the list)

I am going to grab less, greater, less_equal, greater_equal...

@tqchen
Copy link
Member Author

tqchen commented Oct 5, 2018

@junrushao1994 transpose should be in the list, sorry the list was not complete

@MarisaKirisame
Copy link
Contributor

I am taking/had taken multiply/divide/mod/relu/tanh/sigmoid/negative.

@srkreddy1238
Copy link
Contributor

@MarisaKirisame
I have covered multiply, divide, mod, tanh, sigmoid, negative already in #1813

@srkreddy1238
Copy link
Contributor

srkreddy1238 commented Oct 5, 2018

To keep all of us on same page #1813 covers
multiply, mod, tanh, sigmoid, negative, floor, ceil, trunc, abs, pow, resize, upsampling, batch_flatten, pool2d and global_pool2d

@junrushao
Copy link
Member

junrushao commented Oct 5, 2018

expand_dims is in #1819
Comparisons greater greater_equal less less_equal not_equal equal are in #1824

This was referenced Oct 5, 2018
@junrushao
Copy link
Member

junrushao commented Oct 5, 2018

ok I am going to take reshape , transpose, copy and concatenate.

concatenate seems to have been done, so what i need to do is only change the API a little bit to keep numpy consistency.

@junrushao
Copy link
Member

junrushao commented Oct 5, 2018

@srkreddy1238 you forgot to mention that you have done round and all pool2d-related operators in your pr #1813 as well.

To keep all of us on same page #1813 covers
multiply, mod, tanh, sigmoid, negative, floor, ceil, trunc, abs, pow, resize, upsampling, batch_flatten, pool2d and global_pool2d

@tqchen
Copy link
Member Author

tqchen commented Oct 5, 2018

take right_shift

@tmoreau89
Copy link
Contributor

take left_shift

@MarisaKirisame
Copy link
Contributor

I need squeeze for ad with broadcast (right now it is assuming no broadcast). I will take it.

@MarisaKirisame
Copy link
Contributor

I had done zeros_like and ones_like. I think I will take zeros and ones too for symmetricity

@Luo-Liang
Copy link
Contributor

attempting maximum

@tqchen
Copy link
Member Author

tqchen commented Oct 6, 2018

I think there is a python parser that does the similar thing but needs to confirm with @jroesch @joshpoll

@junrushao
Copy link
Member

@tqchen Sounds cool!

@joshpoll
Copy link
Contributor

joshpoll commented Oct 6, 2018

You can do this with mypy types, see #1781 for an example. We could add type annotations to the operators, which will provide the sanity checks you want. Mypy is a static analyzer, so in order to get its benefits you need to run it separately or have it integrated into your IDE.

@srkreddy1238
Copy link
Contributor

srkreddy1238 commented Oct 8, 2018

taking conv2d_transpose

@siju-samuel
Copy link
Member

@tqchen how to handle multiple outputs? i was trying dropout and batchnorm, both have multiple outputs.
In RELAY_REGISTER_OP, like set_num_inputs, do we have something like set_num_outputs?
I think currently Array<Type>& types will have only (inputs + one output)

@srkreddy1238
Copy link
Contributor

multiple outputs is needed for split too.

@MarisaKirisame
Copy link
Contributor

@siju-samuel @srkreddy1238 multiple output is possible only by wrapping all of them in a tuple type

@slyubomirsky
Copy link
Contributor

slyubomirsky commented Oct 9, 2018

I'll take batch_norm and dropout as well, to finish out level 1

edit: Sorry, @siju-samuel, I didn't see your comment and didn't mean to snipe you with those! Tell me if you're still trying those, or else I could finish my own attempts. I don't mind either way.

@siju-samuel
Copy link
Member

i started with reduce ops, you can do with batch_norm & dropout

@MarisaKirisame
Copy link
Contributor

broadcast_to, collapse_sum, broadcast_to_like, collapse_sum_like

@zhiics
Copy link
Member

zhiics commented Oct 9, 2018

attempting where

@siju-samuel
Copy link
Member

strided_slice

@srkreddy1238
Copy link
Contributor

Attempting split to conclude Level 3

@anijain2305
Copy link
Contributor

Will attempt prod

@jroesch
Copy link
Member

jroesch commented Nov 1, 2018

Thanks to everyone for the hard work on getting 99% of the way there. I'm making a push to now add the compute and scheduling behavior for all of these operators which should enable users to use Relay for end-to-end inference tasks, enable new frontends and more. If you you would be interested in helping read more here: #2051.

@yzhliu
Copy link
Member

yzhliu commented Nov 17, 2018

@tqchen I believe all listed operators have been implemented, could you double check?

@tqchen
Copy link
Member Author

tqchen commented Nov 19, 2018

Thanks to everyone for the hard work, this issue is closed as most ops are in, we will follow up in #2051

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests