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.

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

set find_unused_parameters=False in DDP as in pytorch #5435

Merged
merged 9 commits into from
Jan 13, 2021
Merged

set find_unused_parameters=False in DDP as in pytorch #5435

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
d6c3702
set find unused params to False
awaelchli Jan 9, 2021
e2b7e22
add changelog
awaelchli Jan 9, 2021
420d8c9
fix changelog
awaelchli Jan 9, 2021
8190671
fix test
awaelchli Jan 9, 2021
1b46fee
update docs
awaelchli Jan 9, 2021
472cdb0
update changelog
awaelchli Jan 9, 2021
3ea0ba3
Merge branch 'release/1.2-dev' into feature/find_unused
tchaton Jan 11, 2021
465a15f
Merge branch 'release/1.2-dev' into feature/find_unused
Borda Jan 11, 2021
510cbaa
Merge branch 'release/1.2-dev' into feature/find_unused
awaelchli Jan 13, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,9 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
- Changed `iou` [func] to allow float input ([#4704](https://github.com/PyTorchLightning/pytorch-lightning/pull/4704))


- Changed the default of `find_unused_parameters` to `False` in DDP ([#5435](https://github.com/PyTorchLightning/pytorch-lightning/pull/5435))


- Changed `callbacks` argument in `Trainer` to allow `Callback` input ([#5446](https://github.com/PyTorchLightning/pytorch-lightning/pull/5446))


Expand Down
6 changes: 1 addition & 5 deletions pytorch_lightning/plugins/ddp_plugin.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ def configure_ddp(

def configure_ddp(self, model, device_ids):
model = LightningDistributedDataParallel(
model, device_ids=device_ids, find_unused_parameters=True
model, device_ids=device_ids, **self._ddp_kwargs
)
return model

Expand All @@ -72,10 +72,6 @@ def configure_ddp(self, model, device_ids):
the model wrapped in LightningDistributedDataParallel

"""
# if unset, default `find_unused_parameters` `True`
self._ddp_kwargs["find_unused_parameters"] = self._ddp_kwargs.get(
"find_unused_parameters", True
)
model = LightningDistributedDataParallel(
model,
device_ids=device_ids,
Expand Down
2 changes: 2 additions & 0 deletions tests/models/test_sync_batchnorm.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import torch.nn.functional as F

from pytorch_lightning import Trainer, seed_everything, LightningModule
from pytorch_lightning.plugins.ddp_plugin import DDPPlugin
from pytorch_lightning.utilities import FLOAT16_EPSILON
from tests.base.datamodules import MNISTDataModule
from tests.base.develop_utils import set_random_master_port
Expand Down Expand Up @@ -107,6 +108,7 @@ def test_sync_batchnorm_ddp(tmpdir):
sync_batchnorm=True,
num_sanity_val_steps=0,
replace_sampler_ddp=False,
plugins=[DDPPlugin(find_unused_parameters=True)]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

had to set this here to true, as mentioned by @SeanNaren in the comment above.
turns out this modification to the test is already in master branch

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah for a test this is fine, but for the user you can't enable this unless you pass your own plugin, which is why I don't like this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, it's also not agnostic to command line input / device without directly. A new trainer flag is probably best for user.

)

result = trainer.fit(model, dm)
Expand Down