Backstitch rnnlm by freewym · Pull Request #2096 · kaldi-asr/kaldi

freewym · 2017-12-21T20:39:19Z

@hainan-xv Please take a look.

hainan-xv · 2017-12-22T04:10:50Z

egs/ami/s5b/local/rnnlm/tuning/run_lstm_tdnn_b.sh

 fi

 if [ $stage -le 3 ]; then
+  backstitch_opt="--backstitch-scale $alpha --backstitch-interval $back_interval"


there should probably be a way to easily turn off the backstitch? e.g. a --use-backstitch false option for the script, and if set to false, backstitch_opt="" ?
Or maybe it's automatically turned off if alpha=0? In that case, maybe you can state this clearly in the code

Regarding turning backstitch off: I prefer to have this level of script not be too configurable. But I think it would be good idea to rename the script to run_lstm_tdnn_bs_1a.sh and have the link from run_lstm_tdnn_bs.sh, to encode that it's a script with backstitch.

hainan-xv · 2017-12-22T04:11:23Z

egs/wsj/s5/steps/libs/nnet3/report/log_parse.py

+    valid_prob_strings = common_lib.get_command_stdout(
+        'grep -e {0} {1}'.format(key, valid_prob_files))
+
+    # LOG


what is this comment for?

They are example strings to be matched for the regular expression.

hainan-xv · 2017-12-22T04:14:08Z

scripts/rnnlm/train_rnnlm.sh

 embedding_l2=0.005
 embedding_lrate_factor=0.1  # the embedding learning rate is the
                            # nnet learning rate times this factor.
+backstitch_scale=0.0        # backstitch training scale


similar to the comment above? does 0.0 mean no backstitch?

hainan-xv · 2017-12-22T04:15:43Z

src/rnnlm/rnnlm-core-training.cc

  computer.Run();  // This is the forward pass.

-  ProcessOutput(minibatch, derived, word_embedding,
+  ProcessOutput(true, minibatch, derived, word_embedding,


it's probably easier to read if you write
bool is_backstitch_step = true; ProcessOutput(is_backstitch_step, ....)

hainan-xv · 2017-12-22T04:16:17Z

src/rnnlm/rnnlm-core-training.cc

 }

+void RnnlmCoreTrainer::TrainBackstitch(
+    bool is_backstitch_step1,


1 means the 1st step in the backstitch training.

hainan-xv · 2017-12-22T04:20:59Z

src/rnnlm/rnnlm-core-training.h

                   "related to parallelization by model averaging.");
+    opts->Register("backstitch-training-scale", &backstitch_training_scale,
+                   "backstitch training factor. "
+                   "if 0 then in the normal training mode. It is referred as "


referred to something as

hainan-xv · 2017-12-22T04:22:45Z

scripts/rnnlm/train_rnnlm.sh

@@ -192,7 +197,7 @@ while [ $x -lt $num_iters ]; do
             --embedding.max-param-change=$embedding_max_change \


did you forget to add the option for embedding training?

hainan-xv · 2017-12-22T04:23:27Z

src/rnnlmbin/rnnlm-train.cc


+    core_config.backstitch_training_scale = backstitch_training_scale;
+    core_config.backstitch_training_interval = backstitch_training_interval;
+    embedding_config.backstitch_training_scale = backstitch_training_scale;


why do you make them the same if you have separate options for core_config and embedding_config?

actually i see you declared each option 3 times, but only the one defined in this .cc file take into effect. This is very weird.

I agree that it's weird-- I think it might be clearer if you just have two separate versions of the options that are both set from the command line, and just set them to the same values.

Have made them two separate options in the top-level shell script.

hainan-xv

I noticed a couple of small issues.

hainan-xv · 2017-12-23T03:22:05Z

src/rnnlm/rnnlm-core-training.cc


-  objf_info_.AddStats(weight, objf_num, objf_den, objf_den_exact);
+  if (is_backstitch_step1)
+    objf_info_.AddStats(weight, objf_num, objf_den, objf_den_exact);


Is there a reason why we are only doing this for the back step?

Doing this for the back step corresponds to the stats computed from the parameters updated after the whole 2-step update on a minibatch

hainan-xv · 2017-12-23T03:23:38Z

egs/ami/s5b/local/rnnlm/tuning/run_lstm_tdnn_bs_1a.sh

+mic=sdm1
+stage=-10
+train_stage=0
+alpha=0.8


add some comments on what the 2 variables are?

hainan-xv · 2017-12-23T03:24:18Z

egs/ami/s5b/local/rnnlm/tuning/run_lstm_tdnn_bs_1a.sh

+fi
+
+if [ $stage -le 3 ]; then
+  backstitch_opt="--rnnlm.backstitch-scale $alpha --rnnlm.backstitch-interval $back_interval --embedding.backstitch-scale $alpha --embedding.backstitch-interval $back_interval"


this line might be too long

hainan-xv · 2017-12-23T03:25:37Z

scripts/rnnlm/train_rnnlm.sh

                            # nnet learning rate times this factor.
+backstitch_scale=0.0        # backstitch training scale
+backstitch_interval=1       # backstitch training interval
 cmd=run.pl  # you might want to set this to queue.pl


I just noticed this here - @danpovey should we just change the default to queue.pl then?

I prefer to always leave the default of cmd at run.pl and have it always passed in from the command line.

hainan-xv · 2017-12-23T03:32:07Z

egs/ami/s5b/local/rnnlm/tuning/run_lstm_tdnn_bs_1a.sh

+  backstitch_opt="--rnnlm.backstitch-scale $alpha --rnnlm.backstitch-interval $back_interval --embedding.backstitch-scale $alpha --embedding.backstitch-interval $back_interval"
+  rnnlm/train_rnnlm.sh --embedding_l2 $embedding_l2 \
+                       --stage $train_stage \
+                       --num-epochs $epochs --cmd "queue.pl" $backstitch_opt $dir


Does this really work? It doesn't seem to me that rnnlm/train_rnnlm.sh has the 4 variables defined in this string. Shouldn't you use --backstitch-scale and --backstitch-interval here?

Sorry. Should be OK now.

hainan-xv

2nd pass review. The most important issue probably has to do with options to rnnlm/train_rnnlm.sh

hainan-xv

LGTM now. Though I would suggest running the script for at least one iterations just to make sure it still runs after all these changes.

danpovey · 2018-02-28T23:40:08Z

Looks like I may have overlooked merging this. I assume this is still good to merge?

freewym · 2018-02-28T23:41:12Z

It needs a little bit more test. I will let you know when it is ready.

…

On Wed, Feb 28, 2018 at 6:40 PM, Daniel Povey ***@***.***> wrote: Looks like I may have overlooked merging this. I assume this is still good to merge? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#2096 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADWAkjFAnjHtksoqwreRWjOFIRhLa37mks5tZePigaJpZM4RKU3T> .

-- Yiming Wang Department of Computer Science The Johns Hopkins University 3400 N. Charles St. Baltimore, MD 21218

freewym · 2018-03-03T21:22:41Z

@danpovey I am done with this PR.

…aldi-asr#2096)

hainan-xv reviewed Dec 22, 2017

View reviewed changes

freewym force-pushed the backstitch_rnnlm branch from 89e17d0 to 9d0e700 Compare December 23, 2017 02:59

hainan-xv reviewed Dec 23, 2017

View reviewed changes

freewym force-pushed the backstitch_rnnlm branch from e346b9e to fa9e7ef Compare March 3, 2018 21:21

freewym changed the title ~~WIP: Backstitch rnnlm~~ Backstitch rnnlm Mar 3, 2018

freewym added 4 commits March 3, 2018 17:40

backstitch support for RNNLM

86f9aa1

added support for plotting rnnlm objf

fd776f2

fix

16af7a3

fix

91576d2

freewym force-pushed the backstitch_rnnlm branch from fa9e7ef to 8047fc8 Compare March 3, 2018 22:50

moved l2 to the 2nd step of backstitch

8111d5d

freewym force-pushed the backstitch_rnnlm branch from 8047fc8 to 8111d5d Compare March 3, 2018 23:27

danpovey merged commit 03b0ea8 into kaldi-asr:master Mar 3, 2018

freewym deleted the backstitch_rnnlm branch March 3, 2018 23:45

LvHang pushed a commit to LvHang/kaldi that referenced this pull request Apr 14, 2018

[src,scripts,egs] Enable backstich training for RNNLM; add examples (k…

e6f5fea

…aldi-asr#2096)

Skaiste pushed a commit to Skaiste/idlak that referenced this pull request Sep 26, 2018

[src,scripts,egs] Enable backstich training for RNNLM; add examples (k…

2911779

…aldi-asr#2096)

		@@ -192,7 +197,7 @@ while [ $x -lt $num_iters ]; do
		--embedding.max-param-change=$embedding_max_change \

Conversation

freewym commented Dec 21, 2017

Uh oh!

hainan-xv Dec 22, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hainan-xv left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hainan-xv left a comment

Choose a reason for hiding this comment

Uh oh!

hainan-xv left a comment

Choose a reason for hiding this comment

Uh oh!

danpovey commented Feb 28, 2018

Uh oh!

freewym commented Feb 28, 2018 via email

Uh oh!

freewym commented Mar 3, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

hainan-xv Dec 22, 2017 •

edited

Loading