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

Batch processing of beam search #318

Merged
merged 36 commits into from Nov 5, 2018

Conversation

@mn5k
Copy link
Contributor

commented Jul 30, 2018

Batchsize ATT CTC RNNLM WER (test_clean)
1 (orig.) 6.5
48 6.5

@sw005320 sw005320 requested review from kan-bayashi and takaaki-hori Jul 30, 2018

@takaaki-hori
Copy link
Contributor

left a comment

This modification seems to replace the original recognize_beam() that deals with only a single input sequence on CPU. But some modules, e.g., word-based LMs do not support batch processing yet. It is better to test it with WSJ recipe including the word-based LM with batchsize=1.
Other people are also touching the beam search part, e.g., for multi-encoder decoding.
I think it is better to keep the original version for a while in addition to the batch version.

@@ -19,7 +19,7 @@ def main():
# general configuration
parser.add_argument('--gpu', default=None, type=int, nargs='?',
help='GPU ID (negative value indicates CPU)')
parser.add_argument('--ngpu', default=0, type=int,
parser.add_argument('--rec-ngpu', default=0, type=int,

This comment has been minimized.

Copy link
@takaaki-hori

takaaki-hori Aug 9, 2018

Contributor

Is it necessary to change --ngpu to --rec-ngpu?
It seems to be obvious that asr_recog.py uses multiple GPUs for recognition by specifying --ngpu

@@ -30,6 +30,8 @@ def main():
help='Random seed')
parser.add_argument('--verbose', '-V', default=1, type=int,
help='Verbose option')
parser.add_argument('--rec-batchsize', default=1, type=int,

This comment has been minimized.

Copy link
@takaaki-hori

takaaki-hori Aug 9, 2018

Contributor

I think we may change it to --batchsize, since asr_recog.py does only recognition. The role of the option is obvious even if it does not have prefix rec.

@@ -105,6 +105,25 @@ def main():
help='Apply label smoothing with a specified distribution type')
parser.add_argument('--lsm-weight', default=0.0, type=float,
help='Label smoothing weight')
# recognition related

This comment has been minimized.

Copy link
@takaaki-hori

takaaki-hori Aug 9, 2018

Contributor

A bit confusing. It's better to say recognition options to compute CER/WER

@stale

This comment has been minimized.

Copy link

commented Aug 27, 2018

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the Stale label Aug 27, 2018

@kan-bayashi kan-bayashi removed the Stale label Aug 27, 2018

@sw005320

This comment has been minimized.

Copy link
Contributor

commented Aug 30, 2018

Can you please revisit it? This function is really important.

@mn5k

This comment has been minimized.

Copy link
Contributor Author

commented Aug 31, 2018

Thank you for the review.
I'll modify the code and add an option for word-based LM.

@mn5k mn5k changed the title Batch processing of beam search [WIP]Batch processing of beam search Sep 4, 2018

@mn5k

This comment has been minimized.

Copy link
Contributor Author

commented Sep 7, 2018

@takaaki-hori There is a class (MultiLevelLM, #235) that combines character-based and word-based language models in extlm_pytorch.py. Could you tell me the WER of this method?
The WER of test_eval92 was high as 102.6% in my condition.

@takaaki-hori

This comment has been minimized.

Copy link
Contributor

commented Sep 7, 2018

Maybe something wrong.
In my test, I got a bit better result than LookAheadLM in WSJ with a 20k-word LM.
But I have not tested it recently. I will revisit it in #406 .

mn5k added 3 commits Sep 21, 2018
@mn5k

This comment has been minimized.

Copy link
Contributor Author

commented Sep 21, 2018

I used editdistance in
https://github.com/aflc/editdistance
for the computation of CER/WER now.
Should I implement it without the external package?

@sw005320

This comment has been minimized.

Copy link
Contributor

commented Sep 21, 2018

No problem. You can just use it.

@mn5k mn5k changed the title [WIP]Batch processing of beam search Batch processing of beam search Oct 5, 2018

@sw005320

This comment has been minimized.

Copy link
Contributor

commented Oct 31, 2018

I want to restart this project. It seems that we don't have serious issue, and we may merge it. @mn5k, do you have any concerns?

@sw005320 sw005320 referenced this pull request Oct 31, 2018
16 of 21 tasks complete
@mn5k

This comment has been minimized.

Copy link
Contributor Author

commented Nov 2, 2018

I think there's no problem.

@sw005320

This comment has been minimized.

Copy link
Contributor

commented Nov 2, 2018

we can use batch decoding as default. maybe 1 is enough. then could you add the test to check batch and non batch decodings yield the same results?

@sw005320 sw005320 merged commit 55d07e1 into espnet:master Nov 5, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@sw005320

This comment has been minimized.

Copy link
Contributor

commented Nov 5, 2018

Thanks a lot! I merged it.

@mn5k

This comment has been minimized.

Copy link
Contributor Author

commented Nov 5, 2018

I added a test code to check the output of two search algorithms.

As I mentioned before, my search algorithm does not fully correspond to the original search.
I only checked the search without a fusion of other external modules.

@JaejinCho

This comment has been minimized.

Copy link
Contributor

commented on src/asr/asr_pytorch.py in 658269a Jan 21, 2019

I think torch.load should be torch_load.

This comment has been minimized.

Copy link
Contributor

replied Jan 21, 2019

@JaejinCho, thanks!
@mn5k, could you test it and fix it?

This comment has been minimized.

Copy link
Contributor Author

replied Jan 22, 2019

Thank you. I'll fix it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.