Keras: Fix K.rnn documentation on unroll parameter

Created on 26 Jan 2017  Â·  8Comments  Â·  Source: keras-team/keras

Edit

Docs state the below, but unroll is a parameter used by both backends.

unroll: with TensorFlow the RNN is always unrolled, but with Theano you can use this boolean flag to unroll the RNN.

For theano backend we use scan but for tf we use while_loop, even through tf also supports scan. Also, does anyone know if using scan instead of while_loop has any performance or other benefits?

Cheers,
Ben

Please make sure that the boxes below are checked before you submit your issue. If your issue is an implementation question, please ask your question on StackOverflow or join the Keras Slack channel and ask there instead of filing a GitHub issue.

Thank you!

  • [x ] Check that you are up-to-date with the master branch of Keras. You can update with:
    pip install git+git://github.com/fchollet/keras.git --upgrade --no-deps

  • [x ] If running on TensorFlow, check that you are up-to-date with the latest version. The installation instructions can be found here.

  • [x ] If running on Theano, check that you are up-to-date with the master branch of Theano. You can update with:
    pip install git+git://github.com/Theano/Theano.git --upgrade --no-deps

  • [ x] Provide a link to a GitHub Gist of a Python script that can reproduce your issue (or just copy the script here if it is short).

All 8 comments

I'm pretty sure theano can optimize a scan better than an unrolled RNN. What I don't know is if TF will have different performance between unrolled and scan.

That's, like, not true. Keras has supported dynamic TF RNNs for as long as
they have been available in TF.

On Jan 26, 2017 10:00, "Ben" notifications@github.com wrote:

I'm pretty sure theano can optimize a scan better than an unrolled RNN.
What I don't know is if TF will have different performance between unrolled
and scan.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/fchollet/keras/issues/5191#issuecomment-275461815,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AArWb4mOF7GQRtaAIR2Wf9qmhkPDersyks5rWN8kgaJpZM4Lu8X8
.

@fchollet Then are the docs out of date or am I just misreading them?

unroll: with TensorFlow the RNN is always unrolled, but with Theano you can use this boolean flag to unroll the RNN.

@fchollet Thanks! I missed it because TF uses while_loop instead of a scan.

The docs make it sound like tensorflow K.rnn ignores unroll, which it doesn't.

Is there any advantage to control_flow_ops.while_loop over tf.scan?

So my two comments are:

  • Should update docs because unrolling is supported in both backends.
  • Determine if there is any performance difference with using scan over while_loop. No idea what works best.

Updated documentation in this PR:
https://github.com/fchollet/keras/pull/5192

As to the difference between while_loop and scan in tensorflow, anyone know if there is a performance difference or should I just try it and see?

@fchollet serves me right for reading the doc instead of just reading the code ;)

Another weird inconsistency:

Docs:

input_length: not relevant in the TensorFlow implementation. Must be specified if using unrolling with Theano.

Tensorflow implementation gets timesteps from the input shape and ignores the input_length parameter

time_steps = tf.shape(inputs)[0]

Theano implementation requires the input_length parameter.

    if unroll:
        if input_length is None:
            raise ValueError('When specifying `unroll=True`, '
                             'an `input_length` '
                             'must be provided to `rnn`.')

Would it be more consistent if both got the input shape the same way? The time_steps parameter could be used by both backends, or could be removed and both could get the shape from the input.

Closing the issue. Just a small PR for the documentation.https://github.com/fchollet/keras/pull/5192

Was this page helpful?
0 / 5 - 0 ratings

Related issues

Imorton-zd picture Imorton-zd  Â·  3Comments

harishkrishnav picture harishkrishnav  Â·  3Comments

amityaffliction picture amityaffliction  Â·  3Comments

somewacko picture somewacko  Â·  3Comments

snakeztc picture snakeztc  Â·  3Comments