Skip to content

Supervision optimization#2302

Merged
danpovey merged 9 commits intokaldi-asr:masterfrom
chenzhehuai:supervision-opt
Mar 23, 2018
Merged

Supervision optimization#2302
danpovey merged 9 commits intokaldi-asr:masterfrom
chenzhehuai:supervision-opt

Conversation

@chenzhehuai
Copy link
Contributor

Supervision optimization to make the algorithm linear-time in the minibatch size, per minibatch, instead of quadratic-time.

  • by: change to call Concat(const Fst &fst1, MutableFst *fst2)

  • details:

previously we call "Concat(MutableFst<Arc> *fst1, const Fst<Arc> &fst2)" from AppendSupervision() by:

  for (int32 i = 0; i < input.size(); i++) {
    const Supervision &src = *(input[i]);
    fst::Concat(&output_supervision->back().fst, src.fst);
  }

now change to call "Concat(const Fst<Arc> &fst2, MutableFst<Arc> *fst1)" by

  for (int32 i =  input.size()-1; i >= 0; i--) {
    const Supervision &src = *(input[i]);
    fst::Concat(src.fst, &output_supervision->back().fst);
  }

  • run chain-supervision-test:

/export/a12/zchen/works/concat/chain-supervision-test.log

  • test whether the output of nnet3-chain-merge-egs is the same before and after modification:
zchen@g02:/export/a12/zchen/works/concat$  nnet3-chain-merge-egs --minibatch-size=256,128,64 ark:exp/chain/tdnn_lstm_1e_sp/egs/storage/2/cegs.1.ark ark,t:tmp/test3.txt
zchen@g02:/export/a12/zchen/works/concat$  nnet3-chain-merge-egs --minibatch-size=256,128,64 ark:exp/chain/tdnn_lstm_1e_sp/egs/storage/2/cegs.1.ark ark,t:tmp/test.txt
diff /export/a12/zchen/works/concat/tmp/test.txt /export/a12/zchen/works/concat/tmp/test3.txt

@chenzhehuai
Copy link
Contributor Author

chenzhehuai commented Mar 22, 2018

add things below

  1. simplify the interface AppendSupervision by: i) delete compactify 2) use single output_supervision instead of vector
  2. modify discriminative-supervision.cc correspondingly, along with test files and other calling functions
  3. test correctness:
zchen@g02:/export/a12/zchen/works/concat$ nnet3-chain-merge-egs --minibatch-size=256,128,64 ark:exp/chain/tdnn_lstm_1e_sp/egs/storage/2/cegs.1.ark ark,t:tmp/test4.txt
zchen@g02:/export/a12/zchen/works/concat$ diff tmp/test4.txt tmp/test3.txt     
  1. test speedup:
/export/a12/zchen/works/concat/test_time.sh

zchen@login:/export/a12/zchen/works/concat$ grep real watanabe*
watanabe.log:real       3m28.864s
watanabe.log.new:real   0m23.637s

Copy link
Contributor

@danpovey danpovey left a comment

Choose a reason for hiding this comment

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

A few cosmetic comments.
Please check the changes carefully, as we may not test all of them (e.g. the discriminative-training ones).

output_supervision->reserve(input.size());
for (int32 i = 0; i < input.size(); i++) {
(*output_supervision) = *(input[num_inputs-1]);
for (int32 i = num_inputs-2; i > -1; i--) {
Copy link
Contributor

Choose a reason for hiding this comment

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

here and elsewhere, please put space around the operator -, i.e. 'num_inputs - 2'. This is how we normally do it. And I prefer 'i >= 0' to 'i > -1'.

KALDI_ERR << "mismatch between inputs";
}
}
DiscriminativeSupervision &out_sup = (*output_supervision);
Copy link
Contributor

Choose a reason for hiding this comment

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

no need for parentheses.

DiscriminativeSupervision &out_sup = (*output_supervision)[i];
fst::TopSort(&(out_sup.den_lat));
out_sup.Check();
KALDI_ERR << "mismatch between inputs";
Copy link
Contributor

Choose a reason for hiding this comment

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

please make this a bit longer, and start with a capital.

Zhehuai Chen and others added 2 commits March 22, 2018 17:58
@chenzhehuai
Copy link
Contributor Author

done.
I also do a test on discriminative-supervision.cc, now it is the same

/export/a12/zchen/works/concat/test_disc.sh

@danpovey
Copy link
Contributor

thanks! Will merge when tests finish.

@danpovey danpovey merged commit 6dbe790 into kaldi-asr:master Mar 23, 2018
@chenzhehuai chenzhehuai deleted the supervision-opt branch March 23, 2018 03:17
LvHang pushed a commit to LvHang/kaldi that referenced this pull request Apr 14, 2018
Skaiste pushed a commit to Skaiste/idlak that referenced this pull request Sep 26, 2018
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