-
Notifications
You must be signed in to change notification settings - Fork 130
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
Add test for optimize_out_slice_nd #543
base: master
Are you sure you want to change the base?
Changes from all commits
d3bf1ed
7669f71
e729446
f66ed78
4749b27
4ab5c4e
fd8d4f7
7ab7c35
32fbe4c
db5938a
1f32c1e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3633,44 +3633,29 @@ def windowed_nd(source, window_size, window_left=None, window_right=None, | |
|
||
def slice_nd(x, start, size): | ||
""" | ||
:param tf.Tensor x: shape (B, T, ...) | ||
:param tf.Tensor start: shape (B,), int32 | ||
:param int|tf.Tensor size: scalar | ||
:return: [x[start_1:size], x[start_2:size], ..., x[start_B:size]], shape (B, size, ...) | ||
Like :func:`slice_pad_zeros`, the size in the first axis will always be ``size``, | ||
and we will pad with zeros. | ||
This is a more generic slice function, where arbitrary many common axis between x and start are allowed. | ||
Here we assume that x and start have their axis layed in the same order. | ||
|
||
:param tf.Tensor x: shape (B,T1,...,Tn,D,..) | ||
:param tf.Tensor start: shape (B,T1,..,Tn-1), int32 which automatically indicates n as the slice-axis | ||
:param int|tf.Tensor size: scalar in the range [0..Tn) -1] | ||
:return: ret[b, t1, .., tn-1, 0..size, :] = x[b, t1, .., tn-1, start[B, t1, .., tn-1]+0..size, :] | ||
In case the slices go out of bounds of the slice dimension and we will pad with zeros. | ||
:rtype: tf.Tensor | ||
""" | ||
with tf.name_scope("slice_nd"): | ||
shape = get_shape(x) | ||
n_batch = shape[0] | ||
|
||
batch_idxs = expand_dims_unbroadcast(tf.range(n_batch), 1, size) # (n_batch, size) | ||
batch_idxs = tf.reshape(batch_idxs, (-1,)) # (n_batch*size,) | ||
|
||
window_pos = tf.expand_dims(start, 1) + tf.range(size)[None, :] # (n_batch, size) | ||
window_pos = tf.reshape(window_pos, (-1,)) # (n_batch*size,) | ||
|
||
# build mask for zero-padding | ||
mask = tf.logical_or( | ||
tf.greater(window_pos, shape[1] - 1), tf.less(window_pos, 0)) # (n_batch*size,) tf.bool | ||
|
||
# clip indices so that gather_nd doesn't fail, will zero-pad later | ||
clip_time_idx = tf.clip_by_value(window_pos, 0, shape[1] - 1) | ||
indices = tf.stack([batch_idxs, clip_time_idx]) # (n_batch*size, 2) | ||
indices = tf.transpose(indices) # (2, n_batch*size) | ||
|
||
slices = tf.gather_nd(x, indices) # (n_batch*size, ...) | ||
|
||
# (B, size, ...), we assume time-axis is/was 1 | ||
new_shape = [shape[0], size] + shape[2:] | ||
|
||
# zero-pad | ||
mask_bc = expand_multiple_dims(mask, [-1] * (slices.get_shape().ndims - 1)) | ||
slices = where_bc(mask_bc, tf.zeros_like(slices), slices) | ||
|
||
slices = tf.reshape(slices, new_shape) # (B, size, ...) | ||
return slices | ||
len_common_dims = len(start.shape) # nr of common dims | ||
slice_dim = shape[len_common_dims] # dim of axis to be sliced | ||
# assert size < slice_dim, "Slice size cannot be bigger than the dimension to be sliced." | ||
# Create indexes for the slices where slice_idx[B,T1 .., Tn-1] = start[B,T1 .., Tn-1] + range(size) | ||
# (B,T1 .., Tn-1, size) | ||
slice_idx = tf.tile(tf.expand_dims(start, -1), [1] * len_common_dims + [size]) + tf.range(size) | ||
mask = tf.logical_or(tf.greater(slice_idx, slice_dim - 1), tf.less(slice_idx, 0)) # (B,T1 .., Tn-1, size) | ||
slice_idx = tf.clip_by_value(slice_idx, 0, slice_dim - 1) # cliped slice idx | ||
res = tf.gather(x, slice_idx, axis=len_common_dims, batch_dims=len_common_dims) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think I'm not sure anymore which is the min TF version we want to support (I think we documented this somewhere; can someone check? @patrick-wilken ? @JackTemaki ?). Maybe we need our own wrapper for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
res = where_bc(mask, tf.zeros_like(res), res) # zero-padding | ||
return res | ||
|
||
|
||
def global_tensor(f, name): | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is wrong.
size
is just a scalar. But you need a vector of shape [B] here. What's wrong with the old code?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We cannot use
start
directly to calculate the size as it has shape[B,T0,..]
instead of[B,]
.So I am not sure how to set
self.output.size_placeholder
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the meaning of
self.output.size_placeholder
if we have more than 1 spatial axis? Should any element ofself.output.size_placeholder
have size(B,)
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
size_placeholder
is a dict, mapping each spatial axis (counted without batch dim) to such[B]
tensor.So if you have a tensor
[B,T1,T2,F]
, then for each batch entryb
you find it's length of axisT1
insize_placeholder[0][b]
, and the one forT2
insize_placeholder[1][b]
.However with this it's not possible that lengths depend on anything else then the batch entry, i.e. you cannot model that the some batch entry should have a different length in axis
T2
for different time stepst1
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is what we have here, though.
What is
self.output.size_placeholder
used for anyways? Does it cause any problems if set wrong?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
size_placeholder
is mostly used for masking, e.g which entries to ignore when calculating the loss, or reducing a sequence (e.g. take the average/min/max over the sequence), or also for layers which concat in time and such.Many layers consider this, if this is set wrong then weird things can happen and your calculations will be wrong and depend on batching.
So yes, I'd say setting this is kind of important.
I don't know a way to set it how you want it. Maybe @albertz knows more?