Skip to content

Conversation

guillaumekln
Copy link
Contributor

This simplifies the usage of the layer outside of Keras where a typical usage would be to create the layer once and then setup the memory dynamically, e.g.:

attention = tfa.seq2seq.LuongAttention(units)
# ... later in the code ...
attention.setup_memory(memory, memory_sequence_length)

@qlzh727
Copy link
Member

qlzh727 commented Jul 17, 2019

By changing the setup_memory to be public, this could imply to user that it can be called multiple times, which should cause issue when using the overall keras model. On the other hand, it should work if this is only used for building up graph, and train with customized logic.

Can we add some warning to user about this side effect if the memory is reset?

@guillaumekln
Copy link
Contributor Author

I'm not very familiar with the whole Keras workflow. Could you provide the warning string that I should add in this case?

@qlzh727
Copy link
Member

qlzh727 commented Jul 30, 2019

Sorry for the late reply. I guess the Attention itself is already not very aligned with keras framework (eg, require tensor as inputs to layer from call() instead of init argument). If user want just use keras to build the network and not use model.fit/eval/predict, then this change probably will not affect them.

@seanpmorgan seanpmorgan merged commit 13cf110 into tensorflow:master Jul 30, 2019
@guillaumekln guillaumekln deleted the manual-memory-reset branch June 9, 2020 08:16
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.

5 participants