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

Improve shape function of tf.sparse_concat when inputs are fully defined #24018

Merged

Conversation

yongtang
Copy link
Member

This fix tries to address the issue raised in #21964 where the shape function of tf.sparse_concat always returns TensorShape([Dimension(None), Dimension(None)]) even if the inputs are fully defined.

This fix addresses the issue by finding the shape if the inputs are fully defined.

This fix fixes #21964.

Signed-off-by: Yong Tang yong.tang.github@outlook.com

This fix tries to address the issue raised in 21964 where
the shape function of tf.sparse_concat always returns
`TensorShape([Dimension(None), Dimension(None)])` even
if the inputs are fully defined.

This fix addresses the issue by finding the shape if the inputs
are fully defined.

This fix fixes 21964.

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
@ymodak ymodak self-assigned this Nov 28, 2018
@ymodak ymodak requested a review from ebrevdo November 28, 2018 18:41
@ymodak ymodak added the awaiting review Pull request awaiting review label Nov 28, 2018
@dksb dksb added this to Assigned Reviewer / Pending Review in PR Queue Dec 21, 2018
@rthadur rthadur added the size:S CL Change Size: Small label Dec 27, 2018
@ymodak ymodak added the prtype:bugfix PR to fix a bug label Feb 11, 2019
@rthadur rthadur assigned rthadur and unassigned ymodak Apr 12, 2019
@tensorflowbutler
Copy link
Member

Nagging Reviewer @ebrevdo: You have been added as a reviewer to this pull request. Please add your review or reassign. It has been 29 days with no activity and the awaiting review label has been applied.

@rthadur rthadur requested review from alextp and removed request for ebrevdo May 16, 2019 20:59
Copy link
Contributor

@alextp alextp left a comment

Choose a reason for hiding this comment

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

Looks great! Just a minor nit

dim = sum(shape[axis] for shape in shapes_value)
output_shape = shapes_value[0]
output_shape[axis] = dim
output_shape = tensor_shape.as_shape(output_shape)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think output_shape is supposed to be a tensor and not a tensorshape

@alextp alextp added the kokoro:force-run Tests on submitted change label May 16, 2019
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label May 16, 2019
Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
@rthadur rthadur requested a review from alextp May 17, 2019 02:51
@tensorflowbutler tensorflowbutler removed the awaiting review Pull request awaiting review label May 17, 2019
@tensorflow-bot tensorflow-bot bot added kokoro:force-run Tests on submitted change ready to pull PR ready for merge process labels May 17, 2019
PR Queue automation moved this from Assigned Reviewer to Approved by Reviewer May 17, 2019
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label May 17, 2019
@tensorflow-copybara tensorflow-copybara merged commit d604b44 into tensorflow:master May 17, 2019
PR Queue automation moved this from Approved by Reviewer to Merged May 17, 2019
tensorflow-copybara pushed a commit that referenced this pull request May 17, 2019
@yongtang yongtang deleted the 21964-sparse.concat-shape branch May 17, 2019 20:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes prtype:bugfix PR to fix a bug ready to pull PR ready for merge process size:S CL Change Size: Small
Projects
PR Queue
  
Merged
Development

Successfully merging this pull request may close these issues.

tf.sparse_concat does not infer result shape from inputs with fully defined shape
8 participants