Only include tokens with <eps> arc in queue #2641
Conversation
|
Thanks! There seems to be a compilation error in lattice-simple-decoder.cc though, from the travis buld. |
|
@danpovey oh alright, I'll try to fix it. |
|
We have already had all frontier token in the queue, why do we add tokens(with epsilon ilabel) again? Just from a quick skim, not for sure. Should this check be done in the first place of queue's push_back() ? |
| } | ||
| } | ||
|
|
||
| for (const Elem *e = toks_.GetList(); e != NULL; e = e->tail) { |
There was a problem hiding this comment.
Yes, @dophist is right; you need to replace the loop at line 812 with this.
| queue.push_back(iter->first); | ||
| best_cost = std::min(best_cost, iter->second->tot_cost); | ||
| StateId key = iter->first; | ||
| if (fst_->NumInputEpsilons(key) != 0) { |
There was a problem hiding this comment.
@vimalmanohar I'm getting an error here because of this method. Can you help with this?
There was a problem hiding this comment.
It looks like fst_ is not a pointer. In Dan's PR, it was changed to a pointer. So here you need to use fst_.
|
@danpovey I think it can be merged now. |
| queue_.push_back(e->key); | ||
| for (const Elem *e = toks_.GetList(); e != NULL; e = e->tail) { | ||
| StateId key = e->key; | ||
| if (fst_.NumInputEpsilons(key) != 0) |
There was a problem hiding this comment.
there is another place where this check should be done, in the loop below this, before you push to the queue. (applies to all the decoders).
| StateId key = iter->first; | ||
| if (fst_.NumInputEpsilons(key) != 0) { | ||
| queue.push_back(key); | ||
| best_cost = std::min(best_cost, iter->second->tot_cost); |
There was a problem hiding this comment.
the assignment of best_cost should not be inside the if-statement. Check that in my original PR I did not make this mistake.
There was a problem hiding this comment.
Oh okay. Actually the PR did not have changes in the lattice-simple-decoder, so I figured the best_cost is only over those objects which are actually being pushed to the queue. Anyway, I'll make this change.
There was a problem hiding this comment.
Thanks. The idea is to make the code equivalent to what it was before (but faster). The best_cost is over all live tokens, which includes states that don't have epsilon transitions frm them.
| for (const Elem *e = toks_.GetList(); e != NULL; e = e->tail) | ||
| queue_.push_back(e->key); | ||
| for (const Elem *e = toks_.GetList(); e != NULL; e = e->tail) { | ||
| StateId key = e->key; |
There was a problem hiding this comment.
actually, I think it would be clearer if this variable were named state, and not key. I know it was key in my branch; I'll fix the conflict later.
…() (kaldi-asr#2641) Note: in one configuration I saw 25% speedup with this change (depends on beams, etc.)
…() (kaldi-asr#2641) Note: in one configuration I saw 25% speedup with this change (depends on beams, etc.)
4 participants
No description provided.