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

[FRONTEND][TENSORFLOW] Support Unstack and Split #2105

Merged
merged 2 commits into from
Dec 13, 2018

Conversation

alexeyr
Copy link
Contributor

@alexeyr alexeyr commented Nov 14, 2018

No description provided.

@alexeyr alexeyr force-pushed the extend_from_tensorflow branch 2 times, most recently from 03fb265 to 76f4651 Compare November 14, 2018 13:40
@alexeyr
Copy link
Contributor Author

alexeyr commented Nov 16, 2018

Test failure should be spurious: the same test failed for other pull requests submitted at similar time, including a doc fix.

if len(outputs) > 1:
tvm_n = tvm_n[num_layer]
inputs.append(tvm_n)
input_shapes[tvm_n] = self._output_shapes[i]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should it be the following for multiple outputs cases?
input_shapes[tvm_n] = (self._output_shapes[i])[num_layer]

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think so. I've added a commit which fixes this, but discovered a problem while trying to create a test case distinguishing them.

@alexeyr alexeyr changed the title [FRONTEND][TENSORFLOW] Support Unstack and Split [WIP][FRONTEND][TENSORFLOW] Support Unstack and Split Nov 16, 2018
@alexeyr
Copy link
Contributor Author

alexeyr commented Nov 16, 2018

I've run into an issue when extending test for unstack. It boils down to this difference:

In Tensorflow,

In [30]: tf.squeeze(tf.placeholder("int32", [1]),0).shape
Out[30]: TensorShape([])

But in TVM,

In [31]: topi.squeeze(tvm.placeholder((1,)),0).shape
Out[31]: [1]

The expected result of squeeze is to drop by one dimension, but in this case it doesn't.
So when I tried to test tf.stack(tf.unstack(in_data, axis=0), axis=0) where in_data has shape (6,), Tensorflow gives the expected result but TVM gives result of shape (6,1).

So, does NNVM support representing scalars with shape () instead of (1,)? I tried to use split_item[0] in d275972#diff-3845514369cc735d96dd63a274a21471R815, but that doesn't work (and I realize it shouldn't, it gets the first output instead of indexing).

Two alternatives if that can't be done:

  1. Error when trying to unstack or squeeze 1d tensors (or later when a shape mismatch is detected).
  2. Mark them with an attribute indicating they "should" be () and handle it in _pack and _expand_dims (do any other operators need it?)

What's best to do here?

@alexeyr alexeyr force-pushed the extend_from_tensorflow branch from d275972 to c350a95 Compare November 21, 2018 11:20
@alexeyr alexeyr changed the title [WIP][FRONTEND][TENSORFLOW] Support Unstack and Split [FRONTEND][TENSORFLOW] Support Unstack and Split Nov 21, 2018
@alexeyr alexeyr force-pushed the extend_from_tensorflow branch 3 times, most recently from 79dd4e2 to 60c9e25 Compare November 21, 2018 11:40
@alexeyr
Copy link
Contributor Author

alexeyr commented Nov 21, 2018

The problem should be fixed now. @srkreddy1238 @siju-samuel seem to be most familiar with this frontend, please review.

@alexeyr
Copy link
Contributor Author

alexeyr commented Nov 22, 2018

Rebased on master.

@alexeyr alexeyr force-pushed the extend_from_tensorflow branch 2 times, most recently from b178471 to 37bf662 Compare December 5, 2018 08:12
@srkreddy1238
Copy link
Contributor

@alexeyr I see two open PR's for Split (other #2123 ).
To give an equal opportunity can you add SplitV and unstack on top of #2123 ?

@alexeyr
Copy link
Contributor Author

alexeyr commented Dec 5, 2018

By rebasing on top on it? OK, will do tomorrow.

@alexeyr alexeyr force-pushed the extend_from_tensorflow branch 3 times, most recently from 69029a9 to 0003a35 Compare December 6, 2018 09:09
@alexeyr
Copy link
Contributor Author

alexeyr commented Dec 6, 2018

@srkreddy1238 Done.

@srkreddy1238
Copy link
Contributor

@alexeyr pls rebase against latest master.

@alexeyr alexeyr force-pushed the extend_from_tensorflow branch from de8da06 to 388bb44 Compare December 7, 2018 09:12
@alexeyr
Copy link
Contributor Author

alexeyr commented Dec 7, 2018

@srkreddy1238 Rebased.

Copy link
Contributor

@srkreddy1238 srkreddy1238 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alexeyr thanks. Just a cosmetic change otherwise LGTM.

nnvm/python/nnvm/frontend/tensorflow.py Outdated Show resolved Hide resolved
@yzhliu yzhliu added the status: need update need update based on feedbacks label Dec 11, 2018
@alexeyr alexeyr force-pushed the extend_from_tensorflow branch from 388bb44 to c3327a5 Compare December 12, 2018 07:08
Copy link
Contributor

@srkreddy1238 srkreddy1238 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@tqchen tqchen merged commit 00d509d into apache:master Dec 13, 2018
@tqchen
Copy link
Member

tqchen commented Dec 13, 2018

Thanks, @srkreddy1238 @alexeyr , this is merged

@tqchen tqchen added status: accepted and removed status: need review status: need update need update based on feedbacks labels Dec 13, 2018
FrozenGene pushed a commit to FrozenGene/tvm that referenced this pull request Dec 27, 2018
wweic pushed a commit to neo-ai/tvm that referenced this pull request Feb 20, 2019
wweic pushed a commit to neo-ai/tvm that referenced this pull request Feb 20, 2019
srkreddy1238 added a commit to srkreddy1238/tvm that referenced this pull request Mar 9, 2019
commit f347b52
Author: Yong Wu <yongwu@amazon.com>
    Get tags of saved model automatically

commit 916576c
Author: Zhi Chen <chzhi@amazon.com>
    Support TensorFlow saved model
    TF parser: return the consistent error message to error handler

commit f1782f3
Author: Yong Wu <yongwu@amazon.com>
    Add tf parser wrapper, infer shape automatically

