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

Moved run_all_in_graph_and_eager_mode in basic decoder #1670

Merged
merged 4 commits into from
Apr 15, 2020

Conversation

autoih
Copy link
Member

@autoih autoih commented Apr 14, 2020

cc @gabrieldemarmiesse. Dividing into several parts since it's large, and not sure if removing use_gpu is a good idea. The reason is using gpu seems not helping a lot in LSTM, only if it's self-attention(transformer) based. May need your suggestions.

@gabrieldemarmiesse
Copy link
Member

About the use of gpu for testing going forward, unless we're testing some code with a custom op, we need a strong reason to run in both cpu and gpu mode.

Copy link
Member

@gabrieldemarmiesse gabrieldemarmiesse left a comment

Choose a reason for hiding this comment

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

Thank your for the pull request! Some changes to be completely equivalent with the previous tests.

tensorflow_addons/seq2seq/tests/basic_decoder_test.py Outdated Show resolved Hide resolved
tensorflow_addons/seq2seq/tests/basic_decoder_test.py Outdated Show resolved Hide resolved
Copy link
Member

@gabrieldemarmiesse gabrieldemarmiesse left a comment

Choose a reason for hiding this comment

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

Thanks again!

@gabrieldemarmiesse gabrieldemarmiesse merged commit ee9ee1d into tensorflow:master Apr 15, 2020
@autoih autoih deleted the ptb1 branch April 15, 2020 22:54
jrruijli pushed a commit to jrruijli/addons that referenced this pull request Dec 23, 2020
* Moved run_all_in_graph_and_eager_mode in basic decoder

* minor removal

* Update tensorflow_addons/seq2seq/tests/basic_decoder_test.py

Co-Authored-By: Gabriel de Marmiesse <[email protected]>

* Update tensorflow_addons/seq2seq/tests/basic_decoder_test.py

Co-Authored-By: Gabriel de Marmiesse <[email protected]>

Co-authored-by: Gabriel de Marmiesse <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants