Cosmetic and other minor fixes for segmentation #1784

vimalmanohar · 2017-07-31T22:58:41Z

No description provided.

danpovey

I think we're making progress. I have a lot of comments but they are all very minor.

danpovey · 2017-07-31T23:03:52Z

egs/wsj/s5/steps/resolve_ctm_overlaps.py

@@ -0,0 +1,322 @@
+#! /usr/bin/env python


This is the kind of script that should go in utils/, probably in utils/ctm/. Please create a directory utils/ctm/, put this there, put fix_ctm.sh and convert_ctm.pl there, and make soft links from their old locations.

danpovey · 2017-07-31T23:26:31Z

egs/wsj/s5/utils/data/get_frame_shift.sh

-head -n 10 $dir/utt2dur | paste - $temp | \
-   awk '{ dur += $2; frames += $4; } END { shift = dur / frames; if (shift > 0.0098 && shift < 0.0102) shift = 0.01; print shift; }' || exit 1;
+frame_shift=$(cat $dir/frame_shift)
+if [ -z "$frame_shift" ]; then


Failing silently here is a problem because not all invocations of this script check the return status: see steps/nnet3/chain/get_egs.sh. Probably best to fix wsj/s5/steps/nnet3/chain/get_egs.sh to check the status, but also add an error message here.

danpovey · 2017-07-31T23:29:36Z

egs/wsj/s5/utils/data/get_reco2utt_for_data.sh

