[Relay/TOPI][OP] Add clip and wrap mode support in take #2858
Conversation
|
cc @tqchen |
|
@Laurawly @yzhliu @merrymercy Could you help review this pr? |
include/tvm/relay/attrs/transform.h
Outdated
| TVM_ATTR_FIELD(mode).set_default("CLIP") | ||
| .describe("Specify how out-of-bound indices will behave." | ||
| "CLIP - clip to the range (default)" | ||
| "WRAP - wrap around the indices"); |
There was a problem hiding this comment.
suggest to use lower-case, to be the same as that in numpy. also add mode="raise" to the description
There was a problem hiding this comment.
Sure. I can change to lower case.
It's a bit tricky to add "raise" mode because current tvm::compute (see https://github.com/dmlc/tvm/blob/master/include/tvm/operation.h#L547) doesn't allow us to add AssertStmt in the body. I think we should add this in a follow-up pr.
cc @tqchen What do you suggest about AssertStmt?
There was a problem hiding this comment.
how about we skip raise for now?
| assert len(inputs) == 2 | ||
| mode = attrs.get_str("mode", "clip") | ||
| if mode == "raise": | ||
| raise RuntimeError("take doesn't support raise mode") |
There was a problem hiding this comment.
In fact, currently mxnet doesn't support raise either.
http://mxnet.incubator.apache.org/api/python/ndarray/ndarray.html?highlight=take#mxnet.ndarray.take
|
Thanks everyone. |
* Update take * Add special case for canonical simplify and fix test cases * Use lower case for wrap and clip * remove unnecssary lower * Fix mxnet converter for take * fix
* Update take * Add special case for canonical simplify and fix test cases * Use lower case for wrap and clip * remove unnecssary lower * Fix mxnet converter for take * fix
* Update take * Add special case for canonical simplify and fix test cases * Use lower case for wrap and clip * remove unnecssary lower * Fix mxnet converter for take * fix
* Update take * Add special case for canonical simplify and fix test cases * Use lower case for wrap and clip * remove unnecssary lower * Fix mxnet converter for take * fix
* Update take * Add special case for canonical simplify and fix test cases * Use lower case for wrap and clip * remove unnecssary lower * Fix mxnet converter for take * fix
* Update take * Add special case for canonical simplify and fix test cases * Use lower case for wrap and clip * remove unnecssary lower * Fix mxnet converter for take * fix
|
I prepared MXNet Hybrid block which uses |
|
@icemelon9 What you think? |
Previously take can return arbitrary values when indices are negative or too larger. It's very dangerous. Now it is fixed in this PR by eanbling take to handle out-of-bound indices in two modes: clip (default) and wrap.
Also fix bugs in HalideIR Simplify and CanonicalSimplify for non-Euclidean mod when operand is negative.