commit 76188a4
Author: Siva <sivar.b@huawei.com>
    [NNVM][TENSORFLOW] bugfix. (apache#2444)

commit 6737739
Author: Ashutosh Parkhi <ashutosh.parkhi@imgtec.com>
    [Tensorflow] Support for Crop (apache#2285)

commit f6c3f99
Author: Alexey Romanov <alexey.v.romanov@gmail.com>
    [FRONTEND][TENSORFLOW] Use input shapes directly instead of 1-element lists (apache#2242)

commit e5d92e1
Author: Dominic Symes <36929632+dominicsymes@users.noreply.github.com>
    [FRONTEND][TENSORFLOW] Bugfix (apache#2326)

commit 00d509d
Author: Alexey Romanov <alexey.v.romanov@gmail.com>
    [FRONTEND][TENSORFLOW] Support Unstack and Split (apache#2105)

commit df9d3ad
Author: Siva <sivar.b@huawei.com>
    [FRONTEND][TENSORFLOW] Bugfix (apache#2267)

commit d1a0c90
Author: Zhebin Jin <zhebin.jzb@alibaba-inc.com>
    [FRONTEND][TENSORFLOW]Add Split and realdiv op support (apache#2123)
    * Add Split and realdiv op support
    * Fix the pad calculation in the case of dilated convolution
srkreddy1238 added a commit to srkreddy1238/tvm that referenced this pull request Mar 9, 2019
commit f347b52
Author: Yong Wu <yongwu@amazon.com>
    Get tags of saved model automatically

commit 916576c
Author: Zhi Chen <chzhi@amazon.com>
    Support TensorFlow saved model
    TF parser: return the consistent error message to error handler

commit f1782f3
Author: Yong Wu <yongwu@amazon.com>
    Add tf parser wrapper, infer shape automatically

commit 76188a4
Author: Siva <sivar.b@huawei.com>
    [NNVM][TENSORFLOW] bugfix. (apache#2444)

commit 6737739
Author: Ashutosh Parkhi <ashutosh.parkhi@imgtec.com>
    [Tensorflow] Support for Crop (apache#2285)

commit f6c3f99
Author: Alexey Romanov <alexey.v.romanov@gmail.com>
    [FRONTEND][TENSORFLOW] Use input shapes directly instead of 1-element lists (apache#2242)

commit e5d92e1
Author: Dominic Symes <36929632+dominicsymes@users.noreply.github.com>
    [FRONTEND][TENSORFLOW] Bugfix (apache#2326)

commit 00d509d
Author: Alexey Romanov <alexey.v.romanov@gmail.com>
    [FRONTEND][TENSORFLOW] Support Unstack and Split (apache#2105)

commit df9d3ad
Author: Siva <sivar.b@huawei.com>
    [FRONTEND][TENSORFLOW] Bugfix (apache#2267)

commit d1a0c90
Author: Zhebin Jin <zhebin.jzb@alibaba-inc.com>
    [FRONTEND][TENSORFLOW]Add Split and realdiv op support (apache#2123)
    * Add Split and realdiv op support
    * Fix the pad calculation in the case of dilated convolution
srkreddy1238 added a commit to srkreddy1238/tvm that referenced this pull request Mar 9, 2019
commit f347b52
Author: Yong Wu <yongwu@amazon.com>
    Get tags of saved model automatically

commit 916576c
Author: Zhi Chen <chzhi@amazon.com>
    Support TensorFlow saved model
    TF parser: return the consistent error message to error handler

commit f1782f3
Author: Yong Wu <yongwu@amazon.com>
    Add tf parser wrapper, infer shape automatically

commit 76188a4
Author: Siva <sivar.b@huawei.com>
    [NNVM][TENSORFLOW] bugfix. (apache#2444)

commit 6737739
Author: Ashutosh Parkhi <ashutosh.parkhi@imgtec.com>
    [Tensorflow] Support for Crop (apache#2285)

commit f6c3f99
Author: Alexey Romanov <alexey.v.romanov@gmail.com>
    [FRONTEND][TENSORFLOW] Use input shapes directly instead of 1-element lists (apache#2242)

commit e5d92e1
Author: Dominic Symes <36929632+dominicsymes@users.noreply.github.com>
    [FRONTEND][TENSORFLOW] Bugfix (apache#2326)

commit 00d509d
Author: Alexey Romanov <alexey.v.romanov@gmail.com>
    [FRONTEND][TENSORFLOW] Support Unstack and Split (apache#2105)

commit df9d3ad
Author: Siva <sivar.b@huawei.com>
    [FRONTEND][TENSORFLOW] Bugfix (apache#2267)

commit d1a0c90
Author: Zhebin Jin <zhebin.jzb@alibaba-inc.com>
    [FRONTEND][TENSORFLOW]Add Split and realdiv op support (apache#2123)
    * Add Split and realdiv op support
    * Fix the pad calculation in the case of dilated convolution
srkreddy1238 added a commit to srkreddy1238/tvm that referenced this pull request Mar 11, 2019
commit 76188a4
Author: Siva sivar.b@huawei.com
[NNVM][TENSORFLOW] bugfix. (apache#2444)

commit 6737739
Author: Ashutosh Parkhi ashutosh.parkhi@imgtec.com
[Tensorflow] Support for Crop (apache#2285)

commit f6c3f99
Author: Alexey Romanov alexey.v.romanov@gmail.com
[FRONTEND][TENSORFLOW] Use input shapes directly instead of 1-element lists (apache#2242)

commit e5d92e1
Author: Dominic Symes 36929632+dominicsymes@users.noreply.github.com
[FRONTEND][TENSORFLOW] Bugfix (apache#2326)

commit 00d509d
Author: Alexey Romanov alexey.v.romanov@gmail.com
[FRONTEND][TENSORFLOW] Support Unstack and Split (apache#2105)

commit df9d3ad
Author: Siva sivar.b@huawei.com
[FRONTEND][TENSORFLOW] Bugfix (apache#2267)

commit d1a0c90
Author: Zhebin Jin zhebin.jzb@alibaba-inc.com
[FRONTEND][TENSORFLOW]Add Split and realdiv op support (apache#2123)
* Add Split and realdiv op support
* Fix the pad calculation in the case of dilated convolution
srkreddy1238 added a commit to srkreddy1238/tvm that referenced this pull request Mar 12, 2019
commit 76188a4
Author: Siva sivar.b@huawei.com
[NNVM][TENSORFLOW] bugfix. (apache#2444)

commit 6737739
Author: Ashutosh Parkhi ashutosh.parkhi@imgtec.com
[Tensorflow] Support for Crop (apache#2285)

commit f6c3f99
Author: Alexey Romanov alexey.v.romanov@gmail.com
[FRONTEND][TENSORFLOW] Use input shapes directly instead of 1-element lists (apache#2242)

commit e5d92e1
Author: Dominic Symes 36929632+dominicsymes@users.noreply.github.com
[FRONTEND][TENSORFLOW] Bugfix (apache#2326)

commit 00d509d
Author: Alexey Romanov alexey.v.romanov@gmail.com
[FRONTEND][TENSORFLOW] Support Unstack and Split (apache#2105)

commit df9d3ad
Author: Siva sivar.b@huawei.com
[FRONTEND][TENSORFLOW] Bugfix (apache#2267)

commit d1a0c90
Author: Zhebin Jin zhebin.jzb@alibaba-inc.com
[FRONTEND][TENSORFLOW]Add Split and realdiv op support (apache#2123)
* Add Split and realdiv op support
* Fix the pad calculation in the case of dilated convolution
srkreddy1238 added a commit that referenced this pull request Mar 19, 2019
* [FRONTEND][TENSORFLOW] Enhance with left over patches from NNVM.

commit 76188a4
Author: Siva sivar.b@huawei.com
[NNVM][TENSORFLOW] bugfix. (#2444)

commit 6737739
Author: Ashutosh Parkhi ashutosh.parkhi@imgtec.com
[Tensorflow] Support for Crop (#2285)

commit f6c3f99
Author: Alexey Romanov alexey.v.romanov@gmail.com
[FRONTEND][TENSORFLOW] Use input shapes directly instead of 1-element lists (#2242)

commit e5d92e1
Author: Dominic Symes 36929632+dominicsymes@users.noreply.github.com
[FRONTEND][TENSORFLOW] Bugfix (#2326)

commit 00d509d
Author: Alexey Romanov alexey.v.romanov@gmail.com
[FRONTEND][TENSORFLOW] Support Unstack and Split (#2105)

commit df9d3ad
Author: Siva sivar.b@huawei.com
[FRONTEND][TENSORFLOW] Bugfix (#2267)

commit d1a0c90
Author: Zhebin Jin zhebin.jzb@alibaba-inc.com
[FRONTEND][TENSORFLOW]Add Split and realdiv op support (#2123)
* Add Split and realdiv op support
* Fix the pad calculation in the case of dilated convolution

* 	* review comments

* 	* resnet fix.

* 	* review comments
wweic pushed a commit to wweic/tvm that referenced this pull request Mar 20, 2019
…che#2757)

* [FRONTEND][TENSORFLOW] Enhance with left over patches from NNVM.

commit 76188a4
Author: Siva sivar.b@huawei.com
[NNVM][TENSORFLOW] bugfix. (apache#2444)

commit 6737739
Author: Ashutosh Parkhi ashutosh.parkhi@imgtec.com
[Tensorflow] Support for Crop (apache#2285)

commit f6c3f99
Author: Alexey Romanov alexey.v.romanov@gmail.com
[FRONTEND][TENSORFLOW] Use input shapes directly instead of 1-element lists (apache#2242)

commit e5d92e1
Author: Dominic Symes 36929632+dominicsymes@users.noreply.github.com
[FRONTEND][TENSORFLOW] Bugfix (apache#2326)

commit 00d509d
Author: Alexey Romanov alexey.v.romanov@gmail.com
[FRONTEND][TENSORFLOW] Support Unstack and Split (apache#2105)

commit df9d3ad
Author: Siva sivar.b@huawei.com
[FRONTEND][TENSORFLOW] Bugfix (apache#2267)

commit d1a0c90
Author: Zhebin Jin zhebin.jzb@alibaba-inc.com
[FRONTEND][TENSORFLOW]Add Split and realdiv op support (apache#2123)
* Add Split and realdiv op support
* Fix the pad calculation in the case of dilated convolution

* 	* review comments

* 	* resnet fix.

* 	* review comments
wweic pushed a commit to neo-ai/tvm that referenced this pull request Mar 20, 2019
…che#2757)

* [FRONTEND][TENSORFLOW] Enhance with left over patches from NNVM.

commit 76188a4
Author: Siva sivar.b@huawei.com
[NNVM][TENSORFLOW] bugfix. (apache#2444)

commit 6737739
Author: Ashutosh Parkhi ashutosh.parkhi@imgtec.com
[Tensorflow] Support for Crop (apache#2285)

commit f6c3f99
Author: Alexey Romanov alexey.v.romanov@gmail.com
[FRONTEND][TENSORFLOW] Use input shapes directly instead of 1-element lists (apache#2242)

commit e5d92e1
Author: Dominic Symes 36929632+dominicsymes@users.noreply.github.com
[FRONTEND][TENSORFLOW] Bugfix (apache#2326)

commit 00d509d
Author: Alexey Romanov alexey.v.romanov@gmail.com
[FRONTEND][TENSORFLOW] Support Unstack and Split (apache#2105)

commit df9d3ad
Author: Siva sivar.b@huawei.com
[FRONTEND][TENSORFLOW] Bugfix (apache#2267)

commit d1a0c90
Author: Zhebin Jin zhebin.jzb@alibaba-inc.com
[FRONTEND][TENSORFLOW]Add Split and realdiv op support (apache#2123)
* Add Split and realdiv op support
* Fix the pad calculation in the case of dilated convolution

* 	* review comments

* 	* resnet fix.

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

Successfully merging this pull request may close these issues.

5 participants