Skip to content

Conversation

@freewym
Copy link
Contributor

@freewym freewym commented Aug 11, 2016

…heir results. Fix an option name bug in steps/nnet3/align.sh

ivector_period=$(cat $online_ivector_dir/ivector_period) || exit 1;
# note: subsample-feats, with negative n, will repeat each feature -n times.
ivector_opts="--online-ivectors=scp:$online_ivector_dir/ivector_online.scp --online-ivector_period=$ivector_period"
ivector_opts="--online-ivectors=scp:$online_ivector_dir/ivector_online.scp --online-ivector-period=$ivector_period"
Copy link
Contributor

Choose a reason for hiding this comment

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

What was the difference in performance due to this bug ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't compare. I just found it accidently

Copy link
Contributor

Choose a reason for hiding this comment

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

You should modify the commit message to highlight this bug, as it would be relevant to several recipes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Modifed.

Copy link
Contributor

Choose a reason for hiding this comment

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

Modifying the way this option is specified does not change the alignments, so I guess the option parser in c++ is immune this mis-specification.

@freewym freewym force-pushed the librispeech branch 2 times, most recently from ac05db4 to 79df9db Compare August 11, 2016 00:56
@@ -443,6 +443,73 @@
%WER 15.10 [ 7904 / 52343, 874 ins, 1070 del, 5960 sub ] exp/nnet3/tdnn_sp/decode_test_other_tgmed/wer_13_0.0
%WER 16.29 [ 8528 / 52343, 828 ins, 1320 del, 6380 sub ] exp/nnet3/tdnn_sp/decode_test_other_tgsmall/wer_14_0.0
Copy link
Contributor

@vijayaditya vijayaditya Aug 11, 2016

Choose a reason for hiding this comment

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

This results file is really tough to read and compare. Would you be able to provide a brief set of important results right after you specify the command and present the detailed results after that ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added a subset of the results before the full list of results

@vijayaditya
Copy link
Contributor

Please fix the spelling mistakes in your commit message.

@vijayaditya
Copy link
Contributor

You can add a line new line in the commit message. Also please change
"Fix" --> "Fixed".

In general you should have comments which follow this guideline.
comments should be descriptive ("Opens the file") rather than imperative ("Open the file")

@freewym freewym force-pushed the librispeech branch 2 times, most recently from 997d905 to 48bdb00 Compare August 11, 2016 01:52
the results. Fixed a bug in steps/nnet3/align.sh when supplying
online-ivector-period option to nnet3-align-compiled
@vijayaditya vijayaditya merged commit 3f7d404 into kaldi-asr:master Aug 11, 2016
@freewym freewym deleted the librispeech branch August 11, 2016 02:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants