-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Conformer Transducer Librispeech (Contribution from Samsung AI Cambridge) #1782
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
Conformer Transducer Librispeech (Contribution from Samsung AI Cambridge) #1782
Conversation
…into libri_conformer_large
asumagic
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.
Made some general remarks and pointed out at one issue in the hparams. Other than that, LGTM, and I will be retraining a model shortly.
recipes/LibriSpeech/ASR/transducer/hparams/conformer_transducer.yaml
Outdated
Show resolved
Hide resolved
|
Hey @TParcollet, any news please? Thanks. :-) |
|
Taking the liberty of pushing my current code (without streaming stuff, only the stuff that gets me to 2.8) but bear in mind I'm currently trying to fight git for a bit. |
|
Sure, ping me wheb you are ready for a review.
…On Tue, Jul 18, 2023, 9:12 AM asu ***@***.***> wrote:
Taking the liberty of pushing my current code (without streaming stuff,
only the stuff that gets me to 2.8) but bear in mind I'm currently trying
to fight git for a bit.
—
Reply to this email directly, view it on GitHub
<#1782 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AEA2ZVWHLTL7RHY3DOGXMRLXQ2DSVANCNFSM6AAAAAATPZ2MUE>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
82d72c8 to
167b976
Compare
|
I have yet to upload the checkpoint + edit the README with new results, I will do this tomorrow. The code side of things is ready if you prefer not to delay reviewing for it. With my changes, using gradient averaging, we reach a greedy-search There is one issue I had to work around: In |
|
@TParcollet, @asumagic thank you for this PR!
Regarding DDP, here are my questions:
|
|
@asumagic what about gating the custom_tgt_module as well if num_decoder_layers == 0? |
What do you mean precisely? Only instantiating the
Possibly, but I am not sure what is the best way to do it. If the caller code (i.e. not |
|
Updated README with all results + uploaded the checkpoint files. It took me a bit because I had to rerun evaluation. The current checkpoint includes the useless |
|
Since the Dropbox PR was merged, this now conflicts. Also I forgot that the But let's first decide what we should do regarding the Other than that, it should be all good AFAIK. |
|
If you can think of a quick and easy way of fixing the incompatibility that will arise with existing checkpoints and the removal of custom tgt, then we fix it. Otherwise we move forward |
|
In that case IMO we can move forward. I fixed minor conflicts against |
|
Thank you @asumagic,
|
Sure @mravanelli ! |
This PR refactors our transducer recipe for Librispeech. In practice, we will drop the current CRDNN that hasn't even been training on the full set with a better-performing conformer_transducer.
Todo in another PR