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

How2 speech translation recipe #1102

Merged
merged 19 commits into from Aug 20, 2019
Merged

How2 speech translation recipe #1102

merged 19 commits into from Aug 20, 2019

Conversation

hirofumi0810
Copy link
Collaborator

@hirofumi0810 hirofumi0810 commented Aug 17, 2019 •
edited

  • st1
  • asr1

@codecov
Copy link

codecov bot commented Aug 17, 2019 •
edited

Codecov Report

Merging #1102 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1102   +/-   ##
=======================================
  Coverage   78.83%   78.83%           
=======================================
  Files          93       93           
  Lines        8580     8580           
=======================================
  Hits         6764     6764           
  Misses       1816     1816

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 74a9648...ec7aebb. Read the comment docs.

sw005320
sw005320 reviewed Aug 17, 2019
View reviewed changes
Copy link
Contributor

@sw005320 sw005320 left a comment

Choose a reason for hiding this comment

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

LGTM.
Do you have some results? Can you add them as a RESULTS.md?

egs/how2/asr1/conf/tuning/decode_pytorch_transformer.yaml
@@ -0,0 +1,7 @@
batchsize: 0
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems that the decoding configs of RNN and transformer are very different. If you don't test either of them, you can remove that config.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I want to keep this as it is.

@sw005320 sw005320 added this to the v.0.5.2 milestone Aug 17, 2019
@hirofumi0810
Copy link
Collaborator Author

Do you have some results? Can you add them as a RESULTS.md?

Let me do this in another PR. (due to some personal issues)

@sw005320
Copy link
Contributor

Thanks. Please let me know when it is ready to merge.

@hirofumi0810
Copy link
Collaborator Author

This PR is ready to be merged.

@sw005320 sw005320 merged commit ed7b11a into espnet:master Aug 20, 2019
@sw005320
Copy link
Contributor

Thanks a lot, @hirofumi0810!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sw005320 sw005320 sw005320 left review comments

Assignees
No one assigned
Labels
Recipe
Projects
None yet
Milestone
v.0.5.2
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@hirofumi0810 @sw005320