text:
dynamic_rnn_decoder initial commit

In the raw_rnn @ebrevdo mentions that raw_rnn would be suitable to build a dynamic decoder with.
This is a dynamic decoder based on the raw_rnn as specified above.
Discussion points:

(Critical) The PR makes use of functools, is this allowed according to your standards?
(Critical) The PR makes use of contrib.layers, is this allowed according to the ops folder standards?
(Matter of taste) The PR could make use of _extract_argmax_and_embed, but the decoder_fn makes it more customizable.
(Matter of taste) The PR requires decoder inputs while validating to access the start-of-sequence symbol (could be an issue for another PR later).

Can one of the admins verify this patch?

Please note that this is related to the seq2seq_loss optimization from this PR.

Hi alrojo. The dynamic decoder looks great, thanks for making it! It would be great to have at least some unit tests for it, could you add some to seq2seq_test in kernel_tests? Thanks!

@lukaszkaiser Thanks for your fast response, I can look into the testing part, sure.
By kernel_test do you mean this? And do you have any specific guidelines or standards that I should follow, or should I just try and make something similar to what is already in the seq2seq_test.py file?

Yes, that's exactly the test I meant. But after Eugene's comment on the other issue, I think it might be even better if you moved the code to a new tf.contrib.seq2seq module and made a test for it there. Would that be ok with you? If not, we can put it in seq2seq and then we'll move it. But it seems to make sense to have a new module where the input is always a tensor, never a list of tensors, so we can build a consistent API. Does it sound good to you?

Hi alrojo. How is this PR now that the skeleton for seq2seq is there? Are you ready to rebase in contrib so I can review and we can merge this? Thanks again!

Hi lukasz,
I will update the PR with test cases for the sequence loss and rnn decoder in the new contrib folder this Saturday/Sunday.

