Repository navigation
make Dimension be compatible with integer - #12513
Conversation
|
Can one of the admins verify this patch? |
|
cc @yaroslavvb @pmccarter @martinwicke , who I believe are interested in this PR. Could you take a look please? |
facaiy
changed the title
| TypeError: If dims cannot be converted to a list of dimensions. | ||
| """ | ||
| # TODO(irving): Eliminate the single integer special case. | ||
| from tensorflow.python.util import compat |
There was a problem hiding this comment.
imports should be at the top of the file in tensorflow
There was a problem hiding this comment.
tensor_shape and compat are depended by each other if imports are at the top. I don't know how to deal with the circular imports.
| import numpy as _np | ||
| import six as _six | ||
|
|
||
| from tensorflow.python.framework import tensor_shape |
There was a problem hiding this comment.
I don't like this dependency. util should be small utilities which do not depend on any of the concrete tensorflow stuff.
I think the other fix is better.
There was a problem hiding this comment.
Thanks for comment. Could you please give me a bit more detail about where to fix?
How about hack def _FilterInt(v): function?
or add a if branch at _AssertCompatible(values, dtype)?
There was a problem hiding this comment.
Essentially, I do not think that this approach is the right way to fix the problem. compat.as_int shouldn't know anything about dimension. I think something like the change made in #12558 is the right way to go.
I'll close this pull request now.
There was a problem hiding this comment.
Hi, @alextp . Perhaps the new commit will be interesting for you. Unfortunately, the PR has been closed.
There was a problem hiding this comment.
How about #12127 ? It is more straightforward.
|
Can one of the admins verify this patch? |
| } | ||
| } | ||
| summary: "Reshapes a tensor." | ||
| description: "Given `tensor`, this operation returns a tensor that has the same values\nas `tensor` with shape `shape`.\n\nIf one component of `shape` is the special value -1, the size of that dimension\nis computed so that the total size remains constant. In particular, a `shape`\nof `[-1]` flattens into 1-D. At most one component of `shape` can be -1.\n\nIf `shape` is 1-D or higher, then the operation returns a tensor with shape\n`shape` filled with the values of `tensor`. In this case, the number of elements\nimplied by `shape` must be the same as the number of elements in `tensor`.\n\nFor example:\n\n```\n# tensor \'t\' is [1, 2, 3, 4, 5, 6, 7, 8, 9]\n# tensor \'t\' has shape [9]\nreshape(t, [3, 3]) ==> [[1, 2, 3],\n [4, 5, 6],\n [7, 8, 9]]\n\n# tensor \'t\' is [[[1, 1], [2, 2]],\n# [[3, 3], [4, 4]]]\n# tensor \'t\' has shape [2, 2, 2]\nreshape(t, [2, 4]) ==> [[1, 1, 2, 2],\n [3, 3, 4, 4]]\n\n# tensor \'t\' is [[[1, 1, 1],\n# [2, 2, 2]],\n# [[3, 3, 3],\n# [4, 4, 4]],\n# [[5, 5, 5],\n# [6, 6, 6]]]\n# tensor \'t\' has shape [3, 2, 3]\n# pass \'[-1]\' to flatten \'t\'\nreshape(t, [-1]) ==> [1, 1, 1, 2, 2, 2, 3, 3, 3, 4, 4, 4, 5, 5, 5, 6, 6, 6]\n\n# -1 can also be used to infer the shape\n\n# -1 is inferred to be 9:\nreshape(t, [2, -1]) ==> [[1, 1, 1, 2, 2, 2, 3, 3, 3],\n [4, 4, 4, 5, 5, 5, 6, 6, 6]]\n# -1 is inferred to be 2:\nreshape(t, [-1, 9]) ==> [[1, 1, 1, 2, 2, 2, 3, 3, 3],\n [4, 4, 4, 5, 5, 5, 6, 6, 6]]\n# -1 is inferred to be 3:\nreshape(t, [ 2, -1, 3]) ==> [[[1, 1, 1],\n [2, 2, 2],\n [3, 3, 3]],\n [[4, 4, 4],\n [5, 5, 5],\n [6, 6, 6]]]\n\n# tensor \'t\' is [7]\n# shape `[]` reshapes to a scalar\nreshape(t, []) ==> 7\n```" |
There was a problem hiding this comment.
Revert this change? This is a generated file.
|
I am ready to approve this when the change to ops.pbtxt gets reverted. |
|
Jenkins, test this please. |
This reverts commit 689174a.
|
Jenkins, test this please. |
|
Thanks. The new commits are modified on my personal laptop. It is no surprise if some unit tests are broken. If so, I'll fix them on next Monday. |
|
It seems a trivial mistake, and has been fixed. Could you retest the PR? Thanks. |
|
Jenkins, test this please. |
|
Thanks, @alextp . All tests pass. |
5 participants
The PR is another solution to fix #11974 . I believe that it is better than #12127 .
What changes were proposed in this pull request?
Make Dimension to be compatible with integer, so
[1, Dimension(2)]will be casted to[1, 2]automatically.How was this patch tested?