-
Notifications
You must be signed in to change notification settings - Fork 5.4k
About By-frame Dropout scripts Level PR #1372
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
Conversation
danpovey
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some small comments
| self.config['non-recurrent-projection-dim'] = \ | ||
| self.config['recurrent-projection-dim'] | ||
|
|
||
| if ((self.config['dropout-proportion'] > 1.0 or |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this checking code belongs in check_configs.
| configs.append("component name={0}.cr_trunc type=BackpropTruncationComponent " | ||
| "dim={1} {2}".format(name, cell_dim + rec_proj_dim, bptrunc_str)) | ||
| if lstm_dropout_value != -1.0: | ||
| configs.append("component name={0}.cr_trunc.dropout type=DropoutComponent dim={1} {2} {3}".format(name, cell_dim + rec_proj_dim, lstm_dropout_str, lstm_dropout_per_frame_str)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you should split this line.
| "dim-offset={1} dim={2}".format(name, cell_dim, rec_proj_dim)) | ||
| configs.append("### End LSTM Layer '{0}'".format(name)) | ||
| else: | ||
| configs.append("component-node name={0}.cr_trunc component={0}.cr_trunc " |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you could move the first and last of these lines out of the if-statement.
|
|
||
| if (self.config['dropout-per-frame'] != 'false' and | ||
| self.config['dropout-per-frame'] != 'true'): | ||
| raise xparser_error("dropout-per-frame has invalid value {0}.".format(self.config['dropout-per-frame'])) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please change all xparser_errors to be RuntimeError. IIRC this exception type has been removed.
| configs.append("component name={0}.g type=TanhComponent dim={1} {2}".format(name, cell_dim, repair_nonlin_str)) | ||
| configs.append("component name={0}.h type=TanhComponent dim={1} {2}".format(name, cell_dim, repair_nonlin_str)) | ||
| if lstm_dropout_value != -1.0: | ||
| configs.append("component name={0}.dropout type=DropoutComponent dim={1} {2} {3}".format(name, cell_dim, lstm_dropout_str, lstm_dropout_per_frame_str)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
watch line length. we're not being strict, but don't be ridiculous.
|
|
||
| lstm_dropout_value = self.config['dropout-proportion'] | ||
| lstm_dropout_str = 'dropout-proportion='+str(self.config['dropout-proportion']) | ||
| lstm_dropout_per_frame_value = self.config['dropout-per-frame'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this variable is unused, an since the name is longer than the rhs of the expression, there is not much need for it.
|
... also you should use booleans instead of strings as the config values, i.e. False, no 'false'. |
Related to #1248 (by Vimal , about dropout schedule ) #1324 (some C++ level by-frame code)
This PR include: 1. config generation python code 2. shell scripts
For this stage,