Hi alrojo,
Thanks for contributing! I'm starting to try your code on some real translation data (so there will be more comments).
For now, the first thing I noticed is perhaps to set the default value of encoder_projection to None & have check like:
if encoder_projection:
state = encoder_projection(initial_state, cell.output_size)
else:
state = initial_state
(Maybe you can throw some error if the size doesn't match).
-Thang

Thanks for your feedback @ebrevdo, I will look into it.

Lukasz and I discussed the existing API.  Here's how we think it could be improved.  First, remove the embedding stuff and make inputs optional.  sequence_length may be provided iff inputs is.  If inputs is provided, decoding is allowed to run up to the number of frames in inputs and no longer than that (regarldess of what decoder_fn says).  If inputs is not provided, decoder_fn must say when to stop for each minibatch entry.
There should be separate decoder_fn impls for training and for inference; see below for details on that:

change the decoder_fn to have the following signature (up to you what you call the arguments and the outputs, of course; these are just placeholders)

decoder_fn(time : scalar int,
  cur_state : tensor_tuple [batch_size, ...],
  cur_output : tensor_tuple [batch_size, ...],
  cur_input tensor_tuple [batch_size, ...],
  decoder_state : tensor_tuple [anything])
->
(done : bool vec [batch_size],
 next_state : tensor_tuple [batch_size, ...],
 next_input : tensor_tuple [batch_size, ...],
 next_decoder_state : tensor_tuple[anything])

here tensor_tuple means a (possibly nested) tuple of tensors or single tensor; and, e.g., next_state must have the same nested structure and shape as cur_state (same for next_decoder_state); and next_input should have consistent structure and shape across all calls.
here is how decoder_fn gets called:
training time (inputs provided), first call:
  cur_state: None
  cur_output: None
  cur_input: **Not None** (first input)
  decoder_state: None
training time (inputs provided), subsequent calls:
  cur_state: not None
  cur_output: not None
  cur_input: not None (first input)
  decoder_state: previous emitted decoder state

inference time (**NO** inputs provided), first call:
  cur_state: None
  cur_output: None
  cur_input: **None**
  decoder_state: None
inference time (inputs provided), subsequent calls:
  cur_state: not None
  cur_output: not None
  cur_input: **None**
  decoder_state: previous emitted decoder state

here's are the two simplest decoder_fns (one for training, one for inference):
simplest case, training:
  1. if cur_state: None, cur_output: None, cur_input: not None, decoder_state: None
   return
     next_state: your initial state
     next_input: cur_input
     done = [False] * batch_size
     next_decoder_state: None
  2. if cur_state: not None, cur_output: not None, cur_input: not None, decoder_state: None
    return
      next_state = cur_state
      next_input: cur_input
      done: [False] * batch_size
      next_decoder_state: None

simplest case, inference:
  1. if cur_state: None, cur_output: None, cur_input: None, decoder_state: None
    return
       next_state: your initial state
       next input: your go symbol input
       done: [False] * batch_size
       next_decoder_state: None
   2. if cur_state: not None, cur_output: not None, cur_input: None, decoder_state: None
     return
       next_state: cur_state
       next_input: your choice, e.g. embed of argmax of cur_output
       done: if argmax of cur output is done symbol
       next_decoder_state: None

unit tests should contain these two decoder_fns.

I have started writing up the decoder_fn and I have a question @ebrevdo @lukaszkaiser :
def decoder_fn(...)
Args:
`time` is current timestep, denoted `t`
`cur_state` is the previous decoder state denoted `s_{t-1}`
`cur_output` is ?? (same as `cur_state` or `y_{t-1}`?)
`cur_input` is the input `x_t`
`decoder_state` is a, possibly tuple, structure to store information between
  timesteps (usable for beam search, a simple decoder_fn will not use it).

Returns:
`done` is a boolean vector used by raw_rnn on when to halt computation and
  copy states
`next_state` is the next decoder state denoted `s_t`
`next_input` is the next input denoted `x_{t+1}`
`next_decoder_state` is a, possibly tuple, structure to store information
  between timesteps (usable for beam search, a simple decoder_fn will not use it).

What is the purpose of cur_output? Especially as we do not return a next_output?
Thanks

Also, regarding cur_input as an argument to the decoder_fn. The raw_rnn api does not provide a cur_input as an argument to the loop_fn function. But I don't think we need it anyway in the decoder_fn. I will try and code something up.

I think of it as follows: assume in step {t-1} we got (_, old_state, cur_input, context) = decoder_fn(...). So we have our old_state and cur_input and we run the cell, so: cur_output, cur_state = RNNCell(cur_input, old_state). Now we again run decoder_fn on (time, cur_state, cur_output, cur_input, context).
Note that cur_state is actually after the RNNCell -- I think that's the only reasonable way (if you want the previous one, you can transport it in context). So decoder_fn gets access to everything the Cell produces, so it doesn't need to run it. Is that reasonable?

Yes and cur_input you provide by reading from the input TensorArray.
On Oct 19, 2016 4:46 PM, "Lukasz Kaiser" notifications@github.com wrote:

I think of it as follows: assume in step {t-1} we got (_, old_state,
cur_input, context) = decoder_fn(...). So we have our old_state and
cur_input and we run the cell, so: cur_output, cur_state =
RNNCell(cur_input, old_state). Now we again run decoder_fn on (time,
cur_state, cur_output, cur_input, context).
Note that cur_state is actually after the RNNCell -- I think that's the
only reasonable way (if you want the previous one, you can transport it in
context). So decoder_fn gets access to everything the Cell produces, so it
doesn't need to run it. Is that reasonable?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/tensorflow/tensorflow/pull/4686#issuecomment-254972317,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABtim4d4jJzR63M16YLixpk_ojPBNZfYks5q1qvPgaJpZM4KLMiD
.

I have tried coding the decoder_fn and avoiding users handling tensor_arrays. However, I kept coming back to the decoder_fn essentially being equal to the loop_fn and the rnn_decoder just being a wrapper for loop_fn.
Instead (or perhaps as an extension to the decoder_fn), I made rnn_decoder into a class and modularized all tasks in the loop_fn call so the user can provide custom functions for new_state, new_input, new_output, new_loop_state, all with access to self allowing calls to input_ta and similar if needed. This will allow the user to only have to change the "modules" they need when injecting new code. E.g. I only had to change the input_fn (a few lines of code) when going from training to evaluating. And without having tested it, I also think that you only need to change input_fn for attention support.
My hopes is that this will achieve the equivalent of what our decoder_fn is described to do, but with even less code.
As a result, the decoder_train and decoder_eval are now separated (you call them with different input_fn) and I made an input_fn with the eos and embedding loop_up for early stopping.
I just finished running the code with no bugs ½ hour ago, so I won't be able to submit a clean commit tonight, but I will be at Google tomorrow by 10 to code with @lmthang. If you want more descriptions, see the code or want to discuss feel free to come by.

Dear Alex -- if decoder_rnn becomes a simple function, that's good, right? That's the best about good abstractions, if they solve the problem and are simple, even better! Let's never add complexity for complexity's sake.
I'm not convinced about your grouping of functions. For example, you enforce that state_fn only takes state. But for beam search, it'll have to have access to the context of the beam to re-group the states.
How about doing the "easy" version in this PR and putting it in. We can see then if it's too little and re-do if needed. I'd really prefer such incremental approach to getting in a larger new thing and only later seeing the problems.

FYI, I worked with @alrojo last Friday & was able to run his rnn code on real translation data (which produces similar results as the dynamic_rnn). I have also proposed several simplifications to the code. So, looking forward to a new and cleaner version from @alrojo soon!

Yes please revert to the changes we suggested. We prefer a functional style
over complicated class heirarchies.
On Oct 21, 2016 12:16 AM, "Lukasz Kaiser" notifications@github.com wrote:

Dear Alex -- if decoder_rnn becomes a simple function, that's good, right?
That's the best about good abstractions, if they solve the problem and are
simple, even better! Let's never add complexity for complexity's sake.
I'm not convinced about your grouping of functions. For example, you
enforce that state_fn only takes state. But for beam search, it'll have to
have access to the context of the beam to re-group the states.
How about doing the "easy" version in this PR and putting it in. We can
see then if it's too little and re-do if needed. I'd really prefer such
incremental approach to getting in a larger new thing and only later seeing
the problems.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/tensorflow/tensorflow/pull/4686#issuecomment-255311260,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABtim6dRCeBiRwcdVUK5hK5eNVceAoBEks5q2GbCgaJpZM4KLMiD
.

Hi Alexander,
Thanks again for working on this! Looking forward to the new API we
discussed yesterday (I can quickly test it today if you have some initial
version ready).
-Thang
On Wed, Oct 19, 2016 at 5:44 PM ebrevdo notifications@github.com wrote:
Yes and cur_input you provide by reading from the input TensorArray.
On Oct 19, 2016 4:46 PM, "Lukasz Kaiser" notifications@github.com wrote:

I think of it as follows: assume in step {t-1} we got (_, old_state,
cur_input, context) = decoder_fn(...). So we have our old_state and
cur_input and we run the cell, so: cur_output, cur_state =
RNNCell(cur_input, old_state). Now we again run decoder_fn on (time,
cur_state, cur_output, cur_input, context).
Note that cur_state is actually after the RNNCell -- I think that's the
only reasonable way (if you want the previous one, you can transport it in
context). So decoder_fn gets access to everything the Cell produces, so it
doesn't need to run it. Is that reasonable?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<https://github.com/tensorflow/tensorflow/pull/4686#issuecomment-254972317
,
or mute the thread
<
https://github.com/notifications/unsubscribe-auth/ABtim4d4jJzR63M16YLixpk_ojPBNZfYks5q1qvPgaJpZM4KLMiD
.

—
You are receiving this because you were assigned.
Reply to this email directly, view it on GitHub
https://github.com/tensorflow/tensorflow/pull/4686#issuecomment-254980175,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAYNRXgI4YJ7yz3sP8vekzzSijsr5mdqks5q1rlbgaJpZM4KLMiD
.

I will make a push (without attention, this might take until the weekend as I am currently traveling) that should work, including all of our wish-list changes in a few hours. It will be without official TestCase style kernel_tests however.
Three comments:
@ebrevdo : We need to make rnn_decoder a class instance in order to maximize user-friendliness (went through this with @lmthang this Friday). The reason is we need to access class variables when writing user-friendly decoder_fn's.
@lmthang : I don't think we can have sequence_length be None, this could cause the decoder to get struck and eventually run OOM. Instead I have made it possible to just supply an integer value instead, representing the "maximal_allowed" amount of steps to decoder.
Do you have any specific wishes for the kernel_tests?

It was super nice meeting the team, thanks for having me!
@lmthang you can try to run the code form the new commit, should support inputs=None. Made an extensive docstring for the rnn_decoder on how to use it.

we had discussed a clean API on friday; but this seems like a big diversion from that.

and

why does decoder_fn need self?

From previous answer:

I have tried coding the decoder_fn and avoiding users handling tensor_arrays. However, I kept coming back to the decoder_fn essentially being equal to the loop_fn and the rnn_decoder just being a wrapper for loop_fn.

Instead (or perhaps as an extension to the decoder_fn), I made rnn_decoder into a class and modularized all tasks in the loop_fn call so the user can provide custom functions for new_state, new_input, new_output, new_loop_state, all with access to self allowing calls to input_ta and similar if needed. This will allow the user to only have to change the "modules" they need when injecting new code. E.g. I only had to change the input_fn (a few lines of code) when going from training to evaluating. And without having tested it, I also think that you only need to change input_fn for attention support.

My hopes is that this will achieve the equivalent of what our decoder_fn is described to do, but with even less code.

As a result, the decoder_train and decoder_eval are now separated (you call them with different input_fn) and I made an input_fn with the eos and embedding loop_up for early stopping.

I just finished running the code with no bugs ½ hour ago, so I won't be able to submit a clean commit tonight, but I will be at Google tomorrow by 10 to code with @lmthang. If you want more descriptions, see the code or want to discuss feel free to come by.

EDIT: here is the loop_fn from the "new" rnn_decoder:

    def loop_fn(time, cell_output, cell_state, loop_state):
      elements_finished = (time >= self.sequence_length) #TODO handle seq_len=None
      # get s_t, y_t
      emit_output = cell_output
      if cell_output is None:
        next_cell_state = self.state
      else:
        next_cell_state = self.state_fn(self, cell_state)
      # get x_{t+1}
      next_input, elements_finished = self.inputs_fn(self, time, next_cell_state, elements_finished)
      # get loop_state
      next_loop_state = self.loop_fn(self, loop_state)
      return (elements_finished, next_input, next_cell_state,
              emit_output, next_loop_state)

Then given Lukasz Answer:

I'm not convinced about your grouping of functions. For example, you enforce that state_fn only takes state. But for beam search, it'll have to have access to the context of the beam to re-group the states.

I changed the interface to take the entire loop_fn instead (named  decoder_fn) with access to self. This essentially makes the rnn_decoder a wrapper for raw_rnn to handle things like [this] (https://github.com/alrojo/tensorflow/blob/dynamic_decoder/tensorflow/contrib/seq2seq/python/ops/seq2seq.py#L258). The reason for access to self is to get access to some of the processed variables in rnn_decoder, such as: inputs_ta, sequence_length, input_depth, batch_size, dtype, state, time_major (will be used for attention) and future variables.

decoder_fn should not know anything about inputs_ta

Often the main difference between training, evaluating, attention is to switch the input part. For this we need access to inputs_ta, see here.

variables are accessible via vs.get_variable(); there's no need to store them anywhere

and

batch_size can be inferred from the input shapes.

Alternatively to using classes and self to store information, I guess we could either allow rnn_decoder to pass a **kwargs argument or store all of the above in tf.Variable's and then use vs.get_variable();? (I might need to read a bit more about using vs.get_variable for the latter).
Please let me know what you think on the current architecture of having rnn_decoder as a wrapper for raw_rnn with decoder_fn supplying the loop_fn and if yes, if we should use a **kwargs architecture or tf.Variable architecture to send variables in-between the rnn_decoder and decoder_fn instead of using classes and self.

@ebrevdo: maybe you can help @alrojo  (and perhaps me) here. If it seems too complicated to talk through github, I'll meet you offline.
Maybe, let's look at the two functions loop_fn_train (line 39) and loop_fn (for eval, line 87) in this file. Are these good starting points to talk?

@ebrevdo: maybe you can help @alrojo (and perhaps me) here. If it seems too complicated to talk through github, I'll meet you offline.

Maybe, let's look at the two functions loop_fn_train (line 39) and loop_fn (for eval, line 87) in this file. Are these good starting points to talk?

I will be on standby with further development of the kernel_tests and the decoder_fn_attention until we have agreed on an interface between the rnn_decoder and decoder_fn then.
I am also available on skype/hangout/appear.in today and from Friday onwards in the morning hours (as I will be gmt +1 from Friday).

@lmthang and i will meet tomorrow and discuss the changes.

Hi @alrojo,
I worked with @lukaszkaiser and @ebrevdo. It's a bit tricky to get things right in terms of the API we discussed last time. But I finally have some concrete realization of the API for you to look at (this is only for training, see decoder_fn_train at the end). Writing decoder_fn_eval will be more interesting, which the below code does give some hints. Maybe you can take a look to see if the below code make sense and if you can help write decoder_fn_eval.
Thanks!
-Thang
def dynamic_decoder(cell, decoder_fn, inputs=None,                     sequence_lengths=None,                     parallel_iterations=None,                     swap_memory=False, time_major=False, scope=None):   if inputs is not None:     # Testing input dimensions     if len(inputs.get_shape()) is not 3:       raise ValueError("Inputs must have three dimensions")     if inputs.get_shape()[2] is None:       raise ValueError("Inputs must not be None` in the feature (3'rd) "
"dimension")
# Setup of RNN (dimensions, sizes, length, initial state, dtype)
if not time_major:
# [batch, seq, features] -> [seq, batch, features]
inputs = array_ops.transpose(inputs, perm=[1, 0, 2])
dtype = inputs.dtype
# Get data input information
input_depth = int(inputs.get_shape()[2])
max_time = array_ops.shape(inputs)[0]
# Setup decoder inputs as TensorArray
inputs_ta = tensor_array_ops.TensorArray(dtype, size=0, dynamic_size=True)
inputs_ta = inputs_ta.unpack(inputs)

def loop_fn(time, cell_output, cell_state, loop_state):
if cell_state == None: # first call, before while loop (in raw_rnn)
assert cell_output is None
assert loop_state is None
context_state = None
else: # subsequent calls, inside while loop, after cell excution
if isinstance(loop_state, tuple):
(done, context_state) = loop_state
else:
done = loop_state
context_state = None
# call decoder function
if inputs is not None: # training
  # get next_cell_input
  if cell_state == None:
    next_cell_input = inputs_ta.read(0)
  else:
    batch_size = array_ops.shape(done)[0]
    next_cell_input = control_flow_ops.cond(
        tf.equal(time, max_time),
        lambda: array_ops.zeros([batch_size, input_depth], dtype=dtype),
        lambda: inputs_ta.read(time))
  (next_done, next_cell_state, _, emit_output, next_context_state) = decoder_fn(cell_state, cell_output, context_state)
else: # inference
  # next_cell_input is obtained through decoder_fn
  (next_done, next_cell_state, next_cell_input, emit_output, next_context_state) = decoder_fn(cell_state, cell_output, context_state)

# check if we are done
if next_done is None: # training
  next_done = time >= sequence_lengths

# build next_loop_state
if next_context_state is None:
  next_loop_state = next_done
else:
  next_loop_state = (next_done, next_context_state)

return (next_done, next_cell_input, next_cell_state,
        emit_output, next_loop_state)

Run raw_rnn function
outputs_ta, state, _ = (
rnn.raw_rnn(cell, loop_fn,
parallel_iterations=parallel_iterations,
swap_memory=swap_memory, scope=scope))
outputs = outputs_ta.pack()
outputs = tf.Print(outputs, ["shape: ", tf.shape(outputs)])
if not time_major:
# [seq, batch, features] -> [batch, seq, features]
outputs = array_ops.transpose(outputs, perm=[1, 0, 2])
return outputs, state
here's how we use it
def decoder_fn_train(cell_state, cell_output, context_state):
return (done, cell_state, cell_input, cell_output, context_state)
For training, we don't decide the done status and the next
cell_input
if cell_state == None: # first call, return encoder_state
return (None, encoder_state, None, cell_output, context_state)
else:
return (None, cell_state, None, cell_output, context_state)
outputs, state = dynamic_decoder(cell, decoder_fn_train, inputs=emb_inp,
sequence_lengths=decoder_lengths,
swap_memory=True, time_major=True)
`

Nice, great work!
You might want to consider having the decoder_fn as classes to allow user-specific parameters to be passed.
E.g. the code provided in codeshare link would not work if you had not defined encoder_state somewhere above.
Below is decoder_fn_train and decoder_fn_eval, we should consider also providing an abstract class like the RNNCell.
class decoder_fn_train():

  def __init__(self, encoder_state):
    self.encoder_state = encoder_state

  def __call__(self, cell_state, cell_output, context_state):
    if cell_state == None: # first call, return encoder_state
      return (None, self.encoder_state, None, cell_output, context_state)
    else:
      return (None, cell_state, None, cell_output, context_state)

class decoder_fn_eval():

  def __init__(self, encoder_state, embeddings, sos_id, eos_id,
               output_fn=None, dtype=tf.int32):
    self.encoder_state = encoder_state
    self.embeddings = embeddings
    self.dtype = dtype
    self.output_fn = output_fn
    self.sos_id = tf.Variable(sos_id, dtype=self.dtype)
    self.eos_id = tf.Variable(eos_id, dtype=self.dtype)

    self.batch_size = array_ops.shape(self.encoder_state)[0]
    if self.output_fn is None:
      self.output_fn = lambda x: x

  def __call__(self, cell_state, cell_output, context_state):

    if cell_output == None:
      # invariant that this is time == 0
      next_input_id = tf.ones([self.batch_size], dtype=self.dtype) * self.sos_id
      done = tf.zeros([self.batch_size,], dtype=tf.bool)
      cell_state = self.encoder_state
    else:
      next_input_id = tf.argmax(self.output_fn(cell_output), 1)
      done = None#tf.equal(next_input_id, self.eos_id)
    next_input = tf.gather(self.embeddings, next_input_id)

    return (done, cell_state, next_input, cell_output, context_state)

One note:
As pointed in earlier post regarding allowing sequence_lengths=None:

I don't think we can have sequence_length be None, this could cause the decoder to get stuck and eventually run OOM. Instead I have made it possible to just supply an integer value, representing the "maximal_allowed" amount of steps to decoder.

By allowing sequence_lengths = None we rely on the decoder_fn_eval to eventually return True for all sequences.
Imagine validating with an untrained network on a word-to-word model with 80k classes.
Then you would have a 1 in 80k chance of "randomly" reaching the eos symbol.

Regarding attention, we would need to "concat" something extra to the input.
We can do this by either adding the input to the decoder_fn or by reformulating the following:
    # call decoder function
    if inputs is not None: # training
      # get next_cell_input
      if cell_state == None:
        next_cell_input = inputs_ta.read(0)
      else:
        batch_size = array_ops.shape(done)[0]
        next_cell_input = control_flow_ops.cond(
            tf.equal(time, max_time),
            lambda: array_ops.zeros([batch_size, input_depth], dtype=dtype),
            lambda: inputs_ta.read(time))
      (next_done, next_cell_state, _, emit_output, next_context_state) = decoder_fn(cell_state, cell_output, context_state)

into
    # call decoder function
    if inputs is not None: # training
      # get next_cell_input
      if cell_state == None:
        next_cell_input = inputs_ta.read(0)
      else:
        batch_size = array_ops.shape(done)[0]
        next_cell_input = control_flow_ops.cond(
            tf.equal(time, max_time),
            lambda: array_ops.zeros([batch_size, input_depth], dtype=dtype),
            lambda: inputs_ta.read(time))
      (next_done, next_cell_state, additional_input, emit_output, next_context_state) = decoder_fn(cell_state, cell_output, context_state)
      if additional_input is not None:
          # concat along feature dimension
          tf.concat(concat_dim=-1, [next_cell_input, additional_input])

You don't need a class, just a function that accepts the encoder_state and
creates an appropriate decoder_fn with the encoder state available via
closure. That creator function can return the new decoder_fn to the user.
On Oct 29, 2016 11:04 AM, "Alexander Rosenberg Johansen" <
notifications@github.com> wrote:

Regarding attention, we would need to "concat" something extra to the
input.
We can do this by either adding the input to the decoder_fn or by
reformulating the following:
# call decoder function
if inputs is not None: # training
  # get next_cell_input
  if cell_state == None:
    next_cell_input = inputs_ta.read(0)
  else:
    batch_size = array_ops.shape(done)[0]
    next_cell_input = control_flow_ops.cond(
        tf.equal(time, max_time),
        lambda: array_ops.zeros([batch_size, input_depth], dtype=dtype),
        lambda: inputs_ta.read(time))
  (next_done, next_cell_state, _, emit_output, next_context_state) = decoder_fn(cell_state, cell_output, context_state)

into
# call decoder function
if inputs is not None: # training
  # get next_cell_input
  if cell_state == None:
    next_cell_input = inputs_ta.read(0)
  else:
    batch_size = array_ops.shape(done)[0]
    next_cell_input = control_flow_ops.cond(
        tf.equal(time, max_time),
        lambda: array_ops.zeros([batch_size, input_depth], dtype=dtype),
        lambda: inputs_ta.read(time))
  (next_done, next_cell_state, additional_input, emit_output, next_context_state) = decoder_fn(cell_state, cell_output, context_state)
  if additional_input is not None:
      # concat along feature dimension
      tf.concat(concat_dim=-1, [next_cell_input, additional_input])

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/tensorflow/tensorflow/pull/4686#issuecomment-257106172,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABtim5DkMDEtNGiCBWvmGMSXTDEYByb0ks5q44mrgaJpZM4KLMiD
.

Hi @alrojo,
Thanks for the update! Let's use functions instead of classes as @ebrevdo suggested (see below - you did something similar before).
def decoder_fn_train(encoder_state):   def decoder_fn(cell_state, cell_output, context_state):     if cell_state == None: # first call, return encoder_state       return (None, encoder_state, None, cell_output, context_state)     else:       return (None, cell_state, None, cell_output,context_state)
return decoder_fn
`
I agree about the max_len at inference, which I think you can pass to the wrapper as below:
`
def decoder_fn_eval(encoder_state, embeddings, sos_id, eos_id, max_len):
def decoder_fn(time, cell_state, cell_output, context_state):
return decoder_fn
I guess we need to add back thetime` variable to decoder_fn (so that you can compare with max_len), which means you need to do for decoder_fn_train (but not using).
Let's not use output_fn for now to keep things simple & let worry about attention later.
If these sound good, maybe you can put up somethings so that @ebrevdo can check and I can test on real data. Hopefully, we can approve the PR soon!
-T/hang

@lmthang @ebrevdo I have updated the code for the PR accordingly to your comments.
Given we handle the done vector in the decoder_fn for evaluation, we might want to consider doing the same for training, so supplying sequence_lengths in decoder_fn_train instead of this.
Also, which name do you prefer for the decoder? dynamic_decoder, rnn_decoder or something else?

I agree it seems cleaner to provide sequence_lengths to the decoder_fn instead of having dynamic_decoder do it.  However, it also makes the dynamic_decoder that much more complicated (and it's already quite complicated).  What do you think?  I'm open on this one; happy to either have the dynamic_decoder take care of it when inputs is not None, but also happy to let the decoder_fn take care of it.

Is this ready to review?

@vrv: will let you know soon!
@alrojo @ebrevdo: I'd propose to keep sequence_lengths in dynamic_decoder so users don't have to bother handling sequence_lengths in decoder_fn_train. I find it natural (and logically clear) to have (inputs, decoder_lengths) go in pair: they both have to be not None at training time & both need to be None at inference time. We should make sure we enforce those constraints, Alexander.
As Alexander asked, shall we standardize names as below?
dynamic_rnn_decoder
decoder_fn_train
decoder_fn_inference
(Of course, feel free to prepend the names of decoder_fn like you did Alexander like simple_decoder_fn_train, simple_decoder_fn_inference)
-T

I like the name of the decoder.  I'm wondering how everyone feels about
having decoder_fn_{train,inference} be wrapped in a single function that
returns the correct one depending on a python bool is_training?
On Mon, Oct 31, 2016 at 12:57 PM, Thang Luong notifications@github.com
wrote:

@vrv https://github.com/vrv: will let you know soon!
@alrojo https://github.com/alrojo @ebrevdo https://github.com/ebrevdo:
I'd propose to keep sequence_lengths in dynamic_decoder so users don't have
to bother handling sequence_lengths in decoder_fn_train. I find it natural
(and logically clear) to have (inputs, decoder_lengths) go in pair: they
both have to be not None at training time & both need to be None at
inference time. We should make sure we enforce those constraints, Alexander.
As Alexander asked, shall we standardize names as below?
dynamic_rnn_decoder
decoder_fn_train
decoder_fn_inference
(Of course, feel free to prepend the names of decoder_fn like you did
Alexander like simple_decoder_fn_train, simple_decoder_fn_inference)
-T
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/tensorflow/tensorflow/pull/4686#issuecomment-257402462,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABtimznOy42cXfEK6FrAZt25NK0mJ7lkks5q5kgRgaJpZM4KLMiD
.

I'm very happy with the recent changes, but I'd prefer not to wrap too much the different decoder_fn's at this point. They are only in tests for now, right? Let's make this PR as simple as possible and get it in.

Yes, I think it's better to keep decode_fn_{train,inference} as separated functions for now since they take different arguments.

SGTM for keeping them separate functions.  still not sure i understand why
the train function doesn't accept an embeddings dict but the inference one
does?
On Mon, Oct 31, 2016 at 1:17 PM, Thang Luong notifications@github.com
wrote:

Yes, I think it's better to keep decode_fn_{train,inference} as separated
functions for now since they take different arguments.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/tensorflow/tensorflow/pull/4686#issuecomment-257408136,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABtimwEdbK7cbw4Uda-WYdLRvytv7rqFks5q5kzxgaJpZM4KLMiD
.

@ebrevdo: good question! It might be worth thinking (maybe later).
Right now, at training time (when inputs is not None), users can pass whatever inputs they want (embedding lookup vectors or any vectors, etc). Our simple_decoder_fn_inference happens to be a specific decoder_fn that uses argmax for cell_output to get discrete words & then do embedding lookup from the argmax results (so we made an implicit assumption that at training time, inputs passed to the dynamic_rnn_decoder was looked up from the same embeddings matrix).
From that point of view, I think our current dynamic_rnn_decoder seems fine. Those decoder_fn_{train, inference} are just examples of a specific usage.

Okay, I assume we are all good on the code now and I will start writing docstrings followed by kernel_tests.

@alrojo: One thing about the maximum_length, I was wondering we should remove this line maximum_length = array_ops.constant(maximum_length, dtype=dtype)' in the decoder_fn_interference? The reason is users might want to have the maximum_length determined at run time, e.g., twice as long as the maximum encoder lengths tf.round(2 * tf.reduce_max(encoder_lengths))`.

While I was writing the tests, I found that our current code does not support explicit batch_size because the batch_size is inferred dynamically. Thus the input loses shape during training.
Please see commit if you agree with fix-up @ebrevdo .

I have commited an initial draft of the seq2seq_test, please have a look @ebrevdo . Thanks.

I want to confirm that I've tested the argmax decoder on a real dataset and it produces sensible results. Hopefully this PR gets done soon!

I will make another big commit for Thursday. Thanks for your patience :)
The current architecture is dependent on the output projection wrapper. If we remove it, we need to reinstate the output_fn.
The value comparison for the test case is a little complicated as we don't know the embeddings chosen by the random initialized decoder under inference. But maybe if we switch the order and do inference first. I will try it out.

For the training decoder, you don't need output_fn do you?  You just run
the training decoder to get some values, and then matmul those to get your
final output (when training).  No?  Am I wrong?
On Tue, Nov 8, 2016 at 3:16 PM, Alexander Rosenberg Johansen <
notifications@github.com> wrote:

I will make another big commit for Thursday. Thanks for your patience :)
The current architecture is dependent on the output projection wrapper. If
we remove it, we need to reinstate the output_fn.
The value comparison for the test case is a little complicated as we don't
know the embeddings chosen by the random initialized decoder under
inference. But maybe if we switch the order and do inference first. I will
try it out.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/tensorflow/tensorflow/pull/4686#issuecomment-259290141,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABtim8kR6Tcudw0XDngT6JvD9zOmB4W6ks5q8QLmgaJpZM4KLMiD
.

