-
Notifications
You must be signed in to change notification settings - Fork 375
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
Implementing the predict_n method. Using the beam search outputs it returns several seqs for a given seq #116
Conversation
(intended to be used along to Beam Search using TopKDecoder)
Using the beam search outputs it returns several seqs for a given seq
…c_seq input" This reverts commit 4424310. # Conflicts: # seq2seq/evaluator/predictor.py
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good PR! Only two minor things to change. Please see comments.
seq2seq/evaluator/predictor.py
Outdated
@@ -42,3 +43,31 @@ def predict(self, src_seq): | |||
tgt_id_seq = [other['sequence'][di][0].data[0] for di in range(length)] | |||
tgt_seq = [self.tgt_vocab.itos[tok] for tok in tgt_id_seq] | |||
return tgt_seq | |||
|
|||
def predict_n(self, src_seq, n=None): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think n
should be 1 by default because at line 67 you will do int(n)
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For sure. I checked that value on the application I'm building using this library, but it would be better to have in the method definition to ensure a safer running.
if torch.cuda.is_available(): | ||
src_id_seq = src_id_seq.cuda() | ||
|
||
softmax_list, _, other = self.model(src_id_seq, [len(src_seq)]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Line 59-64 are exactly the same as line 35-40, it'd be better to extract a function for that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I placed this code in a new function called get_decoder_features
Removing unused variables returned by this function.
* Modified parameter order of DecoderRNN.forward (#85) * Updated TopKDecoder (#86) * Fixed topk decoder. * Use torchtext from pipy (#87) * Use torchtext from pipe. * Fixed torch text sorting order. * attention is not required when only using teacher forcing in decoder (#90) * attention is not required when only using teacher forcing in decoder * Updated docs and version. * Fixed code style. * bugfix (#92) Fixed field arguments validation. * Removed `initial_lr` when resuming optimizer with scheduler. (#95) * shuffle the training data (#97) * 0.1.5 (#91) * Modified parameter order of DecoderRNN.forward (#85) * Updated TopKDecoder (#86) * Fixed topk decoder. * Use torchtext from pipy (#87) * Use torchtext from pipe. * Fixed torch text sorting order. * attention is not required when only using teacher forcing in decoder (#90) * attention is not required when only using teacher forcing in decoder * Updated docs and version. * Fixed code style. * shuffle the training data * fix example of inflate function in TopKDecoer.py (#98) * fix example of inflate function in TopKDecoer.py * Fix hidden_layer size for one-directional decoder (#99) * Fix hidden_layer size for one-directional decoder Hidden layer size of the decoder was given `hidden_size * 2 if bidirectional else 1`, resulting in a dimensionality error for non-bidirectional decoders. Changed `1` to `hidden_size`. * Adapt load to allow CPU loading of GPU models (#100) * Adapt load to allow CPU loading of GPU models Add storage parameter to torch.load to allow loading models on a CPU that are trained on the GPU, depending on availability of cuda. * Fix wrong parameter use on DecoderRNN (#103) * Fix wrong parameter use on DecoderRNN * Upgrade to pytorch-0.3.0 (#111) * Upgrade to pytorch-0.3.0 * Use pytorch 3.0 in travis env. * Make sure tensor contiguous when attention's not used. (#112) * Implementing the predict_n method. Using the beam search outputs it returns several seqs for a given seq (#116) * Adding a predictor method to return n predicted seqs for a src_seq input (intended to be used along to Beam Search using TopKDecoder) * Checkpoint after batches not epochs (#119) * Pytorch 0.4 (#134) * add contiguous call to tensor (#127) when attention is turned off, pytorch (well, 0.4 at least) gets angry about calling view on a non-contiguous tensor * Fixed shape documentation (#131) * Update to pytorch-0.4 * Remove pytorch manual install in travis. * Allow using pre-trained embedding (#135) * updated docs
No description provided.