Fix regression on default value for find_unused_parameters (#14095)

Author: awaelchli

src/pytorch_lightning/CHANGELOG.md

@@ -70,6 +70,9 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
 - Fixed dtype inference during gradient norm computation ([#14051](https://github.com/Lightning-AI/lightning/pull/14051))
 
 
+- Fixed a bug that caused `ddp_find_unused_parameters` to be set `False`, whereas the intended default is `True` ([#14095](https://github.com/Lightning-AI/lightning/pull/14095))
+
+
 ## [1.7.0] - 2022-08-02
 
 ### Added

src/pytorch_lightning/strategies/ddp_spawn.py

@@ -315,10 +315,20 @@ def post_training_step(self) -> None:
     def register_strategies(cls, strategy_registry: Dict) -> None:
         entries = (
             ("ddp_spawn", "spawn"),
-            ("ddp_spawn_find_unused_parameters_false", "spawn"),
             ("ddp_fork", "fork"),
-            ("ddp_fork_find_unused_parameters_false", "fork"),
             ("ddp_notebook", "fork"),
+        )
+        for name, start_method in entries:
+            strategy_registry.register(
+                name,
+                cls,
+                description=f"DDP strategy with `start_method` '{start_method}'",
+                start_method=start_method,
+            )
+
+        entries = (
+            ("ddp_spawn_find_unused_parameters_false", "spawn"),
+            ("ddp_fork_find_unused_parameters_false", "fork"),
             ("ddp_notebook_find_unused_parameters_false", "fork"),
         )
         for name, start_method in entries:

tests/tests_pytorch/strategies/test_ddp.py

@@ -194,3 +194,15 @@ def root_device(self):
                 assert strategy._get_process_group_backend() == expected_process_group_backend
     else:
         assert strategy._get_process_group_backend() == expected_process_group_backend
+
+
+@pytest.mark.parametrize(
+    "strategy_name,expected_ddp_kwargs",
+    [
+        ("ddp", {}),
+        ("ddp_find_unused_parameters_false", {"find_unused_parameters": False}),
+    ],
+)
+def test_ddp_kwargs_from_registry(strategy_name, expected_ddp_kwargs):
+    trainer = Trainer(strategy=strategy_name)
+    assert trainer.strategy._ddp_kwargs == expected_ddp_kwargs

tests/tests_pytorch/strategies/test_ddp_spawn_strategy.py

@@ -178,3 +178,19 @@ def test_ddp_spawn_strategy_set_timeout(mock_init_process_group):
     mock_init_process_group.assert_called_with(
         process_group_backend, rank=global_rank, world_size=world_size, timeout=test_timedelta
     )
+
+
+@pytest.mark.parametrize(
+    "strategy_name,expected_ddp_kwargs",
+    [
+        ("ddp_spawn", {}),
+        ("ddp_fork", {}),
+        ("ddp_notebook", {}),
+        ("ddp_spawn_find_unused_parameters_false", {"find_unused_parameters": False}),
+        ("ddp_fork_find_unused_parameters_false", {"find_unused_parameters": False}),
+        ("ddp_notebook_find_unused_parameters_false", {"find_unused_parameters": False}),
+    ],
+)
+def test_ddp_kwargs_from_registry(strategy_name, expected_ddp_kwargs):
+    trainer = Trainer(strategy=strategy_name)
+    assert trainer.strategy._ddp_kwargs == expected_ddp_kwargs

tests/tests_pytorch/strategies/test_sharded_strategy.py

@@ -300,3 +300,17 @@ def test_block_backward_sync():
         with strategy.block_backward_sync():
             pass
     model.no_sync.assert_called_once()
+
+
+@pytest.mark.parametrize(
+    "strategy_name,expected_ddp_kwargs",
+    [
+        ("ddp_sharded", {}),
+        ("ddp_sharded_find_unused_parameters_false", {"find_unused_parameters": False}),
+        ("ddp_sharded_spawn", {}),
+        ("ddp_sharded_spawn_find_unused_parameters_false", {"find_unused_parameters": False}),
+    ],
+)
+def test_ddp_kwargs_from_registry(strategy_name, expected_ddp_kwargs):
+    trainer = Trainer(strategy=strategy_name)
+    assert trainer.strategy._ddp_kwargs == expected_ddp_kwargs