i.e., you do:
outputs, ... = dynamic_rnn_decoder(cell, training_fn, inputs)
projected_outputs = tf.matmul(outputs, embedding_matrix)
On Tue, Nov 8, 2016 at 3:23 PM, Eugene Brevdo ebrevdo@gmail.com wrote:

For the training decoder, you don't need output_fn do you?  You just run
the training decoder to get some values, and then matmul those to get your
final output (when training).  No?  Am I wrong?
On Tue, Nov 8, 2016 at 3:16 PM, Alexander Rosenberg Johansen <
notifications@github.com> wrote:

I will make another big commit for Thursday. Thanks for your patience :)
The current architecture is dependent on the output projection wrapper.
If we remove it, we need to reinstate the output_fn.
The value comparison for the test case is a little complicated as we
don't know the embeddings chosen by the random initialized decoder under
inference. But maybe if we switch the order and do inference first. I will
try it out.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/tensorflow/tensorflow/pull/4686#issuecomment-259290141,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABtim8kR6Tcudw0XDngT6JvD9zOmB4W6ks5q8QLmgaJpZM4KLMiD
.

Yes, it is only needed for inference.

For inference you can just put the projection into the decoder_fn right?
On Tue, Nov 8, 2016 at 4:28 PM, Alexander Rosenberg Johansen <
notifications@github.com> wrote:

