Add a new binary for chain e2e #3945
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
danpovey
reviewed
Feb 20, 2020
| @@ -0,0 +1,121 @@ | |||
| // chainbin/chain-make-num-fst-e2e.cc | |||
|
|
|||
| // Copyright 2019 Johns Hopkins University (author: Yiwen Shao) | |||
There was a problem hiding this comment.
Just put Yiwen Shao as the author as you are not an employee (or if you have a part-time job with Hopkins, at least you are not doing this in that status)
|
|
||
| namespace kaldi { | ||
|
|
||
| bool FstTransitionToPdf(const fst::StdVectorFst &fst_transition, |
There was a problem hiding this comment.
I think it should be .. ToPdfPlusOne(), since these are pdf+1. Also I think this conflicts with your comment or commit message in which you say they are zero-based, these seem to be one-based.
| try { | ||
| const char *usage = | ||
| "Converts chain e2e numerator fst (containing transition-ids) to fst (containing pdf-ids, \n" | ||
| "zero-based, eps-removed, and normalized by the normalization fst) \n" |
There was a problem hiding this comment.
here is where it says zero-based. Also this doesn't remove epsilons, it just checks that there are no epsilons.. might be good to clarify. should say pdf-ids + 1.
|
Thanks for the comments! I've made modifications according to it. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add a new binary: chainbin/chain-make-num-fst-e2e
It is used to converts chain e2e numerator fst (containing transition-ids) to fst (containing pdf-ids,
zero-based, eps-removed, and normalized by the normalization fst).
This step is usually in prepare_egs stage while this binary would be necessary for other outside toolkits (e.g. pychain) that directly use fst instead of egs.