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

[feat] Add optimizer state sharding (ZeRO) #636

Closed
wants to merge 1 commit into from

Conversation

vedanuj
Copy link
Contributor

@vedanuj vedanuj commented Oct 19, 2020

Summary:
Adding Zero optimizer state sharding to MMF from fairscale library.
Added to tutorials

It can used with this option : optimizer.enable_state_sharding=True

Differential Revision: D24317858

@facebook-github-bot facebook-github-bot added CLA Signed Do not delete this pull request or issue due to inactivity. fb-exported labels Oct 19, 2020
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D24317858

vedanuj added a commit to vedanuj/mmf that referenced this pull request Oct 19, 2020
Summary:
Pull Request resolved: facebookresearch#636

Adding Zero optimizer state sharding to MMF from [fairscale](https://github.com/facebookresearch/fairscale) library.
Added to tutorials

It can used with this option : `optimizer.enable_state_sharding=True`

Differential Revision: D24317858

fbshipit-source-id: bc6cd687f0e580716b71b7a4daf143dc5097cc3e
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D24317858

vedanuj added a commit to vedanuj/mmf that referenced this pull request Oct 20, 2020
Summary:
Pull Request resolved: facebookresearch#636

Adding Zero optimizer state sharding to MMF from [fairscale](https://github.com/facebookresearch/fairscale) library.
Added to tutorials

It can used with this option : `optimizer.enable_state_sharding=True`

Differential Revision: D24317858

fbshipit-source-id: 471ce803f716b81dc4928d8cba281cceb75ab8d4
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D24317858

vedanuj added a commit to vedanuj/mmf that referenced this pull request Oct 23, 2020
Summary:
Pull Request resolved: facebookresearch#636

Adding Zero optimizer state sharding to MMF from [fairscale](https://github.com/facebookresearch/fairscale) library.
Added to tutorials

It can used with this option : `optimizer.enable_state_sharding=True`

Differential Revision: D24317858

fbshipit-source-id: 4d3e09697df4f4f3b7c84e337d3287595e1b421f
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D24317858

1 similar comment
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D24317858

vedanuj added a commit to vedanuj/mmf that referenced this pull request Oct 23, 2020
Summary:
Pull Request resolved: facebookresearch#636

Adding Zero optimizer state sharding to MMF from [fairscale](https://github.com/facebookresearch/fairscale) library.
Added to tutorials

It can used with this option : `optimizer.enable_state_sharding=True`

Differential Revision: D24317858

fbshipit-source-id: e1c25d81c231f7ab18fbe61e374a539d988e4556
vedanuj added a commit to vedanuj/mmf that referenced this pull request Oct 23, 2020
Summary:
Pull Request resolved: facebookresearch#636

Adding Zero optimizer state sharding to MMF from [fairscale](https://github.com/facebookresearch/fairscale) library.
Added to tutorials

It can used with this option : `optimizer.enable_state_sharding=True`

Differential Revision: D24317858

fbshipit-source-id: aa2b5868a9cddddaf09cb045ea2c5be6bec86b14
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D24317858

Copy link
Contributor

@apsdehal apsdehal left a comment

Choose a reason for hiding this comment

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

This is awesome. Some comments. Do you plan to add any tests for these?

@@ -22,6 +22,7 @@ install_dep: &install_dep
command: |
source activate mmf
pip install --upgrade setuptools
pip install --progress-bar off -r requirements.txt
Copy link
Contributor

Choose a reason for hiding this comment

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

I am somewhat not a fan of this as it creates confusion. setup.py should in itself be enough to install any dependencies.

optimizer = optimizer_class(parameters, **params)

if optimizer_config.get("enable_state_sharding", False):
from fairscale.optim.oss import OSS
Copy link
Contributor

Choose a reason for hiding this comment

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

We can also do a try, except ImportError and ask user to install the fairscale library separately if we don't want explicit dependency on it. This would also solve some things on fbcode side, user who want it should include fairscale in the targets. Adding more dependency always causes more pain.

vedanuj added a commit to vedanuj/mmf that referenced this pull request Oct 24, 2020
Summary:
Pull Request resolved: facebookresearch#636

Adding Zero optimizer state sharding to MMF from [fairscale](https://github.com/facebookresearch/fairscale) library.
Added to tutorials

It can used with this option : `optimizer.enable_state_sharding=True`

Differential Revision: D24317858

fbshipit-source-id: 90161c7b3314b631e85e59e50ca1701ed8daec79
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D24317858

Summary:
Pull Request resolved: facebookresearch#636

Adding Zero optimizer state sharding to MMF from [fairscale](https://github.com/facebookresearch/fairscale) library.
Added to tutorials

It can used with this option : `optimizer.enable_state_sharding=True`

Differential Revision: D24317858

fbshipit-source-id: 6c16e0614bb568c9ac35a70760af560d4a8c4d60
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D24317858

@@ -226,7 +226,24 @@ def build_optimizer(model, config):
)

parameters = get_optimizer_parameters(model, config)
optimizer = optimizer_class(parameters, **params)

if optimizer_config.get("enable_state_sharding", False):
Copy link
Contributor

Choose a reason for hiding this comment

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

It would make sense to write a simple test to test this. We have had issue with build_optimizer in past.

Copy link
Contributor

@apsdehal apsdehal left a comment

Choose a reason for hiding this comment

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

This looks good. Thanks for pushing this. A minor comment about test and should be good to land after that.

@facebook-github-bot
Copy link
Contributor

This pull request has been merged in 3ae71ec.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Do not delete this pull request or issue due to inactivity. fb-exported Merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants