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

Fixes #109

Merged
merged 1 commit into from
Jul 6, 2017
Merged

Fixes #109

merged 1 commit into from
Jul 6, 2017

Conversation

SeanNaren
Copy link
Owner

Formatted decode script, added dels to benchmark and updated train to save best model

@ghost ghost assigned SeanNaren Jul 6, 2017
@ghost ghost added the in progress label Jul 6, 2017
@SeanNaren SeanNaren merged commit 5da825d into master Jul 6, 2017
@ghost ghost removed the in progress label Jul 6, 2017
@SeanNaren SeanNaren deleted the fixes branch July 6, 2017 13:15
@SiddGururani
Copy link
Contributor

About saving the best model, I noticed that you use the WER to determine whether the model is getting better or worse. Shouldn't we be using the loss on the validation set instead? In most of my experiments, I've noticed that the WER might converge to a value but the loss starts increasing after several epochs.

@SeanNaren
Copy link
Owner Author

@SiddGururani problem is I see that the validation loss diverges after a while, which doesn't usually reflect how well the WER/CER is... I was doing an average over the 2 but I thought WER was representative enough!

@@ -372,6 +376,13 @@ def main():
optimizer.load_state_dict(optim_state)
print('Learning rate annealed to: {lr:.6f}'.format(lr=optim_state['param_groups'][0]['lr']))

if best_wer is None or best_wer > wer:
print("Found better validated model, saving to %s" % args.final_model_path)
Copy link

@mit456 mit456 Jul 7, 2017

Choose a reason for hiding this comment

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

Shouldn't it be args.model_path? @SeanNaren

Copy link
Owner Author

Choose a reason for hiding this comment

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

Thanks for that, fixed here

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.

3 participants