nlp refactoring #316

VahidooX · 2020-01-31T19:20:44Z

New PR for #295 that does not touch all files.
Changes to the nemo nlp collection:
++updated the organization of the files, functions, and classes
++**names of all the nlp related files are updated to more appropriate names
++**data_layers.py is split into separate files for each data layer
++**all utils and functions related to datasets are moved into datasets folder
++**all tokenizers are organized into a folder under datasets
++added copyright headers to all files in the collection
++replaced all prints with logging
++fixed and adopted new style
++updated all the files and fixed the imports to avoid polluting the namespace with import *
++merged with nemo refactoring updates

Signed-off-by: Yang Zhang <[email protected]>

Signed-off-by: VahidooX <[email protected]>

lgtm-com · 2020-01-31T19:30:38Z

This pull request introduces 14 alerts and fixes 29 when merging fdc421b into 1fddca2 - view on LGTM.com

new alerts:

7 for 'import *' may pollute namespace
4 for Unused import
2 for Unused local variable
1 for Module is imported with 'import' and 'import from'

fixed alerts:

14 for Unused import
5 for 'import *' may pollute namespace
5 for Module is imported with 'import' and 'import from'
3 for Unused local variable
2 for Implicit string concatenation in a list

Signed-off-by: VahidooX <[email protected]>

lgtm-com · 2020-01-31T21:07:55Z

This pull request introduces 14 alerts and fixes 29 when merging 4f81260 into 1fddca2 - view on LGTM.com

new alerts:

7 for 'import *' may pollute namespace
4 for Unused import
2 for Unused local variable
1 for Module is imported with 'import' and 'import from'

fixed alerts:

14 for Unused import
5 for 'import *' may pollute namespace
5 for Module is imported with 'import' and 'import from'
3 for Unused local variable
2 for Implicit string concatenation in a list

Signed-off-by: VahidooX <[email protected]>

lgtm-com · 2020-01-31T21:23:03Z

This pull request introduces 11 alerts and fixes 29 when merging 0864e65 into 1fddca2 - view on LGTM.com

new alerts:

7 for 'import *' may pollute namespace
2 for Unused local variable
1 for Unused import
1 for Module is imported with 'import' and 'import from'

fixed alerts:

14 for Unused import
5 for 'import *' may pollute namespace
5 for Module is imported with 'import' and 'import from'
3 for Unused local variable
2 for Implicit string concatenation in a list

Signed-off-by: VahidooX <[email protected]>

lgtm-com · 2020-01-31T21:37:47Z

This pull request introduces 9 alerts and fixes 29 when merging 44bd381 into 1fddca2 - view on LGTM.com

new alerts:

7 for 'import *' may pollute namespace
2 for Unused local variable

fixed alerts:

14 for Unused import
5 for 'import *' may pollute namespace
5 for Module is imported with 'import' and 'import from'
3 for Unused local variable
2 for Implicit string concatenation in a list

Signed-off-by: VahidooX <[email protected]>

lgtm-com · 2020-01-31T22:07:06Z

This pull request introduces 10 alerts and fixes 29 when merging b3c1513 into 1fddca2 - view on LGTM.com

new alerts:

8 for 'import *' may pollute namespace
2 for Unused local variable

fixed alerts:

14 for Unused import
5 for 'import *' may pollute namespace
5 for Module is imported with 'import' and 'import from'
3 for Unused local variable
2 for Implicit string concatenation in a list

Signed-off-by: VahidooX <[email protected]>

Signed-off-by: VahidooX <[email protected]> # Conflicts: # scripts/export_bert_to_trt.py # tests/asr/test_asr.py # tests/asr/test_weight_share.py # tests/asr/test_zeroDS.py # tests/test_pytorch_trainers.py # tests/tts/test_tts.py

lgtm-com · 2020-01-31T22:54:00Z

This pull request introduces 5 alerts and fixes 32 when merging 2137704 into 4137c2b - view on LGTM.com

new alerts:

5 for 'import *' may pollute namespace

fixed alerts:

15 for Unused import
5 for Unused local variable
5 for 'import *' may pollute namespace
5 for Module is imported with 'import' and 'import from'
2 for Implicit string concatenation in a list

