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.

Sign up for GitHub

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

Jump to bottom

Update LSTMBlockCell to use LSTMStateTuple as state #6810

Merged
merged 3 commits into from
Feb 17, 2017
Merged

Update LSTMBlockCell to use LSTMStateTuple as state #6810

merged 3 commits into from
Feb 17, 2017

Conversation

jihunchoi
Copy link
Contributor

Fixes #6582

@tensorflow-jenkins
Copy link
Collaborator

Can one of the admins verify this patch?

@drpngx
Copy link
Contributor

drpngx commented Jan 13, 2017

@ebrevdo any interest?

@jihunchoi
Copy link
Contributor Author

jihunchoi commented Jan 13, 2017 •
edited
Loading

Reading my codes written at midnight again, I found LSTMBlockFusedCell should not be fixed! I will revert it back.

@drpngx drpngx added the stat:awaiting response Status - Awaiting response from author label Jan 13, 2017
@jihunchoi jihunchoi changed the title Update LSTMBlockCell, LSTMBlockFusedCell to use LSTMStateTuple as state Update LSTMBlockCell to use LSTMStateTuple as state Jan 14, 2017
@drpngx drpngx added stat:awaiting tensorflower Status - Awaiting response from tensorflower and removed stat:awaiting response Status - Awaiting response from author labels Jan 15, 2017
@drpngx drpngx requested a review from ebrevdo January 24, 2017 21:04
@rmlarsen rmlarsen added the awaiting review Pull request awaiting review label Jan 30, 2017
@jihunchoi jihunchoi mentioned this pull request Feb 9, 2017
Merged
@vrv
Copy link

vrv commented Feb 13, 2017

@jihunchoi just to make sure, this PR is blocked on #7387 ?

@vrv vrv added stale This label marks the issue/pr stale - to be closed automatically if no activity stat:awaiting response Status - Awaiting response from author and removed awaiting review Pull request awaiting review stat:awaiting tensorflower Status - Awaiting response from tensorflower labels Feb 13, 2017
@jihunchoi
Copy link
Contributor Author

jihunchoi commented Feb 13, 2017 •
edited
Loading

No, I mentioned this PR wrongly in #7387 but the message did not disappear after I deleted the mention... :(

@vrv
Copy link

vrv commented Feb 13, 2017

It's okay, just wanted to understand the status -- so this is ready to review again?

@jihunchoi
Copy link
Contributor Author

Yes, the mention can be safely ignored.

@vrv vrv added awaiting review Pull request awaiting review stat:awaiting tensorflower Status - Awaiting response from tensorflower and removed stale This label marks the issue/pr stale - to be closed automatically if no activity stat:awaiting response Status - Awaiting response from author labels Feb 13, 2017
@ebrevdo
Copy link
Contributor

ebrevdo commented Feb 14, 2017

Can you resolve merge conflicts?

@jihunchoi
Copy link
Contributor Author

Was there a conflict? It was able to be automatically merged with tensorflow/master branch.
Anyway, I updated it to follow the latest master branch and it seems there exists no conflict.

ebrevdo
ebrevdo approved these changes Feb 14, 2017
View reviewed changes
Copy link
Contributor

@ebrevdo ebrevdo left a comment

Choose a reason for hiding this comment

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

LGTM

@ebrevdo
Copy link
Contributor

ebrevdo commented Feb 14, 2017

thanks! please ensure all tests pass.

@vrv
Copy link

vrv commented Feb 15, 2017

@tensorflow-jenkins test this please

@vrv vrv merged commit eabd233 into tensorflow:master Feb 17, 2017
@jihunchoi jihunchoi deleted the modify_lstm_block_cell_state branch February 24, 2017 02:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ebrevdo ebrevdo ebrevdo approved these changes

Assignees

@ebrevdo ebrevdo

Labels
awaiting review Pull request awaiting review cla: yes stat:awaiting tensorflower Status - Awaiting response from tensorflower
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@jihunchoi @tensorflow-jenkins @drpngx @vrv @ebrevdo @googlebot @rmlarsen