Skip to content

Add Initial support for ContextNet Encoder and CTC Decoder#630

Merged
titu1994 merged 19 commits intoNVIDIA-NeMo:masterfrom
titu1994:se_context_support
May 16, 2020
Merged

Add Initial support for ContextNet Encoder and CTC Decoder#630
titu1994 merged 19 commits intoNVIDIA-NeMo:masterfrom
titu1994:se_context_support

Conversation

@titu1994
Copy link
Copy Markdown
Collaborator

@titu1994 titu1994 commented May 13, 2020 •
edited
Loading

Changelog

Added

  • Add ContextNetEncoder, ContextNetDecoderForCTC neural modules to ASR collection
  • Add stride_last flag which allows stride and repeat flags to be used simultaneously. It will perform the strided convolution at the final Conv-BN-ReLU sub-block.
  • Add swish as optional activation function
  • Add zero_infinity flag to CTCLoss, default to False.
  • Adds integration test for ContextNetEncoder and ContextNetDecoderForCTC

Modified

  • Update Squeeze and Excitation sub-module to support different context sizes, support different activation
    • Change default se_reduction_ratio to 8 instead of 16.
  • SpecAugment now supports either an integer or floating point value for time_width.
    • If float is passed, adaptively uses it as percentage of current timesteps that should be cut.

Note: Currently, examples/asr/contextnet.py uses JasperDecoderForCTC instead of ContextNetDecoderForCTC. This will be updated in a future PR once full support is present.

titu1994 added 13 commits May 13, 2020 12:14
@titu1994
Add SE + context SE support
5f17859 
Signed-off-by: smajumdar <titu1994@gmail.com>
@titu1994
Add contextnet components
aa28939 
Signed-off-by: smajumdar <titu1994@gmail.com>
@titu1994
Add ContextNet support
86e3dbd 
Signed-off-by: smajumdar <titu1994@gmail.com>
@titu1994
Add config files
8a5c6de 
Signed-off-by: smajumdar <titu1994@gmail.com>
@titu1994
Correct configs
39c1a91 
Signed-off-by: smajumdar <titu1994@gmail.com>
@titu1994
Add streaming speech command
28a4cb1 
Signed-off-by: smajumdar <titu1994@gmail.com>
@titu1994
Add kernel size factor argument
646dd8f 
Signed-off-by: smajumdar <titu1994@gmail.com>
@titu1994
Add docstrings
d990c5c 
Signed-off-by: smajumdar <titu1994@gmail.com>
@titu1994
Update CHANGELOG.md
e14d5d5 
Signed-off-by: smajumdar <titu1994@gmail.com>
@titu1994
Add integration tests
d526488 
Signed-off-by: smajumdar <titu1994@gmail.com>
@titu1994
Style fixes and add docstrings for se_reduction_ratio
851350e 
Signed-off-by: smajumdar <titu1994@gmail.com>
@titu1994
Style fixes in tests
2e208f6 
Signed-off-by: smajumdar <titu1994@gmail.com>
@titu1994
Correct CHANGELOG.md
46cc5c7 
Signed-off-by: smajumdar <titu1994@gmail.com>
@okuchaiev okuchaiev requested review from blisc and okuchaiev May 13, 2020 21:19
okuchaiev
okuchaiev requested changes May 13, 2020
View reviewed changes
Copy link
Copy Markdown
Collaborator

@okuchaiev okuchaiev left a comment

Choose a reason for hiding this comment

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

few small comments

Comment thread nemo/collections/asr/contextnet.py Outdated
logging = nemo.logging


class ContextNetEncoder(TrainableNM):
Copy link
Copy Markdown
Collaborator

@okuchaiev okuchaiev May 13, 2020

Choose a reason for hiding this comment

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

Should this inherit from JasperEncoder ?

Copy link
Copy Markdown
Collaborator Author

@titu1994 titu1994 May 13, 2020

Choose a reason for hiding this comment

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

Good point

Copy link
Copy Markdown
Collaborator Author

@titu1994 titu1994 May 13, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

On second thought, it probably should not inherit JasperEncoder. While yes currently they share exactly same functionality, in the future they will not. In that case, the __init__ call will instantiate multiple JasperBlocks before ContextNetEncoder starts to instantiate its own values.

While there is duplication for now, it is cleaner to separate the two modules

Copy link
Copy Markdown
Collaborator Author

@titu1994 titu1994 May 15, 2020

Choose a reason for hiding this comment

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

Fixed

Comment thread nemo/collections/asr/parts/jasper.py
Comment thread nemo/collections/asr/contextnet.py
Comment thread nemo/collections/asr/losses.py
Comment thread nemo/collections/asr/parts/jasper.py Outdated
titu1994 added 3 commits May 13, 2020 15:35
@titu1994
Correctios to docstrings
a8d7f4c 
Signed-off-by: smajumdar <titu1994@gmail.com>
@titu1994
Add WandB support to contextnet.py
6d3e4ca 
Signed-off-by: smajumdar <titu1994@gmail.com>
@titu1994
Style fixes
8c81303 
Signed-off-by: smajumdar <titu1994@gmail.com>
@lgtm-com
Copy link
Copy Markdown