nemo/collections/nlp/__init__.py

nemo/collections/nlp/callbacks/qa_squad_callback.py

nemo/collections/nlp/callbacks/text_classification_callback.py

nemo/collections/nlp/data/datasets/punctuation_capitalization_dataset.py

nemo/collections/nlp/data/tokenizers/bert_tokenizer.py

nemo/collections/nlp/data/tokenizers/char_tokenizer.py

nemo/collections/nlp/nm/trainables/common/transformer/transformer_modules.py

nemo/collections/nlp/nm/trainables/common/transformer/transformer_nm.py

nemo/collections/nlp/utils/callback_utils.py

nemo/collections/nlp/utils/common_nlp_utils.py

nemo/collections/nlp/utils/loss_utils.py

blisc · 2020-01-31T23:03:30Z

It would be very helpful if you can add copyright headers on all new files

Signed-off-by: VahidooX <[email protected]>

lgtm-com · 2020-01-31T23:44:25Z

This pull request introduces 5 alerts and fixes 32 when merging b8f57bf into 4137c2b - view on LGTM.com

new alerts:

5 for 'import *' may pollute namespace

fixed alerts:

15 for Unused import
5 for Unused local variable
5 for 'import *' may pollute namespace
5 for Module is imported with 'import' and 'import from'
2 for Implicit string concatenation in a list

Signed-off-by: VahidooX <[email protected]>

lgtm-com · 2020-02-04T02:37:59Z

This pull request introduces 8 alerts and fixes 36 when merging 720f5c5 into b4aba85 - view on LGTM.com

new alerts:

5 for 'import *' may pollute namespace
3 for Signature mismatch in overriding method

fixed alerts:

15 for Unused import
6 for 'import *' may pollute namespace
5 for Unused local variable
5 for Module is imported with 'import' and 'import from'
3 for Signature mismatch in overriding method
2 for Implicit string concatenation in a list

examples/nlp/punctuation_capitalization.py

examples/nlp/question_answering_squad.py

tkornuta-nvidia · 2020-02-04T02:48:36Z

Standardize:

nemo_nlp.nm.trainables.huggingface.BERT vs nemo_nlp.nm.trainables.common.huggingface.BERT
nemo_nlp.nm.trainables.huggingface.BERT.list_pretrained_models() vs nemo_nlp.BERT.list_pretrained_models() vs nemo_nlp.huggingface.BERT.list_pretrained_models() vs (those are in the comments of many files)

okuchaiev · 2020-02-04T06:16:36Z

Should be: "nemo_nlp.nm.trainables.huggingface.BERT" not "nemo_nlp.nm.trainables.common.huggingface.BERT".
list_pretrained_models() is a method all Neural Modules have https://github.com/NVIDIA/NeMo/blob/master/nemo/core/neural_modules.py#L330 so there is nothing to standardize in @tkornuta-nvidia point (2) above

ekmb · 2020-02-04T16:17:52Z

please don't merge yet, some of the examples are broken

okuchaiev · 2020-02-04T17:26:02Z

@ekmb you can request changes to block merging

Signed-off-by: Evelina Bakhturina <[email protected]>

lgtm-com · 2020-02-04T18:52:15Z

This pull request introduces 24 alerts and fixes 36 when merging 04a083b into b4aba85 - view on LGTM.com

new alerts:

7 for Wrong number of arguments in a call
7 for Wrong number of arguments in a class instantiation
5 for 'import *' may pollute namespace
3 for Signature mismatch in overriding method
2 for Module is imported with 'import' and 'import from'

fixed alerts:

15 for Unused import
6 for 'import *' may pollute namespace
5 for Unused local variable
5 for Module is imported with 'import' and 'import from'
3 for Signature mismatch in overriding method
2 for Implicit string concatenation in a list

Signed-off-by: Evelina Bakhturina <[email protected]>

Signed-off-by: Yang Zhang <[email protected]>

…into nlp_refactoring_tmp

Signed-off-by: Evelina Bakhturina <[email protected]>

Signed-off-by: VahidooX <[email protected]>

…into nlp_refactoring_tmp

Signed-off-by: Evelina Bakhturina <[email protected]>

…into nlp_refactoring_tmp

lgtm-com · 2020-02-04T19:26:28Z

This pull request introduces 10 alerts and fixes 36 when merging 21f353b into b4aba85 - view on LGTM.com

new alerts:

5 for 'import *' may pollute namespace
3 for Signature mismatch in overriding method
2 for Module is imported with 'import' and 'import from'

fixed alerts:

15 for Unused import
6 for 'import *' may pollute namespace
5 for Unused local variable
5 for Module is imported with 'import' and 'import from'
3 for Signature mismatch in overriding method
2 for Implicit string concatenation in a list

Signed-off-by: Yang Zhang <[email protected]>

…into nlp_refactoring_tmp

Signed-off-by: VahidooX <[email protected]>

lgtm-com · 2020-02-04T19:48:56Z

This pull request introduces 10 alerts and fixes 36 when merging 4cbcfd4 into b4aba85 - view on LGTM.com

new alerts:

5 for 'import *' may pollute namespace
3 for Signature mismatch in overriding method
2 for Module is imported with 'import' and 'import from'

fixed alerts:

15 for Unused import
6 for 'import *' may pollute namespace
5 for Unused local variable
5 for Module is imported with 'import' and 'import from'
3 for Signature mismatch in overriding method
2 for Implicit string concatenation in a list

* Fix formatting and linting Signed-off-by: Martin Hickey <[email protected]> * Update after review Signed-off-by: Martin Hickey <[email protected]> * Update after review Signed-off-by: Martin Hickey <[email protected]> --------- Signed-off-by: Martin Hickey <[email protected]>

yzhang123 and others added 7 commits January 28, 2020 16:33

init commit of nlp refactoring

dfdacaa

Signed-off-by: Yang Zhang <[email protected]>

fixed import errors

889a2eb

Signed-off-by: Yang Zhang <[email protected]>

make absolute imports

a14da52

Signed-off-by: Yang Zhang <[email protected]>

fix import error

ffcd7fb

Signed-off-by: Yang Zhang <[email protected]>

fix imports

52d8a70

Signed-off-by: Yang Zhang <[email protected]>

rebase master

ff7774b

Signed-off-by: Yang Zhang <[email protected]>

add all changed nlp files

fdc421b

Signed-off-by: VahidooX <[email protected]>

Updated thw whole test folder.

4f81260

Signed-off-by: VahidooX <[email protected]>

VahidooX added 2 commits January 31, 2020 13:09

Changed nemo.logging to logging

0864e65

Signed-off-by: VahidooX <[email protected]>

Added transformer to the init

6df0778

Signed-off-by: VahidooX <[email protected]>

Fixed lgtm warnings.

44bd381

Signed-off-by: VahidooX <[email protected]>

Fixed transformer package.

b3c1513

Signed-off-by: VahidooX <[email protected]>

VahidooX added 3 commits January 31, 2020 14:11

Fixed unused local variables.

8c58796

Signed-off-by: VahidooX <[email protected]>

Fixed lgtm.

c5127a1

Signed-off-by: VahidooX <[email protected]>

Fixed lgtm.

ae83cff

Signed-off-by: VahidooX <[email protected]>

VahidooX changed the title ~~nlp refactoring~~ [WIP] nlp refactoring Jan 31, 2020

VahidooX added 2 commits January 31, 2020 14:39

Fixed logging in examples.

b851473

Signed-off-by: VahidooX <[email protected]>

blisc reviewed Jan 31, 2020

View reviewed changes

nemo/collections/nlp/__init__.py Outdated Show resolved Hide resolved

blisc requested changes Jan 31, 2020

View reviewed changes

Moved __all__ after imports. Added more __all__:)

b8f57bf

Signed-off-by: VahidooX <[email protected]>

Added license to all the files except examples.

58715fe

Signed-off-by: VahidooX <[email protected]>

tkornuta-nvidia reviewed Feb 4, 2020

View reviewed changes

examples/nlp/punctuation_capitalization.py Outdated Show resolved Hide resolved

