Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

change --frame argument #1389

Merged
merged 1 commit into from
Feb 11, 2017
Merged

change --frame argument #1389

merged 1 commit into from
Feb 11, 2017

Conversation

LvHang
Copy link
Contributor

@LvHang LvHang commented Feb 1, 2017

@danpovey
Related to #1370 . Attention, this is in "shortcut" branch, not "master" branch.

Hi Dan,

  1. Following you interpretation, I change the code about how to compute the --frame option.
  2. I rewrite the tedlium/s5_r2/local/nnet3/run_tdnn.sh so that it will call steps/nnet3/train_dnn.py to train the model. The annotation (line 78-88) is the original shell version. The code (line 90-130) is what I converted.
  3. The WER result is followed:
    | after-change | ori
    decode-dev | 11.7 | 12.6
    decode_dev_rescore | 10.9 | 11.5
    decode_test | 11.8 | 11.9
    decode_test_rescore | 11.2 | 10.9
    (The result in "ori" column is extracted from "RESULT" file in tedlium/s5_r2)

Hang

@danpovey
Copy link
Contributor

danpovey commented Feb 1, 2017

Interesting. I'm not sure what run_tdnn_py.sh was modified from; anyway it is not following the correct pattern of these filenames; get Yiming to explain the system to you.
I think it's better if you rerun the baseline; numbers in the RESULTS file may be out of date.
Also the system that you are using is not up-to-date at all (it uses the deprecated config-generating scripts) so I would not want to check this in. But it doesn't matter for purposes of testing the change. Just run it again without the change. If the egs were not deleted, then re-use them via the common-egs-dir option.

@LvHang
Copy link
Contributor Author

LvHang commented Feb 1, 2017

  1. The "run_tdnn_py.sh" was modified from "tedlium/s5_r2/local/nnet3/run_tdnn.sh". In the 12th stage of "run_tdnn.sh", it call the shell script--"steps/nnet3/tdnn/train.sh". But in Change how the --frame argument is chosen in TDNN training. #1370 , I think you let me modify the "train_dnn.py". So I rewrite it. I asked Yiming and move them to tuning directory.
  2. I will rerun the baseline right now.
  3. You said the config-generating scripts is out-of-date. However, I check the "master" and "shortcut" branch in your repository, all of them use the "steps/nnet3/tdnn/make_configs.py". I guess you mean we need to write xconfig and use "steps/nnet3/xconfig_to_configs.py", right?
    Hang

@danpovey
Copy link
Contributor

danpovey commented Feb 1, 2017 via email

@LvHang
Copy link
Contributor Author

LvHang commented Feb 3, 2017

Hi Dan,
I have already run the baseline.The WER result is below:
          | after-change | baseline
decode-dev      |  11.7   | 11.7
decode_dev_rescore  | 10.9    | 11.0
decode_test      | 11.8    | 11.6
decode_test_rescore  | 11.2    | 11.0
Do you think this is what you expect?
If you think it's ok, I'll use the xconfigs.

@danpovey
Copy link
Contributor

danpovey commented Feb 3, 2017

I think those differences are just the normal differences we see when we rerun.
I'll merge the change but I don't need any example script, just leave the one-line change
of common.py.

@LvHang
Copy link
Contributor Author

LvHang commented Feb 3, 2017

Ok. I need to modify it on "master" branch or "shortcut" branch? Thanks

@danpovey
Copy link
Contributor

danpovey commented Feb 3, 2017 via email

@LvHang
Copy link
Contributor Author

LvHang commented Feb 3, 2017

Update.
Thank you for giving me the chance to read the process of nnet3. Can I help something others? Thanks!
Hang

@danpovey danpovey merged commit 5b03ada into kaldi-asr:shortcut Feb 11, 2017
david-ryan-snyder pushed a commit to david-ryan-snyder/kaldi that referenced this pull request Apr 12, 2017
… training (kaldi-asr#1389)

... makes it vary on each iteration, not in big chunks of time.
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