lgtm-com Bot commented May 14, 2020

This pull request introduces 1 alert when merging 8c81303 into a22d325 - view on LGTM.com

new alerts:

  • 1 for Unused import

@titu1994
Remove unused import
66924b6 
Signed-off-by: smajumdar <titu1994@gmail.com>
blisc
blisc previously approved these changes May 14, 2020
View reviewed changes
Comment thread examples/asr/contextnet.py

# (ContextNet uses the Jasper baseline encoder and decoder)
encoder = nemo_asr.ContextNetEncoder(
feat_in=contextnet_params["AudioToMelSpectrogramPreprocessor"]["features"],
Copy link
Copy Markdown
Collaborator

@blisc blisc May 14, 2020

Choose a reason for hiding this comment

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

Just a note that you can add this inside the yaml itself.
See https://confluence.atlassian.com/bitbucket/yaml-anchors-960154027.html

Copy link
Copy Markdown
Collaborator Author

@titu1994 titu1994 May 14, 2020

Choose a reason for hiding this comment

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

Thanks for the hint !

@titu1994
Refactor ContextNetEncoder to subclass JasperEncoder
81330ba 
Signed-off-by: smajumdar <titu1994@gmail.com>
@lgtm-com
Copy link
Copy Markdown

lgtm-com Bot commented May 14, 2020

This pull request introduces 1 alert when merging 81330ba into a22d325 - view on LGTM.com

new alerts:

  • 1 for Unused import

@titu1994
Remove unused imports
7ea9183 
Signed-off-by: smajumdar <titu1994@gmail.com>
@titu1994 titu1994 merged commit 99ef493 into NVIDIA-NeMo:master May 16, 2020
@titu1994 titu1994 deleted the se_context_support branch May 16, 2020 05:16
ZeroCool940711 pushed a commit to ZeroCool940711/NeMo that referenced this pull request May 16, 2020
@titu1994 @ZeroCool940711
Add Initial support for ContextNet Encoder and CTC Decoder (NVIDIA-Ne…
b2d677f 
…Mo#630)

* Add SE + context SE support

Signed-off-by: smajumdar <titu1994@gmail.com>

* Add contextnet components

Signed-off-by: smajumdar <titu1994@gmail.com>

* Add ContextNet support

Signed-off-by: smajumdar <titu1994@gmail.com>

* Add config files

Signed-off-by: smajumdar <titu1994@gmail.com>

* Correct configs

Signed-off-by: smajumdar <titu1994@gmail.com>

* Add streaming speech command

Signed-off-by: smajumdar <titu1994@gmail.com>

* Add kernel size factor argument

Signed-off-by: smajumdar <titu1994@gmail.com>

* Add docstrings

Signed-off-by: smajumdar <titu1994@gmail.com>

* Update CHANGELOG.md

Signed-off-by: smajumdar <titu1994@gmail.com>

* Add integration tests

Signed-off-by: smajumdar <titu1994@gmail.com>

* Style fixes and add docstrings for se_reduction_ratio

Signed-off-by: smajumdar <titu1994@gmail.com>

* Style fixes in tests

Signed-off-by: smajumdar <titu1994@gmail.com>

* Correct CHANGELOG.md

Signed-off-by: smajumdar <titu1994@gmail.com>

* Correctios to docstrings

Signed-off-by: smajumdar <titu1994@gmail.com>

* Add WandB support to contextnet.py

Signed-off-by: smajumdar <titu1994@gmail.com>

* Style fixes

Signed-off-by: smajumdar <titu1994@gmail.com>

* Remove unused import

Signed-off-by: smajumdar <titu1994@gmail.com>

* Refactor ContextNetEncoder to subclass JasperEncoder

Signed-off-by: smajumdar <titu1994@gmail.com>

* Remove unused imports

Signed-off-by: smajumdar <titu1994@gmail.com>
Signed-off-by: ZeroCool <alejandrogilelias940711@gmail.com>
dcurran90 pushed a commit to dcurran90/NeMo that referenced this pull request Oct 15, 2024
@derekhiggins
De-duplicate prompt templates (NVIDIA-NeMo#630)
416f47e 
Use a single jinja template for the prompts with and
without a document. Also remove the conditionals
checking for te presence of a document.

Fixes NVIDIA-NeMo#629

Signed-off-by: Derek Higgins <derekh@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@okuchaiev okuchaiev okuchaiev approved these changes

@blisc blisc blisc left review comments

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

@titu1994 @blisc @okuchaiev