Add dynamic_decode and Fix decoder issue #208
Conversation
gpengzhi
force-pushed
the
decoder-issue
branch
2 times, most recently
from
54427fc
to
51b37dc
Compare
gpengzhi
force-pushed
the
decoder-issue
branch
from
51b37dc
to
2307d99
Compare
There was a problem hiding this comment.
beam_search_decode relying on BeamSearchDecoder is not compatible to the new dynamic_decode. Can you update using beam_search in texar.utils, and add to beam_search the mode of standard beam search (make sure the same results with BeamSearchDecoder). Then beam_search_decode should be deleted.
This PR would make huge impact. Let's make sure it's bug-free, by reproducing example results in at least transformer/, seq2seq_atten, and text_style_transfer.
gpengzhi
force-pushed
the
decoder-issue
branch
4 times, most recently
from
81cab99
to
a2dee78
Compare
gpengzhi
force-pushed
the
decoder-issue
branch
from
a2dee78
to
735f079
Compare
| @@ -41,6 +42,8 @@ def _merge_beam_dim(tensor): | |||
| Returns: | |||
| Reshaped tensor of shape [A*B, ...] | |||
| """ | |||
| if not isinstance(tensor, tf.Tensor): | |||
| return tensor | |||
There was a problem hiding this comment.
Why directly return tensor here instead of converting it into a tf.Tensor and continue the subsequent operations?
There was a problem hiding this comment.
When we implement beam search decoding in AttentionRNNDecoder by using this function, state used in this function is AttentionWrapperState which contains int type attribute time. This check is used to only reshape the correct attributes in state. We also have similar checking in texar-pytorch.
There was a problem hiding this comment.
Thanks for explaining. Can you add brief comments to make the code more readable? E.g., "if tensor is not tf.Tensor, then tensor is xxx, return directly".
Resolve #199