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

Add training call argument for MultiHeadAttention #42625

Merged

Conversation

WindQAQ
Copy link
Member

@WindQAQ WindQAQ commented Aug 24, 2020

Miss training call argument for dropout layer. /cc @tanzhenyu for visibility.

@google-ml-butler google-ml-butler bot added the size:S CL Change Size: Small label Aug 24, 2020
@gbaned gbaned self-assigned this Aug 24, 2020
@gbaned gbaned added the comp:keras Keras related issues label Aug 24, 2020
@gbaned gbaned added this to Assigned Reviewer in PR Queue via automation Aug 24, 2020
@gbaned gbaned requested a review from tanzhenyu August 24, 2020 17:10
@gbaned gbaned added the awaiting review Pull request awaiting review label Aug 27, 2020
@WindQAQ
Copy link
Member Author

WindQAQ commented Sep 3, 2020

Hi @tanzhenyu, sorry for bothering you. Can you review this when time allows? We are going to deprecate MultiHeadAttention in addons, and this will give us a full spectrum and more stable API during deprecation.

Copy link
Contributor

@tanzhenyu tanzhenyu left a comment

Choose a reason for hiding this comment

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

Thank you!

PR Queue automation moved this from Assigned Reviewer to Approved by Reviewer Sep 3, 2020
@google-ml-butler google-ml-butler bot added kokoro:force-run Tests on submitted change ready to pull PR ready for merge process labels Sep 3, 2020
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Sep 3, 2020
@gbaned gbaned added ready to pull PR ready for merge process and removed awaiting review Pull request awaiting review ready to pull PR ready for merge process labels Sep 4, 2020
@google-ml-butler google-ml-butler bot added kokoro:force-run Tests on submitted change ready to pull PR ready for merge process labels Sep 4, 2020
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Sep 4, 2020
@saberkun
Copy link
Member

Is the training argument required? I thought keras will automatically inject training from models.
Something would not work if it is inside a tf.module?

@WindQAQ
Copy link
Member Author

WindQAQ commented Sep 30, 2020

Is the training argument required? I thought keras will automatically inject training from models.
Something would not work if it is inside a tf.module?

Hi @saberkun It will be injected when using it inside tf.keras.Model or tf.keras.layers.Layer but will not work inside tf.Module. I think this is not an usually case actually.

tf-models-copybara-bot pushed a commit to tensorflow/models that referenced this pull request Oct 5, 2020
Imported from GitHub PR tensorflow/tensorflow#42625

Miss `training` call argument for dropout layer. /cc @tanzhenyu for visibility.
Copybara import of the project:

--
ba2198fc735a2deca08c68a3c6266d02e01dfe3b by Tzu-Wei Sung <windqaq@gmail.com>:

Add training call argument for MultiHeadAttention

Remove trailing space

PiperOrigin-RevId: 335507127
tf-models-copybara-bot pushed a commit to tensorflow/models that referenced this pull request Oct 5, 2020
Imported from GitHub PR tensorflow/tensorflow#42625

Miss `training` call argument for dropout layer. /cc @tanzhenyu for visibility.
Copybara import of the project:

--
ba2198fc735a2deca08c68a3c6266d02e01dfe3b by Tzu-Wei Sung <windqaq@gmail.com>:

Add training call argument for MultiHeadAttention

Remove trailing space

PiperOrigin-RevId: 335507127
@tensorflow-copybara tensorflow-copybara merged commit 8ba1672 into tensorflow:master Oct 5, 2020
PR Queue automation moved this from Approved by Reviewer to Merged Oct 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes comp:keras Keras related issues ready to pull PR ready for merge process size:S CL Change Size: Small
Projects
PR Queue
  
Merged
Development

Successfully merging this pull request may close these issues.

None yet

7 participants