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

Adding Conformer model #1327

Merged
merged 75 commits into from Nov 3, 2020
Merged

Adding Conformer model #1327

merged 75 commits into from Nov 3, 2020

Conversation

VahidooX
Copy link
Collaborator

@VahidooX VahidooX commented Oct 21, 2020 •
edited

Here are some of the main changes:

  • Added the modules needed for Conformer
  • Added use_start_end_tokens to the data layers to support dropping these tokens
  • Updated our CTC loss to support different reduction methods including 'mean_batch'. User may select other reduction approaches from Pytorch. User may set it via ctc_reduction param added to the model config.
  • Added log_prediction parameter to model's config to control if we want to see prediction samples in output or not
  • Added LSTM decoder and Swish activation
  • Added NoamScheduler
  • Added subsampling module which supports VGGNet and striding approach for subsampling
  • Added multi-head attention, relative multi-head attention along with positional embedding and relative positional embedding
  • Fixed the bug in the data layer which added some paddings after normalization (fixing this bug would fail the tests! going to investigate it)

@VahidooX
Added initial code of Conformer.
543466b 
Signed-off-by: Vahid <vnoroozi@nvidia.com>
@VahidooX
Added initial code of Conformer.
99e9233 
Signed-off-by: Vahid <vnoroozi@nvidia.com>
@VahidooX
Added log_every_n_steps.
428a72d 
Signed-off-by: Vahid <vnoroozi@nvidia.com>
@lgtm-com
Copy link

lgtm-com bot commented Oct 21, 2020

This pull request introduces 3 alerts when merging 428a72d into 4e4597a - view on LGTM.com

new alerts:

  • 2 for Signature mismatch in overriding method
  • 1 for Unused local variable

@VahidooX
dropped older multi_head_att modules.
56770ad 
Signed-off-by: Vahid <vnoroozi@nvidia.com>
@VahidooX
Dropped dropout_in and params.
7213125 
Signed-off-by: Vahid <vnoroozi@nvidia.com>
@VahidooX
Fixed code style.
a4c4c79 
Signed-off-by: Vahid <vnoroozi@nvidia.com>
@lgtm-com
Copy link

lgtm-com bot commented Oct 21, 2020

This pull request introduces 3 alerts when merging a4c4c79 into 4e4597a - view on LGTM.com

new alerts:

  • 2 for Signature mismatch in overriding method
  • 1 for Unused import

@VahidooX
Updated docs.
2c900aa 
Signed-off-by: Vahid <vnoroozi@nvidia.com>
@lgtm-com
Copy link

lgtm-com bot commented Oct 22, 2020

This pull request introduces 3 alerts when merging 2c900aa into 4e4597a - view on LGTM.com

new alerts:

  • 2 for Signature mismatch in overriding method
  • 1 for Unused import

@VahidooX
Removed unused import.
ac86cb3 
Signed-off-by: Vahid <vnoroozi@nvidia.com>
@lgtm-com
Copy link

lgtm-com bot commented Oct 22, 2020

This pull request introduces 2 alerts when merging ac86cb3 into c09012b - view on LGTM.com

new alerts:

  • 2 for Signature mismatch in overriding method

@VahidooX
fixed docs.
75aa1e4 
Signed-off-by: Vahid <vnoroozi@nvidia.com>
@lgtm-com
Copy link

lgtm-com bot commented Oct 22, 2020

This pull request introduces 2 alerts when merging 75aa1e4 into c09012b - view on LGTM.com

new alerts:

  • 2 for Signature mismatch in overriding method

@VahidooX
Fixed license header.
25fff7f 
Signed-off-by: Vahid <vnoroozi@nvidia.com>
@lgtm-com
Copy link

lgtm-com bot commented Oct 22, 2020

This pull request introduces 2 alerts when merging 25fff7f into c09012b - view on LGTM.com

new alerts:

  • 2 for Signature mismatch in overriding method

@VahidooX
Fixed license header.
09d217c 
Signed-off-by: Vahid <vnoroozi@nvidia.com>
@lgtm-com
Copy link

lgtm-com bot commented Oct 22, 2020

This pull request introduces 2 alerts when merging 09d217c into c09012b - view on LGTM.com

new alerts:

  • 2 for Signature mismatch in overriding method

@VahidooX
fixed style.
cda38ae 
Signed-off-by: Vahid <vnoroozi@nvidia.com>
@lgtm-com
Copy link

lgtm-com bot commented Oct 22, 2020

This pull request introduces 2 alerts when merging cda38ae into c09012b - view on LGTM.com

new alerts:

  • 2 for Signature mismatch in overriding method

@VahidooX
Updated tests with _target_.
c24b0f6 
Signed-off-by: Vahid <vnoroozi@nvidia.com>
@lgtm-com
Copy link

lgtm-com bot commented Oct 22, 2020

This pull request introduces 2 alerts when merging c24b0f6 into 53b31f7 - view on LGTM.com

new alerts:

  • 2 for Signature mismatch in overriding method

@VahidooX
Updated tests with _target_.
7a61905 
Signed-off-by: Vahid <vnoroozi@nvidia.com>
@lgtm-com
Copy link

lgtm-com bot commented Oct 22, 2020

This pull request introduces 2 alerts when merging 7a61905 into 53b31f7 - view on LGTM.com

new alerts:

  • 2 for Signature mismatch in overriding method

@lgtm-com
Copy link

lgtm-com bot commented Oct 22, 2020

This pull request introduces 2 alerts when merging ef7a236 into 53b31f7 - view on LGTM.com

new alerts:

  • 2 for Signature mismatch in overriding method

@VahidooX
Fixed style.
3402668 
Signed-off-by: Vahid <vnoroozi@nvidia.com>
@lgtm-com
Copy link

lgtm-com bot commented Oct 31, 2020

This pull request introduces 1 alert when merging 6791136 into 75774cf - view on LGTM.com

new alerts:

  • 1 for Signature mismatch in overriding method

@VahidooX
fixed the feat_in bug.
dfeafd1 
Signed-off-by: Vahid <vnoroozi@nvidia.com>
@lgtm-com
Copy link

lgtm-com bot commented Nov 2, 2020

This pull request introduces 1 alert when merging dfeafd1 into 75774cf - view on LGTM.com

new alerts:

  • 1 for Signature mismatch in overriding method

@VahidooX
fixed log_predciction.
a74a327 
Signed-off-by: Vahid <vnoroozi@nvidia.com>
@lgtm-com
Copy link

lgtm-com bot commented Nov 2, 2020

This pull request introduces 1 alert when merging a74a327 into 75774cf - view on LGTM.com

new alerts:

  • 1 for Signature mismatch in overriding method

blisc
blisc requested changes Nov 2, 2020
View reviewed changes
Copy link
Collaborator

@blisc blisc left a comment

Choose a reason for hiding this comment

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

Mostly minor issues.

examples/asr/experimental/conformer/conformer_bpe.yaml Outdated Show resolved Hide resolved
examples/asr/experimental/conformer/conformer_char.yaml Outdated Show resolved Hide resolved
nemo/collections/asr/models/ctc_bpe_models.py Outdated Show resolved Hide resolved
nemo/collections/asr/models/ctc_bpe_models.py Outdated Show resolved Hide resolved
nemo/collections/asr/models/ctc_models.py Outdated Show resolved Hide resolved
nemo/collections/asr/modules/multi_head_attention.py Outdated Show resolved Hide resolved
nemo/collections/asr/modules/multi_head_attention.py Outdated Show resolved Hide resolved
nemo/collections/asr/modules/multi_head_attention.py Outdated Show resolved Hide resolved
nemo/collections/asr/modules/multi_head_attention.py Outdated Show resolved Hide resolved
nemo/collections/asr/modules/subsampling.py Outdated Show resolved Hide resolved
@VahidooX
fixed log bug.
6600de9 
Signed-off-by: Vahid <vnoroozi@nvidia.com>
@VahidooX
fixed style.
4b314db 
Signed-off-by: Vahid <vnoroozi@nvidia.com>
@lgtm-com
Copy link

lgtm-com bot commented Nov 2, 2020

This pull request introduces 1 alert when merging 4b314db into 75774cf - view on LGTM.com

new alerts:

  • 1 for Signature mismatch in overriding method

@VahidooX
added pos_emb_max_len.
58f9588 
Signed-off-by: Vahid <vnoroozi@nvidia.com>
@VahidooX
added pos_emb_max_len.
306ddf6 
Signed-off-by: Vahid <vnoroozi@nvidia.com>
@lgtm-com
Copy link

lgtm-com bot commented Nov 2, 2020

This pull request introduces 1 alert when merging 306ddf6 into 75774cf - view on LGTM.com

new alerts:

  • 1 for Signature mismatch in overriding method

@VahidooX
moved subsampling.
7dabb78 
Signed-off-by: Vahid <vnoroozi@nvidia.com>
@lgtm-com
Copy link

lgtm-com bot commented Nov 2, 2020

This pull request introduces 1 alert when merging 7dabb78 into 75774cf - view on LGTM.com

new alerts:

  • 1 for Signature mismatch in overriding method

@VahidooX
added open_dict()
90cb95c 
Signed-off-by: Vahid <vnoroozi@nvidia.com>
@lgtm-com
Copy link

lgtm-com bot commented Nov 2, 2020

This pull request introduces 1 alert and fixes 1 when merging 90cb95c into 75774cf - view on LGTM.com

new alerts:

  • 1 for Signature mismatch in overriding method

fixed alerts:

  • 1 for 'import *' may pollute namespace

@VahidooX
moved conformerblock.
353e652 
Signed-off-by: Vahid <vnoroozi@nvidia.com>
@lgtm-com
Copy link

lgtm-com bot commented Nov 3, 2020

This pull request introduces 1 alert and fixes 1 when merging 353e652 into ece60b0 - view on LGTM.com

new alerts:

  • 1 for Signature mismatch in overriding method

fixed alerts:

  • 1 for 'import *' may pollute namespace

@VahidooX
fixed import bug.
5e974b8 
Signed-off-by: Vahid <vnoroozi@nvidia.com>
@lgtm-com
Copy link

lgtm-com bot commented Nov 3, 2020

This pull request introduces 1 alert and fixes 1 when merging 5e974b8 into ece60b0 - view on LGTM.com

new alerts:

  • 1 for Signature mismatch in overriding method

fixed alerts:

  • 1 for 'import *' may pollute namespace

@VahidooX
fixed code style.
3eef062 
Signed-off-by: Vahid <vnoroozi@nvidia.com>
@lgtm-com
Copy link

lgtm-com bot commented Nov 3, 2020

This pull request introduces 1 alert and fixes 1 when merging 3eef062 into ece60b0 - view on LGTM.com

new alerts:

  • 1 for Signature mismatch in overriding method

fixed alerts:

  • 1 for 'import *' may pollute namespace

@lgtm-com
Copy link

lgtm-com bot commented Nov 3, 2020

This pull request introduces 1 alert and fixes 1 when merging 9c1a652 into f0eeaed - view on LGTM.com

new alerts:

  • 1 for Signature mismatch in overriding method

fixed alerts:

  • 1 for 'import *' may pollute namespace

blisc
blisc approved these changes Nov 3, 2020
View reviewed changes
Copy link
Collaborator

@blisc blisc left a comment

Choose a reason for hiding this comment

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

LGTM

nemo/collections/asr/parts/conformer_modules.py Outdated Show resolved Hide resolved
titu1994
titu1994 approved these changes Nov 3, 2020
View reviewed changes
Copy link
Collaborator

@titu1994 titu1994 left a comment

Choose a reason for hiding this comment

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

LGTM

@VahidooX
added conformerencoder to all.
302b9f3 
Signed-off-by: Vahid <vnoroozi@nvidia.com>
@lgtm-com
Copy link

lgtm-com bot commented Nov 3, 2020

This pull request introduces 1 alert and fixes 1 when merging 302b9f3 into f0eeaed - view on LGTM.com

new alerts:

  • 1 for Signature mismatch in overriding method

fixed alerts:

  • 1 for 'import *' may pollute namespace

@VahidooX VahidooX merged commit 61ab906 into NVIDIA:main Nov 3, 2020
titu1994 pushed a commit to titu1994/NeMo that referenced this pull request Nov 4, 2020
@VahidooX @titu1994
Adding Conformer model (NVIDIA#1327)
77303c8
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@titu1994 titu1994 titu1994 approved these changes

@blisc blisc blisc approved these changes

@okuchaiev okuchaiev Awaiting requested review from okuchaiev

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@VahidooX @titu1994 @blisc