Yes, it is only needed for inference.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/tensorflow/tensorflow/pull/4686#issuecomment-259302889,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABtimwElH6qsmj2B7T7CxSHPxEQYqMpsks5q8ROvgaJpZM4KLMiD
.

Hi @alrojo,
To elaborate, here's a concrete piece of code that works:
logits = layers.linear(cell_output, num_decoder_symbols, scope=scope)
next_input_id = math_ops.cast(math_ops.argmax(logits, 1), dtype=dtype)

to replace the following line in simple_decoder_fn_inference:
next_input_id = math_ops.cast(math_ops.argmax(cell_output, 1), dtype=dtype)

Note that layers is from contrib.layers.python.layers.

Hi @lmthang and @ebrevdo
I have updated the seq2seq_test using output_fn as a projection instead of the cell wrapper.
Please note I also made some minor changes to the simple_decoder_fn_inference function in two separate commits.

To elaborate, here's a concrete piece of code that works:

logits = layers.linear(cell_output, num_decoder_symbols, scope=scope)
next_input_id = math_ops.cast(math_ops.argmax(logits, 1), dtype=dtype)
to replace the following line in simple_decoder_fn_inference:

next_input_id = math_ops.cast(math_ops.argmax(cell_output, 1), dtype=dtype)
Note that layers is from contrib.layers.python.layers

Sure, I can put that in instead of the output_fn.
Question:
How would I train the projection? I assume it should be accessible both while training and validating. Perhaps take example in the newly updated seq2seq_test?
Also, I am available on Skype at github_alrojo if you are interested in doing some pair programming! :)

Hi @alrojo,
At training time, you can also do something similar (replacing lines 78-83 in your seq2seq_test):
outputs = layers.linear(outputs, num_decoder_symbols, scope=decoder_scope)

Make sure you pass in the same scope for both training and testing.
Does that help?

So instead of passing the projection function explicitly, we use the scopes to share weights with a projection matrix?
I can try and make that work, yes.
Regarding

(replacing lines 78-83 in your seq2seq_test):

outputs = layers.linear(outputs, num_decoder_symbols, scope=decoder_scope)

As of my understanding, linear, which is based on fully_connected uses flatten which together works like
[seq_len, batch_size, features] -> [seq_len, batch_size*features] -> [seq_len, num_out]
While we want
[seq_len, batch_size, features] -> [seq_len*batch_size, features] -> [seq_len*batch_size, num_output] -> [seq_len, batch_size, num_output]
So it is not straight forward, should I make a wrapper for this? Perhaps call it tf.contrib.seq2seq.linear_seq?

Hi @alrojo,
On the question of whether we use function or scope to share the projection matrix, perhaps @ebrevdo or @lukaszkaiser can better answer? I'm using scope in my own testing code right now.
On your comments about layers.linear, sorry, I'm not sure if I follow. I thought the code does what we want: It first reshape the 3D tensor to [-1, last_dimension] here, do the transformation, and put back into the original 3D shape here.
-Thang

On your comments about layers.linear, sorry, I'm not sure if I follow. I thought the code does what we want: It first reshape the 3D tensor to [-1, last_dimension] here, do the transformation, and put back into the original 3D shape here.

@lmthang You are completely right, my apologies!

As for scopes vs function, I'm fine with both, I think scope is slightly easier but do just any way that works for you.

One more thing I'd like to add: we should add back the cell_input argument to decoder_fn because attention will need it.
More concretely, in dynamic_decoder_rnn, we do something like:
if inputs is not None: # training
  ...
  (next_done, next_cell_state, next_cell_input, emit_output, next_context_state) = decoder_fn(time, cell_state, next_cell_input, cell_output, context_state)
else: # inference
  (next_done, next_cell_state, next_cell_input, emit_output, next_context_state) = decoder_fn(time, cell_state, None, cell_output, context_state)

simple_decoder_fn_train will just simply return cell_input. simple_decoder_fn_inference expects that cell_input is None.
def simple_decoder_fn_train(encoder_state):
  def decoder_fn(time, cell_state, cell_input, cell_output, context_state):
    if cell_state == None: # first call, return encoder_state
      return (None, encoder_state, cell_input, cell_output, context_state)
    else:
      return (None, cell_state, cell_input, cell_output, context_state)
  return decoder_fn

Hi @alrojo,
Continuing from my earlier code, one more thing you should add to make the simple_decoder_fn_inference correct:
if cell_output == None:
  ...
  cell_output = array_ops.zeros(num_decoder_symbols, dtype=dtypes.float32)
else:
  ...
  cell_output = logits

This part array_ops.zeros(num_decoder_symbols, dtype=dtypes.float32) is a temporary hack so that raw_rnn can get the right output size. Also, we ideally want to return the word indices beside the logits l but this is slightly tricky to do we need to change raw_rnn. We (=TF team) will address these issues later once this PR is approved.
-Thang

I added the next_input.

Continuing from my earlier code

Okay, I will implement that tomorrow.

Hi @lmthang
The layers.linear is now used instead.
The cell_output now uses the logits projected.
Note: I had to add an extra argument, num_decoder_symbols, to the simple_decoder_fn_inference.
Please let me know if you have more code blocks that you would like to change.
Furthermore, I assume the only major challenge left (before writing docstrings and building) is the equality test mentioned by @ebrevdo ? (please see latest "requested changes").

@alrojo, the changes look great to me! thanks for your help!
-Thang

You can also flatten the state using nest.flatten and get the batch size
from the first entry.
On Nov 11, 2016 10:30 PM, "Thang Luong" notifications@github.com wrote:
@alrojo https://github.com/alrojo: one more thing, the way we obtain the
batch_size from encoder_state in simple_decoder_fn_inference is not correct
when encoder_state is a tuple, e.g., when using BasicLSTMCell. Perhaps, we
can pass batch_size as another argument to that simple_decoder_fn_inference?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/tensorflow/tensorflow/pull/4686#issuecomment-260105241,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABtimzx49HzZ9xnSrVu03FFgFLKxhj5wks5q9Vz8gaJpZM4KLMiD
.

@lmthang @ebrevdo we now have support for BasicLSTMCell. However, to make it work I had to include this hack to the init of the decoder functions.

That's fine.  We'll clean this up after you push.
On Mon, Nov 14, 2016 at 7:51 AM, Alexander Rosenberg Johansen <
notifications@github.com> wrote:

@lmthang https://github.com/lmthang @ebrevdo
https://github.com/ebrevdo we now have support for BasicLSTMCell.
However, to make it work I had to include this hack
https://github.com/alrojo/tensorflow/blob/dynamic_decoder/tensorflow/contrib/seq2seq/python/ops/decoder_fn.py#L36
to the init of the decoder functions.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/tensorflow/tensorflow/pull/4686#issuecomment-260372976,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABtimwFQiYxEPZT98UUeZnFljT9Wjri8ks5q-IN0gaJpZM4KLMiD
.

Let's keep it as is for now.
On Mon, Nov 14, 2016 at 9:06 AM, Alexander Rosenberg Johansen <
notifications@github.com> wrote:

@alrojo commented on this pull request.
In tensorflow/contrib/seq2seq/python/kernel_tests/seq2seq_test.py
https://github.com/tensorflow/tensorflow/pull/4686:

       [decoder_outputs_train, decoder_state_train])

   decoder_outputs_inference_res, decoder_state_inference_res = sess.run(

       [decoder_outputs_inference, decoder_state_inference])

   # Assert outputs

   self.assertEqual((decoder_sequence_length, batch_size, output_size),

                    decoder_outputs_train_res.shape)

   self.assertEqual((batch_size, decoder_size),

                    decoder_outputs_inference_res.shape[0:2])

   self.assertEqual((batch_size, decoder_size),

                    decoder_state_train_res.shape)

   self.assertEqual((batch_size, decoder_size),

                    decoder_state_inference_res.shape)

   # The dynamic decoder might end earlier than `maximal_length`

   # under inference

   true_value = (decoder_sequence_length>=

@ebrevdo https://github.com/ebrevdo would you like to keep kernel_tests
as is, or modify dynamic_rnn_decoder to also return the sequence_lengths
in order to support value assertion? (as elaborated above)
Thanks
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/tensorflow/tensorflow/pull/4686, or mute the thread
https://github.com/notifications/unsubscribe-auth/ABtim2d7TV0hovsRCRLQkSftxFydiKl6ks5q-JUCgaJpZM4KLMiD
.

@alrojo Can you rebase, please?

@rmlarsen done.
I assume the code is finalized now. Do you want me to continue with making docstrings? In such case I plan on having it ready by the end of the week.

Hi @lmthang @ebrevdo @lukaszkaiser
I have pushed the docstrings.

Hello, I have tested the latest commit of this pull request, and find that an error occurs when using with MultiRNNCell of LSTMCell.
A single-layer LSTMCell seems to work fine (I've tested the LSTMCell with state_is_tuple option), however when I wrap the cell with MultiRNNCell, the following exception is raised.
ValueError: The two structures don't have the same number of elements.
First structure: Tensor("Train/Model/simple_decoder_fn_train/packed:0", shape=(4, 2, ?, 1024), dtype=float32),
second structure: (LSTMStateTuple(c=1024, h=1024), LSTMStateTuple(c=1024, h=1024), LSTMStateTuple(c=1024, h=1024), LSTMStateTuple(c=1024, h=1024)).

Hi @jihunchoi , thanks for pointing this out!
Yes, it's because that tf.convert_to_tensor changes the RNNCell to a tensor. I assume the reason why we use tf.convert_to_tensor was because the user might supply a numpy matrix instead? (@ebrevdo?)
I thought about your solution as well, but I found it overengineering the data structures from the RNNCell. Instead I removed the tf.convert_to_tensor. Your code should run just fine now.

Let's get this change in and we can fix it afterwards.
On Fri, Nov 18, 2016 at 1:14 PM, Alexander Rosenberg Johansen <
notifications@github.com> wrote:

Hi @jihunchoi https://github.com/jihunchoi , thanks for pointing this
out!
Yes, it's because that tf.convert_to_tensor changes the RNNCell to a
tensor. I assume the reason why we use tf.convert_to_tensor was because
the user might supply a numpy matrix instead? (@ebrevdo
https://github.com/ebrevdo?)
I thought about your solution as well, but I found it overengineering the
data structures from the RNNCell. Instead I removed the
tf.convert_to_tensor. Your code should run just fine now.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/tensorflow/tensorflow/pull/4686#issuecomment-261643391,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABtimzc-t1kjhoSHC_J1-05YloDrLDmdks5q_hUsgaJpZM4KLMiD
.

Hi @lukaszkaiser @ebrevdo @lmthang ,
Unless you have more change request, the code is ready to be tested and submitted.

@tensorflow-jenkins test this please.

@ebrevdo could you perhaps help me dissect the Windows Cmake Tests failures from @tensorflow-jenkins ? Thanks

It looks like the errors are not associated with the code in this PR? @ebrevdo

Yeah you can ignore those errors.
On Thu, Nov 24, 2016 at 9:53 AM, Alexander Rosenberg Johansen <
notifications@github.com> wrote:

It looks like the errors are not associated with the code in this PR?
@ebrevdo https://github.com/ebrevdo
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/tensorflow/tensorflow/pull/4686#issuecomment-262825378,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABtim00416XQKoTFpKMG9m8ISKpk-LVvks5rBc8xgaJpZM4KLMiD
.

Failures are unrelated.  OK to merge.

@tensorflow-jenkins test this please.

@ebrevdo I have redesigned at tested with the Linux CPU build, it seems to work now.
On specifics of implementation:

this leads to an error:
name, "".join(traceback.format_list(tb))))
ValueError: Variable root/rnn/gru_cell/gates/weights already exists, disallowed. Did you mean to set reuse=True in VarScope? Originally defined at:
due to a change i recently pushed. this is because you're using the "rnn" scope twice. what you should do is:
with tf.variable_scope("rnn") as scope:
tf.nn.dynamic_rnn(..., scope=scope)
scope.set_reuse()
... = tf.contrib.seq2seq.dynamic_rnn_decoder(..., scope=scope)
...
... = tf.contrib.seq2seq.dynamic_rnn_decoder(..., scope=scope)

I did the following instead:
with tf.variable_scope("rnn") as scope:
  tf.nn.dynamic_rnn(..., scope=scope)

with tf.variable_scope("decoder") as scope:
  ... = tf.contrib.seq2seq.dynamic_rnn_decoder(..., scope=scope)
  ...
  scope.reuse_variables()
  ...
  ... = tf.contrib.seq2seq.dynamic_rnn_decoder(..., scope=scope)

@tensorflow-jenkins Test this please

Awesome ..! Thanks for a superb collaboration @lukaszkaiser @ebrevdo @lmthang :)