tkornuta-nvidia reviewed Feb 4, 2020

View reviewed changes

examples/nlp/question_answering_squad.py Show resolved Hide resolved

blisc previously approved these changes Feb 4, 2020

View reviewed changes

okuchaiev requested a review from ekmb February 4, 2020 17:25

import fixes, nm default shuffleFalse

04a083b

Signed-off-by: Evelina Bakhturina <[email protected]>

ekmb dismissed blisc’s stale review via 04a083b February 4, 2020 18:41

ekmb and others added 8 commits February 4, 2020 10:58

shuffle fixed

e8e9770

Signed-off-by: Evelina Bakhturina <[email protected]>

fix shuffle

f555326

Signed-off-by: Yang Zhang <[email protected]>

Merge branch 'nlp_refactoring_tmp' of https://github.com/NVIDIA/NeMo …

17dbaf6

…into nlp_refactoring_tmp

default shFalse in text_dl

1852d59

Signed-off-by: Evelina Bakhturina <[email protected]>

Shuffle bug fixed.

42b4ecc

Signed-off-by: VahidooX <[email protected]>

Merge branch 'nlp_refactoring_tmp' of https://github.com/NVIDIA/NeMo …

46087d1

…into nlp_refactoring_tmp

nmt fix

5b4a75e

Signed-off-by: Evelina Bakhturina <[email protected]>

Merge branch 'nlp_refactoring_tmp' of https://github.com/NVIDIA/NeMo …

21f353b

…into nlp_refactoring_tmp

yzhang123 and others added 3 commits February 4, 2020 11:29

fix squad unittest

e7853bf

Signed-off-by: Yang Zhang <[email protected]>

Merge branch 'nlp_refactoring_tmp' of https://github.com/NVIDIA/NeMo …

cc674c9

…into nlp_refactoring_tmp

Set shuffles to False in some data layers.

4cbcfd4

Signed-off-by: VahidooX <[email protected]>

ekmb changed the title ~~[WIP] nlp refactoring~~ nlp refactoring Feb 4, 2020

ekmb approved these changes Feb 4, 2020

View reviewed changes

VahidooX merged commit 42a5f20 into master Feb 4, 2020

VahidooX deleted the nlp_refactoring_tmp branch February 4, 2020 23:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

nlp refactoring #316

nlp refactoring #316

VahidooX commented Jan 31, 2020 •

edited

Loading

lgtm-com bot commented Jan 31, 2020

lgtm-com bot commented Jan 31, 2020

lgtm-com bot commented Jan 31, 2020

lgtm-com bot commented Jan 31, 2020

lgtm-com bot commented Jan 31, 2020

lgtm-com bot commented Jan 31, 2020

blisc commented Jan 31, 2020

lgtm-com bot commented Jan 31, 2020

lgtm-com bot commented Feb 4, 2020

tkornuta-nvidia commented Feb 4, 2020

okuchaiev commented Feb 4, 2020

ekmb commented Feb 4, 2020

okuchaiev commented Feb 4, 2020

lgtm-com bot commented Feb 4, 2020

lgtm-com bot commented Feb 4, 2020

lgtm-com bot commented Feb 4, 2020

nlp refactoring #316

nlp refactoring #316

Conversation

VahidooX commented Jan 31, 2020 • edited Loading

lgtm-com bot commented Jan 31, 2020

lgtm-com bot commented Jan 31, 2020

lgtm-com bot commented Jan 31, 2020

lgtm-com bot commented Jan 31, 2020

lgtm-com bot commented Jan 31, 2020

lgtm-com bot commented Jan 31, 2020

blisc commented Jan 31, 2020

lgtm-com bot commented Jan 31, 2020

lgtm-com bot commented Feb 4, 2020

tkornuta-nvidia commented Feb 4, 2020

okuchaiev commented Feb 4, 2020

ekmb commented Feb 4, 2020

okuchaiev commented Feb 4, 2020

lgtm-com bot commented Feb 4, 2020

lgtm-com bot commented Feb 4, 2020

lgtm-com bot commented Feb 4, 2020

VahidooX commented Jan 31, 2020 •

edited

Loading