Skip to content

NormalizeComponent with add-log-std option #482

Merged
danpovey merged 7 commits intokaldi-asr:chainfrom
pegahgh:chain-normalization-with-log
Feb 6, 2016
Merged

NormalizeComponent with add-log-std option #482
danpovey merged 7 commits intokaldi-asr:chainfrom
pegahgh:chain-normalization-with-log

Conversation

@pegahgh
Copy link
Contributor

@pegahgh pegahgh commented Feb 3, 2016

No description provided.

Copy link
Contributor

Choose a reason for hiding this comment

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

you should call it add_log_stddev_, not add_log_std_.
Also, " is added as node to output layer." is not the right terminology as used in nnet3, its an extra dimension of the output.

@pegahgh
Copy link
Contributor Author

pegahgh commented Feb 3, 2016

changed option names to add-log-stddev.

Copy link
Contributor

Choose a reason for hiding this comment

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

if you don't need the assert statement then delete it.

@pegahgh pegahgh closed this Feb 3, 2016
@pegahgh pegahgh reopened this Feb 3, 2016
Copy link
Contributor

Choose a reason for hiding this comment

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

You wouldn't need to use a temporary matrix if you computed this before the other part of the input derivative.

@pegahgh
Copy link
Contributor Author

pegahgh commented Feb 4, 2016

fixed!

std::string NonlinearComponent::Info() const {
std::stringstream stream;
KALDI_ASSERT(InputDim() == OutputDim()); // always the case
stream << Type() << ", dim=" << InputDim();
Copy link
Contributor

Choose a reason for hiding this comment

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

You should extend the Info(), so that it's clear from the info string whether you have this extra feature.
Probably if the feature is included, it should say input-dim=xxx, output-dim=xxx, include-log-stddev=true

Copy link
Contributor

Choose a reason for hiding this comment

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

pegah, you still haven't done this (fix the info string)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had extended the NormalizeComponent::Info() to contain input-output dim if add-log-stddev is true.
I modified NonlinearComponent::Info() . Is this what you asked for?

@pegahgh
Copy link
Contributor Author

pegahgh commented Feb 4, 2016

Fixed!

@danpovey
Copy link
Contributor

danpovey commented Feb 6, 2016

I see- looks right.

danpovey added a commit that referenced this pull request Feb 6, 2016
NormalizeComponent with add-log-std option
@danpovey danpovey merged commit b86aeb3 into kaldi-asr:chain Feb 6, 2016
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