Thanks for your help @alrojo! Much appreciated. Stay tune for more changes that will really make training seq2seq easier.

authors: alrojo danmane ebrevdo jihunchoi lmthang lukaszkaiser rmlarsen tensorflow-jenkins vrv


text:
[NNAPI]call memcpy() with null pointer dereference.

create initialize.tflite with model_personalization tflite-transfer-convert.
execute delegate with the following option.tflite::StatefulNnApiDelegate::Options options;
options.accelerator_name = "nnapi-reference"; // use CPU
auto delegate = new tflite::StatefulNnApiDelegate(options);

fatal error has occurred.signal 11 (SIGSEGV), code 1 (SEGV_MAPERR), fault addr 0x0
Cause: null pointer dereference

nn_input_memory_->get_data_ptr() returns nullptr.

total_input_byte_size == 0

I suspect that just adding the check would not make the model run well through NNAPI delegate. We'd need to figure out the root cause on the nn_input_memory_ is not properly updated.

source: gharchive/pull-request

repo: tensorflow/tensorflow

license_type: permissive

license_source: github-api


text:
Update eigen_backward_spatial_convolutions.h

Replaced backward spatial convolution with faster and a bit more streamlined implementation based on TF 1.13.1, with some minor changes. Really shines at larger input/output Tensor sizes (128x128 and larger, batch agnostic) with at least 8 input and 8 output channels. It is upto 30% faster than the current method with 64 input/output channels, or 25% faster with 32 in_ch/out_ch .
It is sadly 2x slower on small number of channels (4 in_ch/out_ch and smaller) on my machine. Tested on Clang 16.

This code doesn't actually compile with the latest version of tensorflow - there's some kind of type mismatch that causes the convolution kernels to fail to build.

authors: Panjaksli cantonios


text:
Prepare for building NPM package.

This change is

Reviewed 4 of 4 files at r1.
Review status: 3 of 4 files reviewed at latest revision, all discussions resolved, some commit checks failed.

Comments from Reviewable

authors: dsmilkov nkreeger

repo: tensorflow/tfjs-node

license_source: bigquery


text:
Unable to install additional features

Getting error when installing additional features
For "pip install git+https://github.com/notadamking/tensortrade.git[tf,tensorforce,baselines,ccxt,fbm]"
I get the error:
"Collecting git+https://github.com/notadamking/tensortrade.git[tf,tensorforce,baselines,ccxt,fbm]
Cloning https://github.com/notadamking/tensortrade.git[tf,tensorforce,baselines,ccxt,fbm] to /tmp/pip-req-build-yinma2e8
Running command git clone -q 'https://github.com/notadamking/tensortrade.git[tf,tensorforce,baselines,ccxt,fbm]' /tmp/pip-req-build-yinma2e8
fatal: unable to access 'https://github.com/notadamking/tensortrade.git[tf,tensorforce,baselines,ccxt,fbm]/': The requested URL returned error: 400
ERROR: Command errored out with exit status 128: git clone -q 'https://github.com/notadamking/tensortrade.git[tf,tensorforce,baselines,ccxt,fbm]' /tmp/pip-req-build-yinma2e8 Check the logs for full command output."
And for the command "pip install tensortrade[tf,tensorforce,baselines,ccxt,fbm]"
I get the following
"WARNING: tensortrade 1.0.4.dev0 does not provide the extra 'baselines'
WARNING: tensortrade 1.0.4.dev0 does not provide the extra 'ccxt'
WARNING: tensortrade 1.0.4.dev0 does not provide the extra 'fbm'
WARNING: tensortrade 1.0.4.dev0 does not provide the extra 'tensorforce'
WARNING: tensortrade 1.0.4.dev0 does not provide the extra 'tf'"

Can you tell me which document are you following? It's most certainly an obsolete one, and would like to mark it as such in some way.

https://tensortradex.readthedocs.io/en/latest/overview/getting_started.html#installation

Damn... thanks. Will fix that :)

Please be aware that you are referring to the documentation for a fork of the project, and not the official one (that you can find on tensortrade.org). In any case those were old references to the way TensorTrade package was splitted up in different components.
The "vanilla" pip install now takes care of everything you need.
Closing as fixed

source: gharchive/issue

authors: carlogrisetti tshiamotman

repo: tensortrade-org/tensortrade


text:
Quick fix for substring check

Motivation and Context
Quick fix for substring check to fix #49
Description
Substring check for dataset name dataset traversal now checks for a substring that ends with : which prevents from accidentally including other datasets that have the same name in the start of the datasets name
eg:
MNIST and MNISTLabel
Types of changes
What types of changes does your code introduce? Put an x in all the boxes that apply:

[x] Bug fix (non-breaking change which fixes an issue)
[ ] New feature (non-breaking change which adds functionality)
[ ] Breaking change (fix or feature that would cause existing functionality to change)

Is this PR ready for review, or a work in progress?

[ ] Ready for review
[x] Work in progress

How Has This Been Tested?
Put an x in the boxes that apply:

[ ] Current tests cover modifications made
[ ] New tests have been added to the test suite
[ ] Modifications were made to existing tests to support these changes
[ ] Tests may be needed, but they are not included when the PR was proposed
[ ] I don't know. Help!

Checklist:

[x] My code follows the code style of this project.
[ ] My change requires a change to the documentation.
[ ] I have updated the documentation accordingly.
[x] I have read the CONTRIBUTING document.
[x] I have signed (or will sign when prompted) the tensorwork CLA.
[ ] I have added tests to cover my changes.
[ ] All new and existing tests passed.

@rlizzo
This is a quick fix for the issue #49. If this solution is good enough, I'll go ahead and add test cases for the same before merging the PR. Just wanted to make sure there are no logical changes on the way that rewrites those functions entirely.

@rlizzo I changed the code so that SEP replaces :. One reason I found not to edit the dataset_record_count_db_key_from_raw_key function is, all of our *_db_key_from_raw_key has a similar structure -> returns the key without any separator. Changing that might bite us back later. What do you say? Anyway, this should fix our current issue.

awesome. this works for me! merging now!

authors: hhsecond rlizzo

repo: tensorwerk/hangar-py


text:
tt_lib.tensor.complex_recip operation fails with low PCC error [Wormhole]

Describe the bug
tt_lib.tensor.complex_recip operation breaks with low PCC error in some test cases.
To Reproduce
Steps to reproduce the behavior:

Run unit test test_complex_recip.py  using this command:
pytest tests/tt_eager/python_api_testing/non_working_unit_tests/wormhole/test_complex_recip.py

Expected behavior
There are four test cases presented in the unit test tests/tt_eager/python_api_testing/non_working_unit_tests/wormhole/test_complex_recip.py and they all are expected to fail with low PCC error.

Two of test cases are breaking with error like this:
RuntimeError: TT_ASSERT @ tt_metal/impl/device/device.cpp:328: (logical_core.x < logical_grid_size.x) and (logical_core.y < logical_grid_size.y) info:       Bounds-Error -- Logical_core={} is outside of logical_grid_size={}       (x=8,y=4)       (x=8,y=9)

And the other two with:
Max ATOL Delta: nan, Max RTOL Delta: nan, PCC: 0.03849241070222418, PCC check failed

Note
Unit test for complex_real for wormhole has the same code as unit test for GS, so to avoid code duplication we use the same unit to replicate errors on WH.
Getting Additional info for the operation under test and its behavior
To get additional information and results for different combinations of input shapes, types, layouts and memory configs for which this operation was tested you can also run locally sweeps for tt_lib.tensor.complex_recip and check the results. To do this you should:

Follow the Getting Started page to setup the repo, environment variables and python-env
Activate source build/python_env/bin/activate
Run sweeps by using python tests/tt_eager/python_api_testing/sweep_tests/run_pytorch_test.py -i tests/tt_eager/python_api_testing/sweep_tests/test_configs/ci_sweep_tests/broken_wormhole/pytorch_complex_recip_test.yaml -o ./result-sweeps
After the run is completed all test sweeps results should be available inside specified output directory (in this case ./result-sweeps). There you will find complex_recip_sweep.csv which holds all executed sweeps, among which you can also find the ones that failed and were recreated by the unit test, which you can get by searching unique data_seed field.

Merged to main

authors: banekg umadevimcw

repo: tenstorrent-metal/tt-metal


text:
I2S output buffer sizing and CB sizing do not always match

For TILE inputs + HEIGHT_SHARDING, I2S op does the following currently:

Sets the output buffer size equal to input shape (legacy shape) and is unaware of sharding.
Calculates CB size for output based on number of tiles per shard.
When the input shape is not a multiple of TILE height * number of shards, the two sizes are different, and leads to assertion with the CB allocation.

I think the solution should be that the size calculation for the output buffer should match CB size calculation.
Example
Branch: main
Compile in debug mode to enable the above assertion.
Test: pytest "tests/ttnn/unit_tests/operations/test_maxpool2d.py::test_run_max_pool[dtype=DataType.BFLOAT8_B-dilation=(1, 1)-stride=(2, 2)-padding=(0, 0)-kernel_size=(2, 2)-act_shape=[1, 256, 56, 56]-device_params={'l1_small_size': 24576}]"

cc: @wransom-TT

repo: tenstorrent/tt-metal


text:
Conv fails with Segmentation fault

Describe the bug
ttnn.conv2d is failing with Segmentation fault for the conv unit test of Fadnet++ model.
To Reproduce
Steps to reproduce the behavior:

Checkout the branch harini/dispnetres_predres5_unittest
Run the command: pytest tests/ttnn/unit_tests/operations/test_conv2d.py::test_dispnetres_conv
The following error will occur:
2024-06-18 11:13:52.332 | DEBUG    | tt_eager.tt_dnn.op_library.sliding_window_op_infra.tt_py_composite_conv:determine_parallel_config:203 - PARALLEL CONFIG :: True :: 512 :: 1 :: SlidingWindowOpParams(stride_h=1, stride_w=1, pad_h=1, pad_w=1, window_h=3, window_w=3, batch_size=1, input_h=30, input_w=18) :: {'act_block_h': 32} -> 17 :: [12, 2] :: 1 :: 1 Fatal Python error: Segmentation fault

Expected behavior
To run without segmentation fault.
Please complete the following environment information:

OS: [e.g. Ubuntu 20.04]
Device: GS

@HariniMohan0102
I've test this case on latest main on this branch
https://github.com/tenstorrent/tt-metal/tree/pjosipovic/test_dispnetres_conv
running pytest tests/ttnn/unit_tests/operations/test_new_conv2d.py::test_dispnetres_conv2
and it's working now.
Can we close this one?


text:
Readiness of the fixes to address #4968

These tasks are currently gating the fixes for #4968.

[ ] Syseng testing to ensure stability on n150/n300: https://tenstorrent.atlassian.net/browse/SYS-573
[ ] SW changes to apply stagger on large matmuls: https://github.com/tenstorrent/tt-metal/issues/9857
[ ] Release for n150 and n300
[ ] Syseng testing to test on Galaxy, both functional and stability
[ ] Release for Galaxy

Progress should be monitored daily.
Milos

Sync Meeting on item 1, Jul 2nd, 2024:

After extensive testing, some systems have failed and some are not. details in syseng issue
Yuqin and Tyler to work on getting the data from systems that hasn't failed by inspection and repeating tests with some probes
Dave/Yuqin to determine if LL has been tested enough before releasing and if a step back is needed in releasing LL to test more with different settings.
all details in https://tenstorrent.atlassian.net/browse/SYS-573
some testing on galaxy 4U systems worked.

Milos


text:
How to make the child physical model object rotate with the parent object

How to make the child physical model object rotate with the parent object
22
Description
When I rotate a model object, I find that the physical model of the child does not rotate, resulting in no effect of collision with another physical object
Version位

Platform

Web Version [X]

Windows [X]

Hello
Physics are not affected by the parent transforms since they are always calculated in world coordinates.
They do affect their child position so for example if a physics object has a box as a child the box with move exactly as its physics model but if the box being moved has a physics object child the physics object will keep its position.
I can possibly add a new physics mode other than the two already available that ensures that the physics body follows the tree transform it would mean that i have to apply "infinite" force sometimes on the object and this can lead to some "funny" results on the physics engine.
Thanks a lot!

Hello
Thank you very much for your answerIf ，It's been bothering me for a long time. If there's a good solution, please let me know.
Thanks a lot!

repo: tentone/nunuStudio

license: MIT


text:
Use pinned nixpkgs for nushell script deps

Without adding --inputs-from we always get the latest versions of the dependencies that are currently available in nixpkgs. Explicitly using the revision from the flake is an easy way to pin and avoid unexpected breakage.

I hope this fixes the build failure. Nushell isn't broken on nixpkgs-23.05 or nixpkgs-master - so I wonder what caused the issue though? Is it a breaking change in nushell itself?

Is it a breaking change in nushell itself?

That is correct.

repo: tenzir/tenzir


text:
Simplify pretty_type_name in vast/detail/logger.hpp

:notebook_with_decorative_cover: Description

Problem:

pretty_type_name does different behavior whether T is a pointer
type or not which is unnecessary by only removing the pointer type if
T is a pointer type.

Solution:

Always use std::remove_pointer_t<T> which will leave the T as is
for non-pointer types, thus preserving the same behavior.

:memo: Checklist

[X] All user-facing changes have changelog entries.
[X] The changes are reflected on docs.tenzir.com/vast, if necessary.
[X] The PR description contains instructions for the reviewer, if necessary.

:dart: Review Instructions

File-by-file.

For what it's worth, I'd recommend vast writes its own pretty_type_name function in terms of the output of __PRETTY_FUNCTION__. The implementation in caf is not great and uses std::type_info which makes the call sites all use typeid which isn't needed in a high quality implementation. I'd prefer an interface like
template<class T>
std::string_view pretty_type_name() {
 / / uses __PRETTY_FUNCTION__ to parse out the type name.
}

Anything against using this?
#include <string_view>

namespace type_name_probe {

/// A templated function whose whole purpose is to embed the name of an
/// explicitly specified template parameter into its signature, which the
/// function returns via platform-specific macros.
template <class T> constexpr std::string_view embed() {
#if defined(_MSC_VER)
  return __FUNCSIG__;
#else
  return __PRETTY_FUNCTION__;
#endif
}

/// A probe with a known type; used for extracting the platform-specific prefix
/// and suffix from the result of embed<T>.
constexpr auto probe = embed<double>();
constexpr auto prefix_length = probe.find("double");
constexpr auto suffix_length =
    probe.size() - prefix_length - std::string_view("double").size();

// Sanity check that the probe did contain the type name.
static_assert(prefix_length != std::string_view::npos,
              "cannot extract typename from function signature");

} // namespace type_name_probe

/// Retrieves the type name of **T** at compile time.
template <class T> constexpr std::string_view type_name() {
  auto name = type_name_probe::embed<T>();
  name.remove_prefix(type_name_probe::prefix_length);
  name.remove_suffix(type_name_probe::suffix_length);
  return name;
}

Anything against using this?
#include <string_view>

namespace type_name_probe {

/// A templated function whose whole purpose is to embed the name of an
/// explicitly specified template parameter into its signature, which the
/// function returns via platform-specific macros.
template <class T> constexpr std::string_view embed() {
#if defined(_MSC_VER)
  return __FUNCSIG__;
#else
  return __PRETTY_FUNCTION__;
#endif
}

/// A probe with a known type; used for extracting the platform-specific prefix
/// and suffix from the result of embed<T>.
constexpr auto probe = embed<double>();
constexpr auto prefix_length = probe.find("double");
constexpr auto suffix_length =
    probe.size() - prefix_length - std::string_view("double").size();

// Sanity check that the probe did contain the type name.
static_assert(prefix_length != std::string_view::npos,
              "cannot extract typename from function signature");

} // namespace type_name_probe

/// Retrieves the type name of **T** at compile time.
template <class T> constexpr std::string_view type_name() {
  auto name = type_name_probe::embed<T>();
  name.remove_prefix(type_name_probe::prefix_length);
  name.remove_suffix(type_name_probe::suffix_length);
  return name;
}

Seems reasonable, albeit not the one I carry around in my toolbox. If we want to switch to a better type_name, I'd recommend we do that as a follow-up PR and fix all the call sites. I'd want some solid tests on our implementation to see if it works with types in inline namespaces, lambdas, type aliases, etc.

I normally use the implementation from https://stackoverflow.com/a/56766138.

I normally use the implementation from https://stackoverflow.com/a/56766138.

The only difference being that that has the prefixes and suffixes hardcoded, while the solution I posted calculates them once at compile time.

I'd recommend we do that as a follow-up PR and fix all the call sites.

Fine by me, although I don't think there's much to test—it really is only being used in log outputs, mostly error messages when a conversion fails.

authors: JoeLoser dominiklohmann tobim

repo: tenzir/vast


text:
can't deploy from ghcr.io

distrobox create -i ghcr.io/tepene/ublue-box -n ublue-box
Image ghcr.io/tepene/ublue-box not found.
Do you want to pull the image now? [Y/n]:
Trying to pull ghcr.io/tepene/ublue-box:latest...
Error: initializing source docker://ghcr.io/tepene/ublue-box:latest: Requesting bearer token: invalid status code from registry 403 (Forbidden)

url is for the image is ghcr.io/tepene/bayou-box:latest.  It's wrong in the README

repo: tepene/box-bayou


text:
Zig Bindings

Hello! I have created zig bindings for termbox2 at https://sr.ht/~kolunmi/termbox2-zig/
Please let me know if anything is incorrect. Thank you for this amazing project!

Fantastic work. I don't have much Zig experience but it all looks good to me. I'll add it to the README. Thank you.

authors: adsr kolunmi


text:
Real Time video

Once trained, is it possible to use this NN with real-time videos? Thanks in advance

I did not do it, but I think it is absolutely possible.

authors: paolaforte ternaus

repo: ternaus/robot-surgery-segmentation


text:
P558 The test already in code for Date serialization can be put in there

The date test exist on the source, you could put it in there.

Adding, thanks

authors: Foxandxss teropa

repo: teropa/build-your-own-angularjs

license: mit


text:
Support for hashicorp/aws provider version >=4

Description
Support for hashicorp/aws provider version >=4, i'd like to update my environment but i cannot as long as this great module requires version 3
Describe the solution you'd like.
required_providers {
aws = {
source  = "hashicorp/aws"
version = ">= 4"
}

this module doesn't require v3, it sets a minimum required version but users are able to use v4 https://github.com/terraform-aws-modules/terraform-aws-rds-aurora/blob/a7a8bf111f6c1eeb4af0c5dfe53d65d05fc54e6c/versions.tf#L7

oh my apologies, thank you!


text:
Add types for logging input variable

Is your request related to a new offering from AWS?
No
Is your request related to a problem? Please describe.
When working with the logging variable, it would be nice to get type autocomplete and documentation in an IDE
For example, I tried this:
  logging = {
    target_bucket = module.s3_access_log_s3_bucket.s3_bucket_id
    target_object_key_format = {
      partitioned_prefix = {
        partition_date_source = "DeliveryTime"
      }
    }
  }

Only to run into the error The argument "target_prefix" is required, but no definition was found
Describe the solution you'd like.
instead of any, logging could have a type like map(object({ ... })) with the expected properties
Describe alternatives you've considered.
alternative would be to use aws_s3_bucket_logging directly, instead of this module
Additional context

it looks like all other variable objects have type input. is there a reason for this?
I'd be happy to open a PR for this if you confirm you'll accept one

commenting to remove stale. if a contributor confirms this makes sense, I will open a PR

commenting to remove stale. if a contributor confirms this makes sense, I will open a PR


text:
chore: add ci-cd workflow for pre-commit checks

Description

add ci-cd workflow for pre-commit checks; replacing ci-cd configuration files for CircleCI

Motivation and Context

provide static checks for pull requests

Breaking Changes

no

How Has This Been Tested?

copy of what is implemented in SQS module repository

@antonbabenko looks like you might have to go in and disconnect CircleCI when you get a chance

Done :) Great work on GH Actions across all repos.

authors: antonbabenko bryantbiggs

repo: terraform-aws-modules/terraform-aws-vpc


text:
Adding "idle-timeout-seconds" terraform variable in for vertex_workbench_notebook pipeline

TL;DR
Hello,
We wuld like to add new terraform variable (idle-timeout-seconds) to control idle vertex ai notebooks for vertex_workbench_notebook. Probably needs to be added into "vertex.tf"
Could you please help us ?
Thanks,
İmran
Terraform Resources
https://cloud.google.com/vertex-ai/docs/workbench/instances/idle-shutdown
https://github.com/terraform-google-modules/terraform-docs-samples/blob/HEAD/vertex_ai/workbench_idle_shutdown/main.tf

Detailed design
No response
Additional information
No response

As of 5.17.0, google_notebook_instance is deprecated, and the docs suggest using google_workbench_instance instead. As you linked, this resource already includes the idle-timeout option you are after.

authors: glasnt itopuz-clgx


text:
Added "encryption_key_name" variable for postgresql and mysql modules…

Added "encryption_key_name" variable for postgresql and mysql modules and "availability_type" variable for mysql
Closes #71

@cromaniuc Can you rebase?

@morgante rebased!

Thanks!

authors: cromaniuc morgante


text:
chore(deps): update common-dev-assets digest to 8331e9d

This PR contains the following updates:

Package
Update
Change

common-dev-assets
digest
eaefbbf -> 8331e9d

Configuration
📅 Schedule: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).
🚦 Automerge: Disabled by config. Please merge this manually once you are satisfied.
♻ Rebasing: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.
👻 Immortal: This PR will be recreated if closed unmerged. Get config help if that's undesired.

[ ] If you want to rebase/retry this PR, click this checkbox.

This PR has been generated by Renovate Bot.

:tada: This PR is included in version 1.2.19 :tada:
The release is available on GitHub release
Your semantic-release bot :package::rocket:


text:
chore(deps): update common-dev-assets digest to c599901

This PR contains the following updates:

Package
Update
Change

common-dev-assets
digest
f1d2dcd -> c599901

Configuration
📅 Schedule: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).
🚦 Automerge: Disabled by config. Please merge this manually once you are satisfied.
♻ Rebasing: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.
👻 Immortal: This PR will be recreated if closed unmerged. Get config help if that's undesired.

[ ] If you want to rebase/retry this PR, click this checkbox.

This PR has been generated by Renovate Bot.

:tada: This PR is included in version 1.0.16 :tada:
The release is available on GitHub release
Your semantic-release bot :package::rocket:


text:
chore(deps): update common-dev-assets digest to 593e4fb

This PR contains the following updates:

Package
Update
Change

common-dev-assets
digest
d8a1b79 -> 593e4fb

Configuration
📅 Schedule: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).
🚦 Automerge: Disabled by config. Please merge this manually once you are satisfied.
♻ Rebasing: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.
👻 Immortal: This PR will be recreated if closed unmerged. Get config help if that's undesired.

[ ] If you want to rebase/retry this PR, check this box

This PR has been generated by Renovate Bot.

/run pipeline

:tada: This PR is included in version 3.3.4 :tada:
The release is available on GitHub release
Your semantic-release bot :package::rocket:


text:
chore(deps): update common-dev-assets digest to fafc30b

This PR contains the following updates:

Package
Update
Change

common-dev-assets
digest
038ee9e -> fafc30b

Configuration
📅 Schedule: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).
🚦 Automerge: Disabled by config. Please merge this manually once you are satisfied.
♻ Rebasing: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.
👻 Immortal: This PR will be recreated if closed unmerged. Get config help if that's undesired.

[ ] If you want to rebase/retry this PR, check this box

This PR has been generated by Renovate Bot.

/run pipeline

:tada: This PR is included in version 1.4.0 :tada:
The release is available on GitHub release
Your semantic-release bot :package::rocket:

repo: terraform-ibm-modules/terraform-ibm-cis


text:
add-cbr-module

Description
add-cbr-module
Types of changes in this PR
Changes that affect the core Terraform module or submodules

[ ] Bug fix
[ ] New feature
[ ] Dependency update

Changes that don't affect the core Terraform module or submodules

[ ] Examples or tests (addition or updates of examples or tests)
[ ] Documentation update
[ ] CI-related update (pipeline, etc.)
[ ] Other

Release required?
Identify the type of release. For information about the changes in a semantic versioning release, see Release versioning.

[ ] No release
[ ] Patch release (x.x.X)
[ ] Minor release (x.X.x)
[ ] Major release (X.x.x)

Release notes content
If a release is required, replace this text with information that users need to know about the release. Write the release notes to help users understand the changes, and include information about how to update from the previous version.
Your notes help the merger write the commit message for the PR that is published in the release notes for the module.
Run the pipeline
If the CI pipeline doesn't run when you create the PR, the PR requires a user with GitHub collaborators access to run the pipeline.
Run the CI pipeline when the PR is ready for review and you expect tests to pass. Add a comment to the PR with the following text:
/run pipeline

Checklist for reviewers

[ ] If relevant, a test for the change is included or updated with this PR.
[ ] If relevant, documentation for the change is included or updated with this PR.

Merge actions for mergers

Use a relevant conventional commit message that is based on the PR contents and any release notes provided by the PR author. The commit message determines whether a new version of the module is needed, and if so, which semver increment to use (major, minor, or patch).
Merge by using "Squash and merge".

Data engine does not support CBR...?
https://cloud.ibm.com/docs/account?topic=account-context-restrictions-whatis#cbr-adopters

@Aayush-Abhyarthi Lets try and find out internally when support will be added

Data engine does not support CBR yet so closing this PR. Keep an eye on updates in https://github.com/terraform-ibm-modules/terraform-ibm-data-engine/issues/9

authors: Aayush-Abhyarthi ocofaigh


text:
chore(deps): update ci dependencies

This PR contains the following updates:

Package
Type
Update
Change

common-dev-assets

digest
d37f07b -> 55c4c41

github.com/terraform-ibm-modules/ibmcloud-terratest-wrapper
require
patch
v1.23.0 -> v1.23.2

terraform-ibm-modules/common-pipeline-assets
action
minor
v1.17.9 -> v1.18.1

Release Notes

terraform-ibm-modules/ibmcloud-terratest-wrapper (github.com/terraform-ibm-modules/ibmcloud-terratest-wrapper)
v1.23.2
Compare Source
Bug Fixes

deps: update packages to latest (#669) (6eab475)

v1.23.1
Compare Source
Bug Fixes

deps: update gomod (#654) (59b80d3)

terraform-ibm-modules/common-pipeline-assets (terraform-ibm-modules/common-pipeline-assets)
v1.18.1
Compare Source
Bug Fixes

install dependencies before running pre-commit stage (#566) (97afd7d)

v1.18.0
Compare Source
Features

added the ability pass tfswitchDir (#565) (d492025)

Configuration
📅 Schedule: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).
🚦 Automerge: Disabled by config. Please merge this manually once you are satisfied.
♻ Rebasing: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.
👻 Immortal: This PR will be recreated if closed unmerged. Get config help if that's undesired.

[ ] If you want to rebase/retry this PR, check this box

This PR has been generated by Renovate Bot.

/run pipeline

/run pipeline

:tada: This PR is included in version 1.1.2 :tada:
The release is available on GitHub release
Your semantic-release bot :package::rocket:


text:
chore(deps): update ci dependencies

This PR contains the following updates:

Package
Type
Update
Change

common-dev-assets

digest
97bdd73 -> d63f9de

github.com/terraform-ibm-modules/ibmcloud-terratest-wrapper
require
patch
v1.42.4 -> v1.42.5

Release Notes

terraform-ibm-modules/ibmcloud-terratest-wrapper (github.com/terraform-ibm-modules/ibmcloud-terratest-wrapper)
v1.42.5
Compare Source
Bug Fixes

deps: update gomod (#903) (15a5b80)

Configuration
📅 Schedule: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).
🚦 Automerge: Disabled by config. Please merge this manually once you are satisfied.
♻ Rebasing: Whenever PR is behind base branch, or you tick the rebase/retry checkbox.
👻 Immortal: This PR will be recreated if closed unmerged. Get config help if that's undesired.

[ ] If you want to rebase/retry this PR, check this box

This PR has been generated by Renovate Bot.

/run pipeline

ℹ Artifact update notice
File name: tests/go.mod
In order to perform the update(s) described in the table above, Renovate ran the go get command, which resulted in the following additional change(s):

12 additional dependencies were updated

Details:

Package
Change

github.com/IBM/go-sdk-core/v5
v5.18.1 -> v5.18.3

github.com/go-logr/logr
v1.3.0 -> v1.4.2

github.com/hashicorp/hcl/v2
v2.17.0 -> v2.22.0

github.com/hashicorp/terraform-json
v0.23.0 -> v0.24.0

github.com/tmccombs/hcl2json
v0.5.0 -> v0.6.4

github.com/zclconf/go-cty
v1.15.0 -> v1.15.1

go.opentelemetry.io/otel
v1.16.0 -> v1.29.0

go.opentelemetry.io/otel/metric
v1.16.0 -> v1.29.0

go.opentelemetry.io/otel/trace
v1.16.0 -> v1.29.0

golang.org/x/crypto
v0.30.0 -> v0.31.0

golang.org/x/mod
v0.17.0 -> v0.18.0

golang.org/x/tools
v0.21.1-0.20240508182429-e35e4ccd0d2d -> v0.22.0

/run pipeline

:tada: This PR is included in version 3.0.2 :tada:
The release is available on:

GitHub release
v3.0.2

Your semantic-release bot :package::rocket:


text:
chore(deps): update common-dev-assets digest to e9138b0

This PR contains the following updates:

Package
Update
Change

common-dev-assets
digest
703195b -> e9138b0

Configuration
📅 Schedule: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).
🚦 Automerge: Disabled by config. Please merge this manually once you are satisfied.
♻ Rebasing: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.
👻 Immortal: This PR will be recreated if closed unmerged. Get config help if that's undesired.

[ ] If you want to rebase/retry this PR, check this box

This PR has been generated by Renovate Bot.

:tada: This PR is included in version 1.1.3 :tada:
The release is available on GitHub release
Your semantic-release bot :package::rocket:


text:
chore(deps): update ci dependencies

This PR contains the following updates:

Package
Type
Update
Change

common-dev-assets

digest
2f74f8e -> e76d022

github.com/terraform-ibm-modules/ibmcloud-terratest-wrapper
require
patch
v1.25.2 -> v1.25.3

Release Notes

terraform-ibm-modules/ibmcloud-terratest-wrapper (github.com/terraform-ibm-modules/ibmcloud-terratest-wrapper)
v1.25.3
Compare Source
Bug Fixes

deps: update module golang.org/x/crypto to v0.16.0 (#706) (a18a1d9)

Configuration
📅 Schedule: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).
🚦 Automerge: Disabled by config. Please merge this manually once you are satisfied.
♻ Rebasing: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.
👻 Immortal: This PR will be recreated if closed unmerged. Get config help if that's undesired.

[ ] If you want to rebase/retry this PR, check this box

This PR has been generated by Renovate Bot.

/run pipeline


text:
chore(deps): update common-dev-assets digest to 5d664dd

This PR contains the following updates:

Package
Update
Change

common-dev-assets
digest
f02a1bb -> 5d664dd

Configuration
📅 Schedule: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).
🚦 Automerge: Disabled by config. Please merge this manually once you are satisfied.
♻ Rebasing: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.
👻 Immortal: This PR will be recreated if closed unmerged. Get config help if that's undesired.

[ ] If you want to rebase/retry this PR, click this checkbox.

This PR has been generated by Renovate Bot.

:tada: This PR is included in version 1.1.0 :tada:
The release is available on GitHub release
Your semantic-release bot :package::rocket:


text:
chore(deps): update common-dev-assets digest to 5fdee53

This PR contains the following updates:

Package
Update
Change

common-dev-assets
digest
240edb4 -> 5fdee53

Configuration
📅 Schedule: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).
🚦 Automerge: Disabled by config. Please merge this manually once you are satisfied.
♻ Rebasing: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.
👻 Immortal: This PR will be recreated if closed unmerged. Get config help if that's undesired.

[ ] If you want to rebase/retry this PR, check this box

This PR has been generated by Renovate Bot.

:tada: This PR is included in version 4.1.0 :tada:
The release is available on GitHub release
Your semantic-release bot :package::rocket:


text:
chore(deps): update common-dev-assets digest to 12e7798

This PR contains the following updates:

Package
Update
Change

common-dev-assets
digest
8685939 -> 12e7798

Configuration
📅 Schedule: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).
🚦 Automerge: Disabled by config. Please merge this manually once you are satisfied.
♻ Rebasing: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.
👻 Immortal: This PR will be recreated if closed unmerged. Get config help if that's undesired.

