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

Updates to conformer. #1898

Merged
merged 19 commits into from
Mar 18, 2021
Merged

Updates to conformer. #1898

merged 19 commits into from
Mar 18, 2021

Conversation

VahidooX
Copy link
Collaborator

@VahidooX VahidooX commented Mar 11, 2021 •
edited
Loading

  • Made positional embedding more efficient.
  • Updated the rel_shift() method to avoid flipping which needs significant memory.
  • Fixed the onnx issue.
  • Added att_context_size feature to limit the context of attention with masking.

@VahidooX
added intial text classification model doc.
de8d0a3 
Signed-off-by: Vahid <vnoroozi@nvidia.com>
@VahidooX
Fixed the onnx export.
97e182b 
Signed-off-by: Vahid <vnoroozi@nvidia.com>
@VahidooX
fixed style.
694b243 
Signed-off-by: Vahid <vnoroozi@nvidia.com>
@VahidooX
Added conv_norm_type to convolutional module.
b2d6fdf 
Signed-off-by: Vahid <vnoroozi@nvidia.com>
@VahidooX VahidooX mentioned this pull request Mar 12, 2021
Closed
@VahidooX VahidooX marked this pull request as ready for review March 17, 2021 22:14
@VahidooX
Copy link
Collaborator Author

VahidooX commented Mar 17, 2021 •
edited
Loading

@borisfom or @drnikolaev would you please checkout the asr_model.py?
I needed to make some changes to the export part. We did not pass the length to the encoder and it was fine for convolutional modules which do not use that input. But Conformer would use that input and export would not work on this model without passing lengths.

titu1994
titu1994 requested changes Mar 17, 2021
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.

Overall looks good, little bit improved docs is required.

nemo/collections/asr/modules/conformer_encoder.py Outdated Show resolved Hide resolved
nemo/collections/asr/modules/conformer_encoder.py Show resolved Hide resolved
nemo/collections/asr/parts/conformer_modules.py Outdated Show resolved Hide resolved
nemo/collections/asr/parts/conformer_modules.py Outdated Show resolved Hide resolved
nemo/collections/asr/parts/conformer_modules.py Outdated Show resolved Hide resolved
nemo/collections/asr/parts/conformer_modules.py Outdated Show resolved Hide resolved
nemo/collections/asr/parts/conformer_modules.py Outdated Show resolved Hide resolved
nemo/collections/asr/parts/multi_head_attention.py Outdated Show resolved Hide resolved
nemo/collections/asr/parts/multi_head_attention.py Show resolved Hide resolved
@VahidooX
dropped conv_type
4cc7180 
Signed-off-by: Vahid <vnoroozi@nvidia.com>
@VahidooX
Added more comments.
9bc406b 
Signed-off-by: Vahid <vnoroozi@nvidia.com>
@VahidooX
Added more comments.
1575743 
Signed-off-by: Vahid <vnoroozi@nvidia.com>
@VahidooX
Added more comments.
03ba202 
Signed-off-by: Vahid <vnoroozi@nvidia.com>
@VahidooX
fixed style.
6db7d90 
Signed-off-by: Vahid <vnoroozi@nvidia.com>
@VahidooX
updated config.
c8ab549 
Signed-off-by: Vahid <vnoroozi@nvidia.com>
@VahidooX
updated config.
5187c27 
Signed-off-by: Vahid <vnoroozi@nvidia.com>
@VahidooX VahidooX changed the title refactor the positional encoding of conformer. Updates to conformer. Mar 18, 2021
@VahidooX
Merge branch 'r1.0.0rc1' of https://github.com/NVIDIA/NeMo into fix_c…
0d87f1f 
…onformer_main
@VahidooX
updated config.
a5a48af 
Signed-off-by: Vahid <vnoroozi@nvidia.com>
@VahidooX
updated config.
a97dbb8 
Signed-off-by: Vahid <vnoroozi@nvidia.com>
titu1994
titu1994 approved these changes Mar 18, 2021
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. If @borisfom gives the go ahead, merge.

nemo/collections/asr/modules/conformer_encoder.py Show resolved Hide resolved
nemo/collections/asr/parts/multi_head_attention.py Show resolved Hide resolved
@VahidooX
added att_context_size.
7ceee9f 
Signed-off-by: Vahid <vnoroozi@nvidia.com>
@VahidooX
added att_context_size.
c0ae7a2 
Signed-off-by: Vahid <vnoroozi@nvidia.com>
@VahidooX
Merge branch 'r1.0.0rc1' of https://github.com/NVIDIA/NeMo into fix_c…
f8a525e 
…onformer_main
titu1994
titu1994 requested changes Mar 18, 2021
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.

Minor patch, python has issues with lists inside functions

nemo/collections/asr/modules/conformer_encoder.py Outdated Show resolved Hide resolved
@VahidooX
set default of att_context_size = None.
727caaf 
Signed-off-by: Vahid <vnoroozi@nvidia.com>
@VahidooX VahidooX requested a review from titu1994 March 18, 2021 23:35
titu1994
titu1994 approved these changes Mar 18, 2021
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 VahidooX merged commit 39b3646 into NVIDIA:r1.0.0rc1 Mar 18, 2021
vadam5 pushed a commit to vadam5/NeMo that referenced this pull request Apr 9, 2021
@VahidooX @vadam5
Updates to conformer. (NVIDIA#1898)
fa9a7d9 
Signed-off-by: Virginia Adams <vadams@nvidia.com>
vadam5 pushed a commit to vadam5/NeMo that referenced this pull request Apr 12, 2021
@VahidooX @vadam5
Updates to conformer. (NVIDIA#1898)
95be62c 
Signed-off-by: Virginia Adams <vadams@nvidia.com>
michalivne pushed a commit to michalivne/NeMo that referenced this pull request Jun 23, 2021
@VahidooX
Updates to conformer. (NVIDIA#1898)
5c95e68 
Signed-off-by: Micha Livne <mlivne@nvidia.com>
mousebaiker pushed a commit to mousebaiker/NeMo that referenced this pull request Jul 8, 2021
@VahidooX @mousebaiker
Updates to conformer. (NVIDIA#1898)
5e9f29a
@VahidooX VahidooX deleted the fix_conformer_main branch January 12, 2022 01:38
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

@borisfom borisfom borisfom approved these changes

@drnikolaev drnikolaev Awaiting requested review from drnikolaev

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

Successfully merging this pull request may close these issues.
3 participants
@VahidooX @titu1994 @borisfom