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

Neural Modules Configuration stage 2: config import-export #339

Merged

tkornuta-nvidia merged 46 commits into master from dev-config-impoexpo

Feb 19, 2020

Contributor

tkornuta-nvidia commented Feb 7, 2020 •

edited

Loading

Functionality implemented in this PR:

Generic export of configuration (init_params) to configuration (yml) file
Generic import of configuration from configuration file
Unit tests for both import and export functionalities
An example script showing how to use import/export
A tutorial showing how to use import/export
A tutorial showing how to implement a custom import/export
Refactor of an existing ASR example to see whether the solution fits to our needs

The import distinguishes two cases: when importing from the root NeuralModule class and from the leaf neural module.

This PR have also "initializes" the module/config/checkpoint versioning. For now I am saving in config the only available information, i.e. version of the NeMo core plus collection name (collections do not have their own versioning, yet). I am also not performing any version checking during import. Generally I think this issue should be discussed in a separate "Problem brainstorming session".

Additionally, this PR cleans up:

utilization of get_module() from NeuralFactory (CV related examples, all relying on ResNet, are left for now - will clean them up along with the introduction of CV collection)

tkornuta-nvidia added 6 commits

February 6, 2020 13:33


          __validate becoming private, unit test fix

005feec

Signed-off-by: Tomasz Kornuta <tkornuta@nvidia.com>


          initial verion of export plus unittest

51bb755

Signed-off-by: Tomasz Kornuta <tkornuta@nvidia.com>


          Unallowed export unit test

e5a9c3c

Signed-off-by: Tomasz Kornuta <tkornuta@nvidia.com>


          Core of the import working, elementary unit test for import implemented

e92d221

Signed-off-by: Tomasz Kornuta <tkornuta@nvidia.com>


          import_config becoming a class method, enabling to check whether conf…

2a6ee86

…ig file is compatible during import

Signed-off-by: Tomasz Kornuta <tkornuta@nvidia.com>


          comments

5e692c5

Signed-off-by: Tomasz Kornuta <tkornuta@nvidia.com>

tkornuta-nvidia added the enhancement label

tkornuta-nvidia requested a review from blisc

February 7, 2020 03:42

tkornuta-nvidia added 3 commits

February 6, 2020 19:48


          yaml added to requirements

08104d1

Signed-off-by: Tomasz Kornuta <tkornuta@nvidia.com>


          print cleanup

d1692d6

Signed-off-by: Tomasz Kornuta <tkornuta@nvidia.com>


          PyYAML fix

d322c8b

Signed-off-by: Tomasz Kornuta <tkornuta@nvidia.com>

lgtm-com bot commented Feb 7, 2020

This pull request introduces 1 alert when merging d322c8b into 4f299f4 - view on LGTM.com

new alerts:

1 for Module is imported with 'import' and 'import from'


          LGTM fix: import nemo in NeuralModule.py

515ce04

Signed-off-by: Tomasz Kornuta <tkornuta@nvidia.com>

redoctopus reviewed

View reviewed changes

nemo/core/neural_modules.py Outdated Show resolved Hide resolved

blisc reviewed

View reviewed changes

nemo/core/neural_modules.py Outdated Show resolved Hide resolved

nemo/core/neural_modules.py Show resolved Hide resolved

nemo/core/neural_modules.py Show resolved Hide resolved

requirements/requirements.txt Outdated Show resolved Hide resolved

tkornuta-nvidia added 2 commits

February 7, 2020 10:49


          Cleanup of get_module() in examples and tutorials

ace54de

Signed-off-by: Tomasz Kornuta <tkornuta@nvidia.com>


          yaml -> ruamel yaml, import and requirements cleanups

4cf1e1e

Signed-off-by: Tomasz Kornuta <tkornuta@nvidia.com>

okuchaiev reviewed

View reviewed changes

nemo/core/neural_modules.py Show resolved Hide resolved

tkornuta-nvidia added 12 commits

February 10, 2020 13:32


          ruamel.yaml

215bb31

Signed-off-by: Tomasz Kornuta <tkornuta@nvidia.com>


          import_from_config, export_to_config refactor

e269115

Signed-off-by: Tomasz Kornuta <tkornuta@nvidia.com>


          check of header content in test

6c428a6

Signed-off-by: Tomasz Kornuta <tkornuta@nvidia.com>


          chatbot_example2 cleanup

0cc1a22

Signed-off-by: Tomasz Kornuta <tkornuta@nvidia.com>


          Simple example of config import/export

d39fc98

Signed-off-by: Tomasz Kornuta <tkornuta@nvidia.com>


          merge with master

c3da23b

Signed-off-by: Tomasz Kornuta <tkornuta@nvidia.com>


          Tutorial on configuration import and export

537872e

Signed-off-by: Tomasz Kornuta <tkornuta@nvidia.com>


          Polishing of the example/tutorial

44cbd73

Signed-off-by: Tomasz Kornuta <tkornuta@nvidia.com>


          merge with master

a175db6