[ ] If you want to rebase/retry this PR, click this checkbox.

This PR has been generated by Renovate Bot.

:tada: This PR is included in version 1.13.1 :tada:
The release is available on:

GitHub release
v1.13.1

Your semantic-release bot :package::rocket:


text:
chore(deps): update terraform github.com/terraform-ibm-modules/terraform-ibm-resource-group to v1.0.3

This PR contains the following updates:

Package
Type
Update
Change

github.com/terraform-ibm-modules/terraform-ibm-resource-group
module
patch
v1.0.2 -> v1.0.3

Release Notes

terraform-ibm-modules/terraform-ibm-resource-group
v1.0.3
Compare Source
Bug Fixes

remove depends_on from resource group name output (#51) (e6e4fd4)

NOTE: This reverts the change that was made in 1.0.2 because it can cause Terraform to create more conservative plans that replace more resources than necessary.

Configuration
📅 Schedule: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).
🚦 Automerge: Disabled by config. Please merge this manually once you are satisfied.
♻ Rebasing: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.
👻 Immortal: This PR will be recreated if closed unmerged. Get config help if that's undesired.

[ ] If you want to rebase/retry this PR, click this checkbox.

This PR has been generated by Renovate Bot.

:tada: This PR is included in version 1.0.1 :tada:
The release is available on GitHub release
Your semantic-release bot :package::rocket:


text:
chore(deps): update ci dependencies

This PR contains the following updates:

Package
Type
Update
Change

github.com/terraform-ibm-modules/ibmcloud-terratest-wrapper
require
minor
v1.27.2 -> v1.28.2

terraform-ibm-modules/common-pipeline-assets
action
minor
v1.20.0 -> v1.21.0

Release Notes

terraform-ibm-modules/ibmcloud-terratest-wrapper (github.com/terraform-ibm-modules/ibmcloud-terratest-wrapper)
v1.28.2
Compare Source
Bug Fixes

deps: update gomod (#754) (2872471)

v1.28.1
Compare Source
Bug Fixes

Add Helper function and refactor tests (#752) (c3e0c1c)

v1.28.0
Compare Source
Features

add resource group print on failure (#746) (17daf42)

terraform-ibm-modules/common-pipeline-assets (terraform-ibm-modules/common-pipeline-assets)
v1.21.0
Compare Source
Features

Release pipeline now creates GIT issue and slack alert on failure (#636) (3a51e77)

Configuration
📅 Schedule: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).
🚦 Automerge: Disabled by config. Please merge this manually once you are satisfied.
♻ Rebasing: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.
👻 Immortal: This PR will be recreated if closed unmerged. Get config help if that's undesired.

[ ] If you want to rebase/retry this PR, check this box

This PR has been generated by Renovate Bot.

/run pipeline

Failed with known issue, 409 creating cluster with duplicate name; due to contention/rate limits.

/run pipeline

:tada: This PR is included in version 1.21.0 :tada:
The release is available on GitHub release
Your semantic-release bot :package::rocket:


text:
Activity tracker target variables should be marked as sensitive

There is an optional api_key in the blob.
https://github.com/terraform-ibm-modules/terraform-ibm-observability-instances/blob/f1754c67e86e98669569e325209852de61d50850/modules/activity_tracker/variables.tf#L100
api_key                    = optional(string)

PR merged


text:
OCM-6285 | feat: Day1 SG support for HCP

What this PR does / why we need it:
Allows HCP clusters to have additional compute SGs on creation, following the introduced support for day 1 HCP SGs in CS/ROSA
Which issue(s) this PR fixes (optional, use fixes #<issue_number>(, fixes #<issue_number>, ...) format, where issue_number might be a GitHub issue, or a Jira story (OCM-xxxx):
Fixes #OCM-6285
Change type

[X] New feature
[ ] Bug fix
[ ] Build
[ ] CI
[ ] Documentation
[ ] Performance
[ ] Refactor
[ ] Style
[ ] Unit tests
[X] Subsystem tests

Checklist

[X] Subject and description added to both, commit and PR.
[X] Relevant issues have been referenced.
https://issues.redhat.com/browse/OCM-6285

/test rosa-sts-private-critical-high-presubmit

/test rosa-sts-private-critical-high-presubmit

/test rosa-sts-private-critical-high-presubmit

/test rosa-sts-private-critical-high-presubmit

/retest-required

/retest-required

/retest-required

/retest-required


text:
[Bug]: Public Server crashes if you appear in the Deep Dark/Ancient City

Bug Description
I currently admin and run a public minecraft server that has almost 40 members in it. I added this mod in along with other updated mods and Resourceful Lib and optional Journey Map as I updated the server to 1.20. Got all the mods put in their proper places and booted up the newly updated server and came in by myself (since I'm admin and wanted to do a quality check before letting everybody back in) to test out the mods to make sure everything was working properly. I used this mod with ease and parsed out my entire base area (which is quite large) with zero issue. Everything was absolutely perfect and I loved the mod to bits with the ease of use. I then went into the nether where we have our nether hub to go to one of my members bases who is in the Deep Dark, specifically made themselves at home within four Ancient Cities, to test out a different unaffiliated mod. This member had a small auto chicken farm on one side, and an auto moss farm on the other (which I later turned off and turned out to not be the issue). Almost immediately upon going through the nether portal to this members base and taking a few steps into their base, the server will crash and immediately restart. I tried to come back in three more times once the server rebooted and without fail would boot me again and error out. It was quite bizarre, and I spent legitimately 14 hours scouring codes (based on the error logs) of each of the other 13 mods I also had on this server trying to figure out which one it could have been. After many hours, I finally put the server into just vanilla, find out it's definitely one of the mods, and basically just did a process of elimination with each mod. Cadmus became the last test, and even tested without Cadmus but kept Resourceful Lib in place, and concluded that Cadmus has some sort of strange bug that may just be having an issue with either one of the mods I use (which honestly is not too intense of mods) or some sort of other catastrophic failure with places like the Deep Dark itself.
I have tested this mod in other places in the Overworld too, and it seems it only errors out in the Deep Dark.
My hypothesis is that when I was updating the server, it had some strange issues during it's update where it couldn't identify wool blocks or many colours, but the server corrected itself with those blocks. I have to wonder if for some reason the mod had difficulty "reading" the naturally generated wool blocks in the Ancient City, OR because the Ancient City is well below 0 from the surface, if perhaps it has a difficulty reading to that depth of the minecraft world. Food for thought, I may be absolutely wrong, but thats what I guess 14 hours of trying to figure out why my server wasn't working has done to my very mushed up brain.
If it's helpful, these are the other mods my server was also using:
FabricAPI, Lithium, Starlight, VoiceChat, VCInteraction, Colytra, JourneyMap, Coords Display, Status
Highly doubt that the mods not playing nice is the cause but I've listed them anyway.
How to Reproduce?

Find an Ancient City
Walk or run in it
Server boots you and crashes

Expected Behavior
Booted from the server, server crashes, then reboots
Version
0.0.1
Mod Loader Version
1.20—1.20.1
Mod Loader
Fabric
Logs or additional context
This is the error message I would get every time I would walk or run around anywhere in the Ancient City:
[Server thread/ERROR]: Encountered an unexpected exception
net.minecraft.class_148: Exception ticking world
at net.minecraft.server.MinecraftServer.method_3813(MinecraftServer.java:901) ~[server-intermediary.jar:?]
at net.minecraft.class_3176.method_3813(class_3176.java:283) ~[server-intermediary.jar:?]
at net.minecraft.server.MinecraftServer.method_3748(MinecraftServer.java:824) ~[server-intermediary.jar:?]
at net.minecraft.server.MinecraftServer.method_29741(MinecraftServer.java:671) ~[server-intermediary.jar:?]
at net.minecraft.server.MinecraftServer.method_29739(MinecraftServer.java:265) ~[server-intermediary.jar:?]
at java.lang.Thread.run(Thread.java:833) ~[?:?]
Caused by: org.spongepowered.asm.mixin.injection.callback.CancellationException: The call method_7731 is not cancellable.
at org.spongepowered.asm.mixin.injection.callback.CallbackInfo.cancel(CallbackInfo.java:101) ~[sponge-mixin-0.12.5+mixin.0.8.5.jar:0.12.5+mixin.0.8.5]
at net.minecraft.class_1755.handler$zzc000$cadmus$emptyContents(class_1755.java:535) ~[server-intermediary.jar:?]
at net.minecraft.class_1755.method_7731(class_1755.java) ~[server-intermediary.jar:?]
at net.minecraft.class_2357$8.method_10135(class_2357.java:395) ~[server-intermediary.jar:?]
at net.minecraft.class_2347.dispense(class_2347.java:16) ~[server-intermediary.jar:?]
at net.minecraft.class_2315.method_10012(class_2315.java:90) ~[server-intermediary.jar:?]
at net.minecraft.class_2315.method_9588(class_2315.java:113) ~[server-intermediary.jar:?]
at net.minecraft.class_4970$class_4971.method_26192(class_4970.java:1113) ~[server-intermediary.jar:?]
at net.minecraft.class_3218.method_14189(class_3218.java:728) ~[server-intermediary.jar:?]
at net.minecraft.class_6757.method_39390(class_6757.java:215) ~[server-intermediary.jar:?]
at net.minecraft.class_6757.method_39377(class_6757.java:98) ~[server-intermediary.jar:?]
at net.minecraft.class_3218.method_18765(class_3218.java:323) ~[server-intermediary.jar:?]
at net.minecraft.server.MinecraftServer.method_3813(MinecraftServer.java:897) ~[server-intermediary.jar:?]
... 5 more
Code of Conduct

[X] I have searched the issue tracker and confirmed that the issue has not been reported
[X] I have checked the FAQ (if one is present) and my issue is not listed
[X] I have verified that I am using the latest version of the mod
[X] I have verified that I have installed all the required dependencies for the mod
[X] I have verified that I do not have Optifine installed. We will close issues if we detect Optifine is in your pack. We cannot fix Optifine issues. Please do not report issues with Optifine present.

I'm sorry you had to go through that, sounds like pain. I fixed the bug that appeared in the log, however I have no idea what it has to do with the Deep Dark. The bug in question was an issue with the way Cadmus prevented players from using water buckets in protected chunks.
I've attached the jar. Before I push the update, can you test it on your server to see if that fixes it? Thank you.
cadmus-fabric-1.20.1-0.0.2.jar.zip

I'll test it out and let you know how it turns out! Thank you!

Gave it a thorough test (and also had to update my server to 1.20.1 but that's ok) and it's works like a charm now! Thank you so much!

authors: AlexNijjar QuillInkyDink

repo: terrarium-earth/Cadmus


text:
Stop adding Row suffix to embeddable POJOs

If we define an embeddable type to represent a compound key, the generated POJO
class shouldn't have a Row suffix on its name because it doesn't represent a
table row.
Update the naming strategy to exclude embeddable types.
Currently, we don't have any places in the code where we're referring to these
classes by name; everywhere we use them, we use type inference. So no changes
to the application code are needed here. But this will be less confusing in
the future if/when we do need to refer to the type of a compound key.

Current dependencies on/for this PR:

main

PR #1387   👈

This comment was auto-generated by Graphite.

repo: terraware/terraware-server


text:
SW-2177 Update Icon Sizes

Change specified TopBar icon to medium
Change Home page card icons to medium

Change specified TopBar icon to medium
Change Home page card icons to medium

repo: terraware/terraware-web