search

k2-fsa / snowfall

Moved to https://github.com/k2-fsa/icefall
Apache License 2.0
143 stars 42 forks source link

Order prediction [not-for-merge] #243

Closed danpovey closed 2 years ago

danpovey commented 2 years ago

The reason I am making a pull request of this is that it has a bunch of fixes for the mmi_bigram_train.py script that enable it to run with the latest version of snowfall, it hasn't been tested in a while. I am hoping that someone, maybe @luomingshuang, can extract just those fixes from this PR and make a PR of that, that can be merged.

luomingshuang commented 2 years ago

OK, before I make a PR, I will test it and make sure it can run successfully.

luomingshuang commented 2 years ago

I train and test the mmi_bigram_train.py script with the latest version of snowfall. And this script can be trained and tested successfully. The WER for test-clean is 12.70%. image

danpovey commented 2 years ago

m OK, thanks. Can you please make a PR with that if you have not already? Keep the same name, mmibigram{train,decode}.py. My feeling is that we should merge this soon because the current version of the script does not work; but it would be more ideal if we could find out why there has been a degradation in WER, the RESULTS.md says 10.62%. Don't assume my changes were correct, I might have made a mistake somewhere.

danpovey commented 2 years ago

@pkufool if you could help Mingshuang look at this and possibly compare with older setups, if someone has one around, it would be good.

luomingshuang commented 2 years ago

OK, I will do it right now.

m OK, thanks. Can you please make a PR with that if you have not already? Keep the same name, mmibigram{train,decode}.py. My feeling is that we should merge this soon because the current version of the script does not work; but it would be more ideal if we could find out why there has been a degradation in WER, the RESULTS.md says 10.62%. Don't assume my changes were correct, I might have made a mistake somewhere.

pkufool commented 2 years ago

OK, I will look at this.

pkufool commented 2 years ago

The WER 12.70% is produced with this PR (TdnnLstmc). I think we have two things to do, first, just keep the fixes for the mmi_bigram_train.py (ignoring the order prediction part), and try to reproduce the 10.62% result (we would merge this first). The second, to find out why there has been a degradation in WER in this PR (TdnnLstmc with order prediction).

luomingshuang commented 2 years ago

I am doing the first thing and making the fixes from this PR adapt to the original model (TdnnLstmb).

The WER 12.70% is produced with this PR (TdnnLstmc). I think we have two things to do, first, just keep the fixes for the mmi_bigram_train.py (ignoring the order prediction part), and try to reproduce the 10.62% result (we would merge this first). The second, to find out why there has been a degradation in WER in this PR (TdnnLstmc with order prediction).

danpovey commented 2 years ago

The order prediction is just something I was trying, I just wanted you to take the unrelated changes, i.e. the fixes, and merge those. You can ignore the order prediction.

luomingshuang commented 2 years ago

The order prediction is just something I was trying, I just wanted you to take the unrelated changes, i.e. the fixes, and merge those. You can ignore the order prediction.

Get it.