Signed-off-by: Tomasz Kornuta <tkornuta@nvidia.com>


          unjenkinsing jasper_an4 example

2c18ed2

Signed-off-by: Tomasz Kornuta <tkornuta@nvidia.com>


          Merge branch 'dev-jasper-unjenkinsing' into dev-config-impoexpo

2c84d9b


          jenkins fix - passing proper paths as args

0b4ce22

Signed-off-by: Tomasz Kornuta <tkornuta@nvidia.com>

tkornuta-nvidia added 2 commits

February 14, 2020 17:54


          Merging config_file and config_dir into a single: config_file

5cbb5e5

Signed-off-by: Tomasz Kornuta <tkornuta@nvidia.com>


          merge with master - solved conflict on jenkins

bf1729e

Signed-off-by: Tomasz Kornuta <tkornuta@nvidia.com>

lgtm-com bot commented Feb 15, 2020

This pull request introduces 1 alert when merging bf1729e into 142bed9 - view on LGTM.com

new alerts:

1 for Unused import


          LGTM fix unused import jasper_an4

2f67801

Signed-off-by: Tomasz Kornuta <tkornuta@nvidia.com>

blisc reviewed

View reviewed changes

examples/asr/jasper_an4.py Show resolved Hide resolved


          batch_size fix in AudioDataLayer

5417f22

Signed-off-by: nvidia <tkornuta@nvidia.com>

okuchaiev previously approved these changes

View reviewed changes

Member

okuchaiev commented Feb 18, 2020

looks good to me, what's up with CI though?

tkornuta-nvidia added 3 commits

February 18, 2020 17:19


          Merge branch 'master' of github.com:NVIDIA/NeMo into dev-config-impoexpo

b6796b8


          Merge branch 'dev-config-impoexpo' of github.com:NVIDIA/NeMo into dev…

04e494f

…-config-impoexpo


          fix: batch_size as an optional command-line parameter

b1e8fa1

Signed-off-by: Tomasz Kornuta <tkornuta@nvidia.com>

tkornuta-nvidia dismissed okuchaiev’s stale review via

b1e8fa1

February 19, 2020 01:32


          custom import example

dfc204d

Signed-off-by: Tomasz Kornuta <tkornuta@nvidia.com>

lgtm-com bot commented Feb 19, 2020

This pull request introduces 6 alerts when merging dfc204d into abaac1b - view on LGTM.com

new alerts:

4 for Unused import
1 for Non-callable called
1 for Module is imported with 'import' and 'import from'


          LGTM warnings cleanup

1d87187

Signed-off-by: Tomasz Kornuta <tkornuta@nvidia.com>

lgtm-com bot commented Feb 19, 2020

This pull request introduces 2 alerts when merging 1d87187 into abaac1b - view on LGTM.com

new alerts:

1 for Non-callable called
1 for Module is imported with 'import' and 'import from'

tkornuta-nvidia marked this pull request as ready for review

February 19, 2020 03:52

tkornuta-nvidia added 3 commits

February 18, 2020 19:55


          reorganization of examples and tutorials related to module configuration

ce9bf03

Signed-off-by: Tomasz Kornuta <tkornuta@nvidia.com>


          LGTM warning - import nemo and from

a902981

Signed-off-by: Tomasz Kornuta <tkornuta@nvidia.com>


          Polished custom configuration: examples and tutorials

bedb48a

Signed-off-by: Tomasz Kornuta <tkornuta@nvidia.com>

lgtm-com bot commented Feb 19, 2020

This pull request introduces 2 alerts when merging bedb48a into abaac1b - view on LGTM.com

new alerts:

1 for Non-callable called
1 for Unused import


          final polishing of the module configuration tutorial

c87d2e2

Signed-off-by: Tomasz Kornuta <tkornuta@nvidia.com>

lgtm-com bot commented Feb 19, 2020

This pull request introduces 2 alerts when merging c87d2e2 into abaac1b - view on LGTM.com

new alerts:

1 for Non-callable called
1 for Unused import

tkornuta-nvidia requested review from okuchaiev and blisc

February 19, 2020 05:28

okuchaiev approved these changes

View reviewed changes

blisc reviewed

View reviewed changes

tests/test_neural_modules_export.py

		@@ -0,0 +1,141 @@
		# ! /usr/bin/python

Collaborator

blisc Feb 19, 2020

can you rename this file? you are not testing Neural Module export, you are testing configuration export

Contributor Author

tkornuta-nvidia Feb 19, 2020

actually you are right, was thinking about that when renamed examples/tutorial

blisc reviewed

View reviewed changes

tests/test_neural_modules_import.py

		@@ -0,0 +1,114 @@
		# ! /usr/bin/python

Collaborator

blisc Feb 19, 2020

Can you rename this file too? again configuration import

Contributor Author

tkornuta-nvidia Feb 19, 2020

ok, creating a separate PR

tkornuta-nvidia merged commit 1b1b821 into master

tkornuta-nvidia deleted the dev-config-impoexpo branch

February 19, 2020 19:41

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet