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.

Sign up for GitHub

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

Jump to bottom

tf.split's -1 support doesn't handle zero dimensions #19360

Closed
girving opened this issue May 17, 2018 · 11 comments
Closed

tf.split's -1 support doesn't handle zero dimensions #19360

girving opened this issue May 17, 2018 · 11 comments
Assignees
@girving
Labels
type:bug Bug

Comments

@girving
Copy link
Contributor

girving commented May 17, 2018

System information

  • Have I written custom code (as opposed to using a stock example script provided in TensorFlow): Yes.
  • OS Platform and Distribution (e.g., Linux Ubuntu 16.04): Colab
  • TensorFlow installed from (source or binary): Colab
  • TensorFlow version (use command below): ('unknown', '1.7.0') and 1.8.0
  • Python version: 3.6.3
  • Bazel version (if compiling from source): N/A
  • GCC/Compiler version (if compiling from source): N/A
  • CUDA/cuDNN version: N/A
  • GPU model and memory: N/A
  • Exact command to reproduce: tf.split(tf.zeros([0]), [0, -1], axis=-1)

Describe the problem

The variable size version of tf.split (SplitV in C++) allows one of the sizes to be -1. The corresponding output will expand as necessary so that the total output size matches the input.

Unfortunately, the -1 support currently assumes the -1 dimension corresponds to positive size. It should handle zero as well. E.g., this should work, but it doesn't:

>>> tf.split(tf.zeros([0]), [0, -1], axis=-1)
Traceback (most recent call last):
  File "/Users/irving/anaconda/envs/openai/lib/python3.6/site-packages/tensorflow/python/framework/ops.py", line 1592, in _create_c_op
    c_op = c_api.TF_FinishOperation(op_desc)
tensorflow.python.framework.errors_impl.InvalidArgumentError: Sum of output sizes must match the size of the original Tensor along the split dimension or the sum of the positive sizes must be less if it contains a -1 for 'split_3' (op: 'SplitV') with input shapes: [0], [2], [] and with computed input tensors: input[1] = <0 -1>, input[2] = <-1>.

By comparison, the positive case works fine:

>>> tf.split(tf.zeros([1]), [0, -1], axis=-1)
[<tf.Tensor 'split_4:0' shape=(0,) dtype=float32>, <tf.Tensor 'split_4:1' shape=(?,) dtype=float32>]
@girving girving changed the title tf.split's -1 support doesn't handle empty tensors tf.split's -1 support doesn't handle zero dimensions May 17, 2018
@girving
Copy link
Contributor Author

girving commented May 17, 2018

Here's a colab exhibiting the problem, though since the repro is a single line it's not really necessary:

https://drive.google.com/file/d/1AyZLBPYkiB3HgUZYL9cWZx3IFu2OzcTI/view?usp=sharing

@girving
Copy link
Contributor Author

girving commented May 17, 2018 •
edited

@benoitsteiner: Did you introduce this restriction? Can't tell if 53f6845 is yours.

@karmel
Copy link

karmel commented May 18, 2018

@ekelsen might have more insight into that particular commit?

@karmel karmel added the stat:awaiting tensorflower Status - Awaiting response from tensorflower label May 18, 2018
@tensorflowbutler
Copy link
Member

Nagging Assignee @karmel: It has been 14 days with no activity and this issue has an assignee. Please update the label and/or status accordingly.

@karmel
Copy link

karmel commented Jun 2, 2018

Bump for @ekelsen - do you have any insight here, or is someone else a better bet?

@tensorflowbutler
Copy link
Member

Nagging Assignee @karmel: It has been 14 days with no activity and this issue has an assignee. Please update the label and/or status accordingly.

@tensorflowbutler
Copy link
Member

Nagging Assignee @karmel: It has been 29 days with no activity and this issue has an assignee. Please update the label and/or status accordingly.

@tensorflowbutler
Copy link
Member

Nagging Assignee @karmel: It has been 44 days with no activity and this issue has an assignee. Please update the label and/or status accordingly.

@karmel
Copy link

karmel commented Jul 17, 2018

Bumping @ekelsen again, who may be back?

@girving girving mentioned this issue Jul 25, 2018
Merged
@girving girving added stat:contribution welcome Status - Contributions welcome type:bug Bug and removed stat:awaiting tensorflower Status - Awaiting response from tensorflower labels Jul 25, 2018
@girving girving self-assigned this Jul 25, 2018
@girving girving removed the stat:contribution welcome Status - Contributions welcome label Jul 25, 2018
@tensorflowbutler
Copy link
Member

Nagging Assignee @girving: It has been 14 days with no activity and this issue has an assignee. Please update the label and/or status accordingly.

@girving
Copy link
Contributor Author

girving commented Aug 8, 2018

I sent a PR fixing this two weeks ago, so nagged me is unnecessary. Hopefully the PR will be reviewed at some point.

girving added a commit to girving/tensorflow that referenced this issue Aug 9, 2018
@girving
Two improvements to tf.split's shape function
88ba64b 
1. Don't break if the size corresponding to a -1 is 0.
2. Infer the size corresponding to a -1 if possible.

Fixes tensorflow#19360.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@girving girving

Labels
type:bug Bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@girving @karmel @tensorflowbutler