+# Apache 2.0
+
+if [ $# -ne 1 ]; then
+  echo "This script outputs a mapping from recording to a list of utterances "


This script doesn't have a very good, or well-described, interface. It seems that in addition to doing what it says it does, it also creates a segments file if one did not already exist. I don't see why that behavior would be desired; and I don't even see why it's necessary, because you could easily do it in a pipe.
I don't recommend to have it create the segments file, because for a script whose primary purpose is to output to stdout, to have a side effect, is very confusing.

danpovey · 2017-07-31T23:30:25Z

egs/wsj/s5/utils/data/internal/perturb_volume.py

+        a new wav.scp file.
+        If --reco2vol is provided, then for each recording, the volume factor
+        specified in that file is applied.
+        Otherwise, a volume factor is chosen randomly between


randomly -> from a uniform distribution

danpovey · 2017-07-31T23:36:29Z

egs/wsj/s5/utils/data/resample_data_dir.sh

+if [ $# -ne 2 ]; then
+  echo "This script adds a sox line in wav.scp to resample the audio at a "
+  echo "different sampling-rate"
+  echo "Usage: $0 <frequency> <data-dir>"


need example.

danpovey · 2017-08-01T00:27:45Z

src/nnet3bin/nnet3-compute-from-egs.cc

@@ -36,7 +36,8 @@ class NnetComputerFromEg {

  // Compute the output (which will have the same number of rows as the number
  // of Indexes in the output of the eg), and put it in "output".


This documentation is wrong now that you have changed the code.

danpovey · 2017-08-01T00:34:20Z

src/nnet3bin/nnet3-get-egs-dense-targets.cc

+                "in compressed format (recommended).  Update: this is now "
+                "only relevant if the features being read are un-compressed; "
+                "if already compressed, we keep we same compressed format when "
+                "dumping-egs.");


danpovey · 2017-08-01T00:35:19Z

src/nnet3bin/nnet3-get-egs-dense-targets.cc

-    po.Register("compress", &compress, "If true, write egs in "
-                "compressed format.");
+    po.Register("compress", &compress, "If true, write egs with input features "
+                "in compressed format (recommended).  Update: this is now "


Replace 'Update: this is now only' with 'This is only'. We don't document the changes to options, except if likely to break things, because the average user won't be aware of, and won't care about, the history.

danpovey · 2017-08-01T00:35:41Z

src/nnet3bin/nnet3-get-egs-dense-targets.cc

@@ -197,6 +197,11 @@ int main(int argc, char *argv[]) {
                "--online-ivectors option");
    po.Register("length-tolerance", &length_tolerance, "Tolerance for "
                "difference in num-frames between feat and ivector matrices");
+    po.Register("targets-length-tolerance", &targets_length_tolerance, 
+                "Tolerance for "
+                "difference in num-frames after subsampling between "


parenthesize (after subsampling)

danpovey · 2017-08-01T00:35:48Z

src/nnet3bin/nnet3-get-egs-dense-targets.cc

+    po.Register("targets-length-tolerance", &targets_length_tolerance, 
+                "Tolerance for "
+                "difference in num-frames after subsampling between "
+                "feat and target matrices");


feat -> feature

danpovey

Some very small comments.

danpovey · 2017-08-01T21:44:51Z

egs/wsj/s5/utils/data/get_frame_shift.sh

-   awk '{ dur += $2; frames += $4; } END { shift = dur / frames; if (shift > 0.0098 && shift < 0.0102) shift = 0.01; print shift; }' || exit 1;
+frame_shift=$(cat $dir/frame_shift)
+if [ -z "$frame_shift" ]; then
+  echo "$0: Could not read get frame shift from directory $dir"


you need to be redirecting this to stderr.

danpovey · 2017-08-01T21:46:30Z

egs/wsj/s5/utils/data/perturb_data_dir_volume.sh

@@ -7,6 +7,17 @@
 # the wav.scp to perturb the volume (typically useful for training data when
 # using systems that don't have cepstral mean normalization).



how much testing have you done with this? Can you please test it with directories that have pipes and those that have raw wav files, and make sure it doesn't break in a very obvious way?

I tested for various cases. Also addressed all other comments.

danpovey · 2017-08-01T21:51:12Z

src/nnet3/nnet-example.h


  /// This constructor creates NnetIo with name "name", indexes with n=0, x=0,
-  /// and t values ranging from t_begin to t_begin + feats.NumRows() - 1, and
+  /// and t values ranging from t_begin to t_begin + t_stride * feats.NumRows() - 1


need parenthesis in excpression (also below), and watch line length.

danpovey · 2017-08-01T21:52:22Z

src/nnet3bin/nnet3-compute-from-egs.cc

-  // of Indexes in the output of the eg), and put it in "output".
-  void Compute(const NnetExample &eg, Matrix<BaseFloat> *output) {
+  // of Indexes in the output with the name 'output_name' of the eg), 
+  // and put it in "output".


this is a pre-existing problem, but please change "output" to '*output'.

danpovey · 2017-08-01T21:53:00Z

src/nnet3bin/nnet3-compute-from-egs.cc


    ParseOptions po(usage);
    po.Register("binary", &binary_write, "Write output in binary mode");
    po.Register("apply-exp", &apply_exp, "If true, apply exp function to "
                "output");
+    po.Register("output-name", &output_name, "Do computation for "
+                "specified output");


output -> output-node.

danpovey · 2017-08-01T21:53:53Z

src/nnet3bin/nnet3-get-egs.cc

@@ -179,10 +177,10 @@ int main(int argc, char *argv[]) {
    ParseOptions po(usage);

    po.Register("compress", &compress, "If true, write egs with input features "
-                "in compressed format (recommended).  Update: this is now "
+                "in compressed format (recommended).  Update: this is "


Remove 'Update: '

vimalmanohar · 2017-08-03T21:14:08Z

If there are no more changes, I can rebase and create a cleaner PR.

danpovey · 2017-08-03T23:34:12Z

I forgot about this. It's OK, I'm going to squash and merge.

* 'master' of https://github.com/kaldi-asr/kaldi: [egs] Update tedlium and aspire recipes to use xconfigs (avoid crash) (kaldi-asr#1790) [egs] fix bugs in Multi-database English LVCSR recipe (kaldi-asr#1785) [src,egs,scripts] Cosmetic and other minor fixes, some required for segmentation PR (kaldi-asr#1784) [egs] update multi_condition script in swbd (kaldi-asr#1788) [src] Circumvent Visual Studio 2017 bug regarding name resolution (kaldi-asr#1783) [tools] adding phonetisaurus install scripts (PR#1734) [scripts] update nnet3 scripts to fix bug where rejecting 'bad' models was not happening (kaldi-asr#1777) [src] make dithering in feature processing more efficient by using random state, thanks: liximin244@gmail.com

…egmentation PR (kaldi-asr#1784)

vimalmanohar added 30 commits May 18, 2017 13:58

long_utts: Minor fix

58fcd61

Merge branch 'master' of github.com:kaldi-asr/kaldi

df90bba

Merge branch 'master' of github.com:kaldi-asr/kaldi

ff4ac04

Merge branch 'master' of github.com:kaldi-asr/kaldi

29ced2a

Merge branch 'master' of github.com:kaldi-asr/kaldi

0f69bbd

Merge branch 'master' of github.com:kaldi-asr/kaldi

143877b

Merge branch 'master' of github.com:kaldi-asr/kaldi

acf4c14

Merge branch 'master' of github.com:kaldi-asr/kaldi

f887582

Merge branch 'master' of github.com:kaldi-asr/kaldi

a5d4ca8

Merge branch 'master' of github.com:kaldi-asr/kaldi

ea930eb

Merge branch 'master' of github.com:kaldi-asr/kaldi

8f05cdf

Merge branch 'master' of github.com:kaldi-asr/kaldi

1eb41ee

Merge branch 'master' of github.com:kaldi-asr/kaldi

79e15ad

Merge branch 'master' of github.com:kaldi-asr/kaldi

61e94b2

Merge branch 'master' of github.com:kaldi-asr/kaldi

d45ecb9

Merge branch 'master' of github.com:kaldi-asr/kaldi

7b01bb0

Merge branch 'master' of github.com:kaldi-asr/kaldi

e1de4e4

Merge branch 'master' of github.com:kaldi-asr/kaldi

41952cd

asr_sad: Squashing commits

6ad6533

asr_sad: Making some changes based on comments

a1b07a2

asr_sad: Fixing some more issues

85345ad

sad_asr: Fixed frame skip

7df4513

asr_sad: Removing some old files

bbd9632

asr_sad: Fixed bug

8bf583e

Merge branch 'master' of github.com:kaldi-asr/kaldi

2e2b3d1

Merge branch 'master' of github.com:kaldi-asr/kaldi

51c32f7

sad: Fixing resolve_ctm_overlaps

a5ed583

Merge branch 'master' of github.com:kaldi-asr/kaldi

232397e

Merge branch 'master' of github.com:kaldi-asr/kaldi

1414f6f

Merge branch 'master' of github.com:kaldi-asr/kaldi

c65ef65

vimalmanohar added 20 commits June 29, 2017 20:56

Bug fixes and missing files added

7ee8f05

sad: Addressing comments

d898add

sad: Addressed all comments

787b964

sad: Fixed bug in merge-targets.py

c24261e

sad: Minor fix in renorm layer

a1f3d5d

Merging from master

d8f5997

sad: Minor fixes

ff41174

sad: Forcing features to be plp

c4a6aa3

sad : Minor fixes

dc5b545

sad: Fixing changes based on comments

be46861

sad: Fixed bugs

4d0555d

sad: Add more checks

1160429

sad: Creating prepare_targets script in steps

c6e10a6

Merging golden master

450a385

sad: Minor fixes

2f5bf5d

asr_sad: minor bug fixes

1918c9d

asr_sad: Minor bug fix in resolving ctm overlaps

29f88ee

asr_sad: Adding recipe for switchboard

e8c0949

asr_sad: Changes based on comments

759640f

Removing segmentation related changes

bec03e7

danpovey reviewed Aug 1, 2017

View reviewed changes

asr_sad_prepare: Addressed all the comments.

4abdf3e

vimalmanohar force-pushed the asr_sad_prepare branch from 9b655fe to 4abdf3e Compare August 1, 2017 04:23

danpovey reviewed Aug 1, 2017

View reviewed changes

asr_sad_prepare: Minor changes

c7f4c7a

danpovey merged commit 35faa4d into kaldi-asr:master Aug 3, 2017

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

[src,egs,scripts] Cosmetic and other minor fixes, some required for s…

6210fdb

…egmentation PR (kaldi-asr#1784)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cosmetic and other minor fixes for segmentation #1784

Cosmetic and other minor fixes for segmentation #1784

vimalmanohar commented Jul 31, 2017

danpovey left a comment

danpovey Jul 31, 2017

danpovey Jul 31, 2017

danpovey Jul 31, 2017

danpovey Jul 31, 2017

danpovey Jul 31, 2017

danpovey Aug 1, 2017

danpovey Aug 1, 2017

danpovey Aug 1, 2017

danpovey Aug 1, 2017

danpovey Aug 1, 2017

danpovey left a comment

danpovey Aug 1, 2017

danpovey Aug 1, 2017

vimalmanohar Aug 2, 2017

danpovey Aug 1, 2017

danpovey Aug 1, 2017

danpovey Aug 1, 2017

danpovey Aug 1, 2017

vimalmanohar commented Aug 3, 2017

danpovey commented Aug 3, 2017

		@@ -36,7 +36,8 @@ class NnetComputerFromEg {

		// Compute the output (which will have the same number of rows as the number
		// of Indexes in the output of the eg), and put it in "output".

		@@ -7,6 +7,17 @@
		# the wav.scp to perturb the volume (typically useful for training data when
		# using systems that don't have cepstral mean normalization).

Cosmetic and other minor fixes for segmentation #1784

Cosmetic and other minor fixes for segmentation #1784

Conversation

vimalmanohar commented Jul 31, 2017

danpovey left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danpovey left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vimalmanohar commented Aug 3, 2017

danpovey commented Aug 3, 2017