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.

Sign up for GitHub

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

Jump to bottom

[src] Partial hypothesis for cuda decoder #4101

Closed
wants to merge 2 commits into from
Closed

[src] Partial hypothesis for cuda decoder #4101

wants to merge 2 commits into from

Conversation

hugovbraun
Copy link
Contributor

Code is ready but setting the WIP flag because we need to finish testing

The cuda decoder can now generates partial hypotheses on the fly. The compute cost for generating those partial hypotheses is very low. The work is done asynchronously while the GPU is working. This work has been integrated in the online pipeline, for the end user getting partial hypotheses just require to add an argument to the DecodeBatch call:

  void DecodeBatch(const std::vector<CorrelationID> &corr_ids,
                   const std::vector<SubVector<BaseFloat>> &wave_samples,
                   const std::vector<bool> &is_first_chunk,
                   const std::vector<bool> &is_last_chunk,
                   std::vector<std::string *> *partial_hypotheses = NULL);

If partial_hypotheses is not null, the vector will contain the partial hypotheses. The pointers contained by that vector must be used before the next DecodeBatch call.

Easiest way to test is to run the cudadecoderbin/batched-wav-nnet3-cuda-online binary, with --print-partial-hypotheses=true. You probably want to reduce the number of parallel channels to be able to read the output, with --num-parallel-streaming-channels.

Sample output:

main():batched-wav-nnet3-cuda-online.cc:351) ========== BEGIN OF PARTIAL HYPOTHESES ==========
main():batched-wav-nnet3-cuda-online.cc:353) CORR_ID #3     : HELLO
main():batched-wav-nnet3-cuda-online.cc:353) CORR_ID #4     : NUMBER
main():batched-wav-nnet3-cuda-online.cc:353) CORR_ID #5     : THE MUSIC
main():batched-wav-nnet3-cuda-online.cc:353) CORR_ID #6     : THE
main():batched-wav-nnet3-cuda-online.cc:353) CORR_ID #7     : A
main():batched-wav-nnet3-cuda-online.cc:353) CORR_ID #8     : THE
main():batched-wav-nnet3-cuda-online.cc:353) CORR_ID #9     : AT
main():batched-wav-nnet3-cuda-online.cc:353) CORR_ID #0     : HE HOPED
main():batched-wav-nnet3-cuda-online.cc:353) CORR_ID #1     : 
main():batched-wav-nnet3-cuda-online.cc:353) CORR_ID #2     : AFTER
main():batched-wav-nnet3-cuda-online.cc:356) =========== END OF PARTIAL HYPOTHESES ===========
main():batched-wav-nnet3-cuda-online.cc:351) ========== BEGIN OF PARTIAL HYPOTHESES ==========
main():batched-wav-nnet3-cuda-online.cc:353) CORR_ID #3     : HELLO
main():batched-wav-nnet3-cuda-online.cc:353) CORR_ID #4     : NUMBER DEN     
main():batched-wav-nnet3-cuda-online.cc:353) CORR_ID #5     : THE MUSIC CAME 
main():batched-wav-nnet3-cuda-online.cc:353) CORR_ID #6     : THE DULL
main():batched-wav-nnet3-cuda-online.cc:353) CORR_ID #7     : A COLD
main():batched-wav-nnet3-cuda-online.cc:353) CORR_ID #8     : THE CHAOS
main():batched-wav-nnet3-cuda-online.cc:353) CORR_ID #9     : AT MOST 
main():batched-wav-nnet3-cuda-online.cc:353) CORR_ID #0     : HE HOPED THERE WOULD
main():batched-wav-nnet3-cuda-online.cc:353) CORR_ID #1     : STUFFED INTO   
main():batched-wav-nnet3-cuda-online.cc:353) CORR_ID #2     : AFTER EARLY    
main():batched-wav-nnet3-cuda-online.cc:356) =========== END OF PARTIAL HYPOTHESES ===========

Right now only the text version of the partial hypothesis can be returned by the online pipeline. If it's useful to also return the int olabels, we can, but I'd like to keep the high-level API as simple as possible. The endpointing will be handled directly in the decoder.

@danpovey
Copy link
Contributor

danpovey commented Jun 12, 2020 via email

@al-zatv
Copy link

al-zatv commented Jun 12, 2020

Thank you for your job! This is really important feature for me.

@hugovbraun hugovbraun changed the title [WIP] [src] Partial hypothesis for cuda decoder [src] Partial hypothesis for cuda decoder Jun 30, 2020
@hugovbraun
Copy link
Contributor Author

@al-zatv glad to hear! Were you able to give it a try?

Testing ok on our side.

@hugovbraun hugovbraun mentioned this pull request Jun 30, 2020
Merged
kkm000
kkm000 requested changes Jul 1, 2020
View reviewed changes
Copy link
Contributor

@kkm000 kkm000 left a comment

Choose a reason for hiding this comment

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

This is mostly about the public API, which will be harder to change later without breaking folk's code.

Feel free to apply changes to #4146 if that's easier for you, I'll merge one immediately after another then.

Thanks for this—I wish I understood CUDA programming one tenth as good as you do! :)

src/cudadecoder/batched-threaded-nnet3-cuda-online-pipeline.h Outdated Show resolved Hide resolved
src/cudadecoder/batched-threaded-nnet3-cuda-online-pipeline.h Outdated Show resolved Hide resolved
src/cudadecoder/batched-threaded-nnet3-cuda-online-pipeline.cc Outdated Show resolved Hide resolved
src/cudadecoder/cuda-decoder.h Outdated Show resolved Hide resolved
src/cudadecoder/cuda-decoder.h Outdated Show resolved Hide resolved
src/cudadecoderbin/batched-wav-nnet3-cuda-online.cc Show resolved Hide resolved
src/cudadecoderbin/batched-wav-nnet3-cuda-online.cc Show resolved Hide resolved
src/cudadecoderbin/batched-wav-nnet3-cuda2.cc Outdated Show resolved Hide resolved
src/cudadecoder/cuda-decoder.cc Outdated Show resolved Hide resolved
src/cudadecoder/cuda-decoder.cc Outdated Show resolved Hide resolved
@hugovbraun
Copy link
Contributor Author

@kkm000 thanks for the great review! I'll push a new commit early next week.

@hugovbraun
Copy link
Contributor Author

Thank you for the detailed review @kkm000 . I've made the necessary changes to both this PR and #4146

@danpovey
Copy link
Contributor

danpovey commented Jul 7, 2020

LMK when you guys think it's ready to merge. Or merge yourself, @kkm

@hugovbraun
Copy link
Contributor Author

We've just found a bug while testing with a new dataset - let's hold for a few days until we fix it

@hugovbraun
Copy link
Contributor Author

I pushed the fix in #4146. Maybe we can close that PR and work only with #4146

@danpovey
Copy link
Contributor

@hugovbraun I am assuming you mean close this PR.. can you do it yourself if that's what you mean?

@hugovbraun hugovbraun closed this Jul 14, 2020
kkm000 pushed a commit that referenced this pull request Jul 15, 2020
@hugovbraun
[src] CudaDecoder endpointing (#4146, #4101)
083c64d 
 * Partial hypotheses
 * PR comments
 * Endpointing
 * PR comments
 * Neg non-em partial traceback bug fix
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kkm000 kkm000 kkm000 requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@hugovbraun @danpovey @al-zatv @kkm000