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.

Sign up for GitHub

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

Jump to bottom

Added PerDimWeightedAverage component for chain models in SWBD, works… #462

Merged
merged 1 commit into from
Jan 21, 2016
Merged

Added PerDimWeightedAverage component for chain models in SWBD, works… #462

merged 1 commit into from
Jan 21, 2016

Conversation

vijayaditya
Copy link
Contributor

… as good as jesus (tdnn_4f) model

This is a re-submission of PR #461

@danpovey
Copy link
Contributor

You added egs/wsj/s5/steps/nnet3/tdnn/train.sh. How does this differ from
nnet3/train_tdnn.sh and would there now be duplication?

On Wed, Jan 20, 2016 at 10:08 PM, Vijayaditya Peddinti <
notifications@github.com> wrote:

… as good as jesus (tdnn_4f) model

This is a re-submission of PR #461

#461

You can view, comment on, or merge this pull request online at:

#462
Commit Summary

  • Added PerDimWeightedAverage component for chain models in SWBD,
    works as good as jesus (tdnn_4f) model

File Changes

Patch Links:

—
Reply to this email directly or view it on GitHub
#462.

@vijayaditya
Copy link
Contributor Author

There is duplication. I would ideally just keep
steps/nnet3/tdnn/{train.sh,make_configs.py} as it is similar in directory
structure to steps/nnet3/lstm/{train.sh,makes_configs.py}.

I will gradually change all existing local/* scripts to use
steps/nnet3/tdnn/* scripts and then delete steps/nnet3/train_tdnn.sh script
and the corresponding make_tdnn_configs.py scripts.

On Wed, Jan 20, 2016 at 7:17 PM, Daniel Povey notifications@github.com
wrote:

You added egs/wsj/s5/steps/nnet3/tdnn/train.sh. How does this differ from
nnet3/train_tdnn.sh and would there now be duplication?

On Wed, Jan 20, 2016 at 10:08 PM, Vijayaditya Peddinti <
notifications@github.com> wrote:

… as good as jesus (tdnn_4f) model

This is a re-submission of PR #461

#461

You can view, comment on, or merge this pull request online at:

#462
Commit Summary

  • Added PerDimWeightedAverage component for chain models in SWBD,
    works as good as jesus (tdnn_4f) model

File Changes

Patch Links:

—
Reply to this email directly or view it on GitHub
#462.

—
Reply to this email directly or view it on GitHub
#462 (comment).

danpovey
danpovey reviewed Jan 21, 2016
View reviewed changes
egs/swbd/s5c/local/chain/run_tdnn_4q.sh
# it has a different splicing configuration
# it uses the PerDimensionWeightedAverage pooling in place of the Jesus layer

# it relies on new steps/nnet3/chain/train_tdnn_b.sh script which accepts more
Copy link
Contributor

Choose a reason for hiding this comment

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

the script doesn't have the _b name now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oops.. changing it

danpovey added a commit that referenced this pull request Jan 21, 2016
@danpovey
Merge pull request #462 from vijayaditya/chain_feature2
763676a 
Added PerDimWeightedAverage component for chain models in SWBD, works…
@danpovey danpovey merged commit 763676a into kaldi-asr:chain Jan 21, 2016
@danpovey
Copy link
Contributor

already merged, include it next time you make a change.

@vijayaditya
Copy link
Contributor Author

OK.

vijayaditya added a commit to vijayaditya/kaldi that referenced this pull request Jan 23, 2016
@vijayaditya
More fixes for bugs introduced due to PR kaldi-asr#462
0ab8e01
vijayaditya added a commit to vijayaditya/kaldi that referenced this pull request Jan 23, 2016
@vijayaditya
More fixes for bugs introduced due to PR kaldi-asr#462
bd0208f
danpovey added a commit that referenced this pull request Jan 23, 2016
@danpovey
Merge pull request #467 from vijayaditya/chain_feature2
50f2083 
More fixes for bugs introduced due to PR #462
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@vijayaditya @danpovey