[FRONTEND][MXNET] Add squeeze_axis support to split operator #1288
Conversation
|
Topi already support squeeze operation, please check whether you can use it for your purpose. |
|
@PariksheetPinjari909 Thanks for your comment. I used squeeze operator to support split with squeeze_axis in topi. Could you review again? |
|
A better way would be directly modify the translator to translate to combination of squeeze then split operation |
|
@tqchen I understand that it is better to modify the translator (mxnet frontend), but couldn't do it. I look like it is difficult to simply apply squeeze symbol after split symbol at there, because the output of split is split matrices (these are unsitable as the input of squeeze). Also vice versea, I look like it is not available to apply squeeze before split. |
|
@nishi-t It might be possible to get each output of the split, and apply squeeze separately on each output? @zhreshold can you comment on this as well? |
|
I think it is possible to slice the output of split and apply squeeze one by one because you have num_outputs already |
topi/python/topi/transform.py
Outdated
|
|
||
| @tvm.tag_scope(tag=tag.INJECTIVE) | ||
| def split(ary, indices_or_sections, axis=0): | ||
| def __split(ary, indices_or_sections, axis=0): |
tqchen
added
status: review in progress
status: need update
|
@tqchen @zhreshold thanks for you comments.
Ok, I'll try it. |
…mxnet, add squeeze_axis support to split.
nishi-t
changed the title
nishi-t
changed the title
|
I addressed the comment. Please review again. |
nnvm/python/nnvm/frontend/mxnet.py
Outdated
| if _parse_bool_str(attrs, 'squeeze_axis'): | ||
| squeeze_attrs = {'axis': axis} | ||
| for o in outputs: | ||
| o = _get_nnvm_op('squeeze')(o, **squeeze_attrs) |
There was a problem hiding this comment.
are you sure this can be replaced in-place?
There was a problem hiding this comment.
Oh, it was my mistake. Thank you for pointing that out. This couldn't be replaced in-place. I'll fix it, but I'll need to find how to update the split symbol with squeeze. Please let me know if you have any good ideas.
|
@zhreshold I tried using Group for my purpose. Could you review this again? |
|
LGTM now, thanks. |
tqchen
added
status: accepted
and removed
status: need update
|
Thanks for all the reviewers and the contributor in improving the change. This is now merged |
In mxnet, split symbol has squeeze_axis option. This PR adds it to topi and nnvm. please review.
https://mxnet.incubator.apache.org/api/python/symbol/symbol.html#mxnet.symbol.split