Refactor decode.py to make it more readable and more modular.

[csukuangfj]

No description provided.

[csukuangfj]

This shows how to do nbest decoding.

It is more readable than before, I think.

[pkufool]

There is a implementation of rescoring nbest with attention decoder in #5, the return value is also a Nbest.

icefall/icefall/decode.py

Lines 892 to 900 in 355e324

	def rescore_nbest_with_attention_decoder(
	nbest: Nbest,
	model: nn.Module,
	memory: torch.Tensor,
	memory_key_padding_mask: torch.Tensor,
	sos_id: int,
	eos_id: int,
	) -> Nbest:

[danpovey]

That's cool.
I'll let Fangjun decide what to do about this.

Yes I think the code in this PR is clearer than before

[pkufool]

Ok, Fangjun could refactor decoding as his plan first, I will merge with his code later. It will cost more time to make that PR work, I am afraid :(

[danpovey]

Why not use the Nbest that's in k2? Just wondering.
Also, this seems to only be a partial replacement of the current code?

[danpovey]

.. of course if it's more convenient to work on the class in Icefall, to avoid compatibility issues, that's fine with me.

[danpovey]

cool

…

On Mon, Sep 13, 2021 at 11:01 PM Wei Kang ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In test/test_decode.py <#44 (comment)>: > + lattice=lattice, num_paths=10, use_double_scores=True, scale=0.5 + ) + # each lattice has only 4 distinct paths that have different word sequences: + # 10->30 + # 10->40 + # 20->30 + # 20->40 + # + # So there should be only 4 paths for each lattice in the Nbest object + assert nbest.fsa.shape[0] == 4 * 2 + assert nbest.shape.row_splits(1).tolist() == [0, 4, 8] + + nbest2 = nbest.intersect(lattice) + tot_scores = nbest2.tot_scores() + argmax = tot_scores.argmax() + best_path = k2.index_fsa(nbest2.fsa, argmax) Ok, Fangjun could refactor decoding as his plan first, I will merge with his code later. It will cost more time to make that PR work, I am afraid :( — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#44 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAZFLO4ZHBWC6PM4QRP7CNTUBYG6BANCNFSM5D5XRYGA> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.

[csukuangfj]

This shows how to use k2 to compute the levenshtein distance among multiple sequences at the same time.
(Ideas are from @danpovey)

If anyone is interested in it, I wrote a colab notebook to demonstrate it step by step
https://colab.research.google.com/drive/1hAo5RUb8cMY4qhdn8HASJu9foXrRdo6m?usp=sharing

[pzelasko]

That’s pretty cool, does it mean we could use it directly to optimize WER as a loss function?

[danpovey]

There would have to be some kind of expectation involved for that to work. There are other uses for this though.

[csukuangfj]

This pull-request is ready for review.
(Will use k2.levenshtein_alignment and k2.levenshtein_graph once k2-fsa/k2#828 gets merged)

There are fewer code duplicates. Also, it is more readable, I believe.

It produces the same WER as the previous code. Would be great if someone can test it using an existing model.

---

I can move class Nbest to k2 if @danpovey you think that looks nicer.
(The code for Nbest here has several new functions (e.g., compute_am_scores, compute_lm_scores) )

[danpovey]

Cool!
Regarding moving that code to k2, we should definitely do that at some point, but I'm OK to keep it temporarily here if that would reduce compatibility issues in the short term. We can also duplicate it with here and k2, and later remove it from k2.

[csukuangfj]

Ok, I am merging it.

icefall now requires k2 >= v1.9.