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.

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

Add Module-level Adapters, Save-Restore and tests #4114

Merged
merged 34 commits into from
May 17, 2022

Conversation

titu1994
Copy link
Collaborator

@titu1994 titu1994 commented May 5, 2022

What does this PR do ?

Adds significant functionality to adapters, allowing multiple module-specific adapters, save and restore of just the adapters themselves rather than the full model (saving several hundred megabytes-to-gigabytes), and a suite of tests for the new functionality.

Collection: [Core, ASR]

Changelog

  • Adds support for both "global" and "module" specific adapters. Module specific adapters allow for targetted adapters to certain modules, independent of the others.
  • Adds support for saving and restoring just the adapter modules, independent of the original model weights. This allows savings of significant disk storage, since just the adapters are usually a tiny fraction of the param count of the models.
  • Adds a battery of tests for the new capabilities.
  • Add support for dropout to adapter modules
  • Add support for stochastic dropout to all adapters via ResidualAddStrategy.

Usage

Subclass implementations can chose between global adapters, module adapters or both. A basic implementation of Adapters for a toy model is provided in the tests/core/mixins/adapters/test_adapter_model_mixin.py

# Global adapters (adds "abc" adapter to all supported modules - encoder+decoder+others1)
model.add_adapter(name="abc", cfg=cfg) 

# Module adapters 
model.add_adapter(name="decoder:abc", cfg=cfg)

# Save all of the adapters (only the adapters themselves)
model.save_adapters("adapters.pt", name=None)
OR
model.save_adapters("adapters.pt", name={global or module adapter name})

# Restore one-or-all adapters (only the adapters themselves)
new_model.load_adapters("adapters.pt", name=None)
OR
new_model.load_adapters("adapters.pt", name={exact module name from state dict}, 
                        map_location='cpu', strict=True)

Before your PR is "Ready for review"

Pre checks:

  • Make sure you read and followed Contributor guidelines
  • Did you write any new necessary tests?
  • Did you add or update any necessary documentation?
  • Does the PR affect components that are optional to install? (Ex: Numba, Pynini, Apex etc)
    • Reviewer: Does the PR have correct import guards for all optional libraries?

PR Type:

  • New Feature
  • Bugfix
  • Documentation

@titu1994 titu1994 marked this pull request as draft May 5, 2022 04:40
@titu1994 titu1994 marked this pull request as ready for review May 6, 2022 01:09
@lgtm-com
Copy link

lgtm-com bot commented May 6, 2022

This pull request introduces 1 alert when merging cf85cf3 into ddd8719 - view on LGTM.com

new alerts:

  • 1 for Unused local variable

@lgtm-com
Copy link

lgtm-com bot commented May 8, 2022

This pull request introduces 1 alert when merging 5462c37 into ddd8719 - view on LGTM.com

new alerts:

  • 1 for Unused local variable

@lgtm-com
Copy link

lgtm-com bot commented May 8, 2022

This pull request introduces 1 alert when merging 644247d into ddd8719 - view on LGTM.com

new alerts:

  • 1 for Unused local variable

@lgtm-com
Copy link

lgtm-com bot commented May 8, 2022

This pull request introduces 1 alert when merging 976166e into ddd8719 - view on LGTM.com

new alerts:

  • 1 for Unused local variable

@lgtm-com
Copy link

lgtm-com bot commented May 9, 2022

This pull request introduces 1 alert when merging aa19bf6 into ddd8719 - view on LGTM.com

new alerts:

  • 1 for Unused local variable

@lgtm-com
Copy link

lgtm-com bot commented May 9, 2022

This pull request introduces 1 alert when merging 3496141 into ddd8719 - view on LGTM.com

new alerts:

  • 1 for Unused local variable

@lgtm-com
Copy link

lgtm-com bot commented May 9, 2022

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

new alerts:

  • 1 for Unused local variable

@lgtm-com
Copy link

lgtm-com bot commented May 9, 2022

This pull request introduces 1 alert when merging f6ab1e5 into ddd8719 - view on LGTM.com

new alerts:

  • 1 for Unused local variable

@lgtm-com
Copy link

lgtm-com bot commented May 9, 2022

This pull request introduces 1 alert when merging e0bc4e1 into 470587a - view on LGTM.com

new alerts:

  • 1 for Unused local variable

@lgtm-com
Copy link

lgtm-com bot commented May 10, 2022

This pull request introduces 1 alert when merging 7ea9643 into 650718f - view on LGTM.com

new alerts:

  • 1 for Unused local variable

@ericharper
Copy link
Collaborator

/blossom-ci

1 similar comment
@ericharper
Copy link
Collaborator

/blossom-ci

@titu1994
Copy link
Collaborator Author

/blossom-ci

@lgtm-com
Copy link

lgtm-com bot commented May 13, 2022

This pull request introduces 1 alert when merging 0904b9e into 1311f4f - view on LGTM.com

new alerts:

  • 1 for Unused local variable

@lgtm-com
Copy link

lgtm-com bot commented May 13, 2022

This pull request introduces 1 alert when merging 777cf7e into 1311f4f - view on LGTM.com

new alerts:

  • 1 for Unused local variable

@lgtm-com
Copy link

lgtm-com bot commented May 13, 2022

This pull request introduces 1 alert when merging bcca9cf into 08df199 - view on LGTM.com

new alerts:

  • 1 for Unused local variable

@lgtm-com
Copy link

lgtm-com bot commented May 14, 2022

This pull request introduces 1 alert when merging 7b63804 into 27129ab - view on LGTM.com

new alerts:

  • 1 for Unused local variable

@lgtm-com
Copy link

lgtm-com bot commented May 14, 2022

This pull request introduces 1 alert when merging eaa973c into 27129ab - view on LGTM.com

new alerts:

  • 1 for Unused local variable

@lgtm-com
Copy link

lgtm-com bot commented May 16, 2022

This pull request introduces 1 alert when merging 7169d13 into ed1ffeb - view on LGTM.com

new alerts:

  • 1 for Unused local variable

@@ -120,7 +121,7 @@ def main(cfg):
raise ValueError("Cannot set `cfg.model.nemo_model` and `cfg.model.pretrained_model`. Select one only.")
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd write "Cannot set both cfg.model.nemo_model and cfg.model.pretrained_model. Set one only."

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

Signed-off-by: smajumdar <titu1994@gmail.com>
Signed-off-by: smajumdar <titu1994@gmail.com>
Signed-off-by: smajumdar <titu1994@gmail.com>
Signed-off-by: smajumdar <titu1994@gmail.com>
Signed-off-by: smajumdar <titu1994@gmail.com>
Signed-off-by: smajumdar <titu1994@gmail.com>
Signed-off-by: smajumdar <smajumdar@nvidia.com>
Signed-off-by: smajumdar <smajumdar@nvidia.com>
Signed-off-by: smajumdar <smajumdar@nvidia.com>
Signed-off-by: smajumdar <smajumdar@nvidia.com>
Signed-off-by: smajumdar <smajumdar@nvidia.com>
Signed-off-by: smajumdar <smajumdar@nvidia.com>
Signed-off-by: smajumdar <smajumdar@nvidia.com>
Signed-off-by: smajumdar <smajumdar@nvidia.com>
Signed-off-by: smajumdar <smajumdar@nvidia.com>
@titu1994 titu1994 merged commit 89994de into NVIDIA:main May 17, 2022
@titu1994 titu1994 deleted the multi_adapter branch May 17, 2022 18:59
@lgtm-com
Copy link

lgtm-com bot commented May 17, 2022

This pull request introduces 1 alert when merging a624f32 into 8318980 - view on LGTM.com

new alerts:

  • 1 for Unused local variable

yaoyu-33 pushed a commit that referenced this pull request May 25, 2022
* First draft of model level tests and support for multiple types adapters in same model

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

* Add save restore tests for adapters

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

* Add save restore tests for adapters

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

* Add adapter only save and restore

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

* Update base adapter config

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

* Add tests

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

* Fix collection of get enabled adapters, limiting to each module's scope

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

* Update docs and add support for resolution of module adapter names

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

* Update ASR adapters to only support module adapters

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

* Add state dict match test

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

* Fix name resolution for set_enabled_adapters

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

* Correct case where name is none for set adapter

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

* Correct case where there are no adapters to save

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

* Update config for training

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

* Force update to internal config upon get or set

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

* Add spec augment update support to adapters

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

* Correct config update

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

* Add dropout support to linear adapters

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

* Add type to config

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

* Add stochastic depth regularization to adapter merge strategy and related tests

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

* Add support for dynamic strategy change

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

* Add support for dynamic strategy change

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

* Add more tests

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

* Add more tests

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

* Remove logging of adapter name

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

* Update changes for reviews

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

* Refactor the utility methods

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

* Refactor the utility methods

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

* Fixed configs for optim and spec augment

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

* Fixed configs for optim and spec augment

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

* Rename method to subclassable private

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

* Add support for adapter module names to be pre-specified in config

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

* Fix imports

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

* Fix typos

Signed-off-by: smajumdar <smajumdar@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants