Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dynamic RNN without padding #1023

Merged
merged 14 commits into from
Jul 19, 2019
Merged

Dynamic RNN without padding #1023

merged 14 commits into from
Jul 19, 2019

Conversation

ArnoldLIULJ
Copy link
Member

Checklist

  • I've tested that my changes are compatible with the latest version of Tensorflow.
  • I've read the Contribution Guidelines
  • I've updated the documentation if necessary.

Motivation and Context

Description

@JingqingZ
Copy link
Member

JingqingZ commented Jul 18, 2019

TODO: example needed for Dynamic RNN in the documentation, changelog, unittest coverage

@zsdonghao
Copy link
Member

do you mean, we can input a batch of text without using padding? or we provide a sequence length into the RNN?

@zsdonghao zsdonghao changed the title without_padding Dynamic RNN without padding Jul 19, 2019
@@ -157,49 +158,104 @@ def build(self, inputs_shape):
self._trainable_weights.append(var)

# @tf.function
def forward(self, inputs, initial_state=None, **kwargs):
def forward(self, inputs, actual_length=None, initial_state=None, **kwargs):
Copy link
Member

Choose a reason for hiding this comment

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

use sequence_length ?

@ArnoldLIULJ
Copy link
Member Author

ArnoldLIULJ commented Jul 19, 2019

*change to sequence_length
*check length of the argument sequence_length

@zsdonghao
Copy link
Member

@JingqingZ JingqingZ requested a review from zsdonghao July 19, 2019 12:55
@zsdonghao zsdonghao merged commit 95c4ca2 into master Jul 19, 2019
@zsdonghao zsdonghao deleted the RNN_without_padding branch July 19, 2019 12:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants