[Tune] Log the choice of RunConfig only when passed into both trainer + tuner (#33454)

Signed-off-by: Justin Yu <justinvyu@berkeley.edu>

Author: justinvyu

python/ray/train/tests/test_tune.py

@@ -23,7 +23,7 @@
 from ray.tune.impl.tuner_internal import _TUNER_PKL
 
 
-@pytest.fixture
+@pytest.fixture(scope="module")
 def ray_start_4_cpus():
     address_info = ray.init(num_cpus=4)
     yield address_info
@@ -272,22 +272,41 @@ def train_func(config):
     assert not results.errors
 
 
+@pytest.mark.parametrize("in_trainer", [True, False])
+@pytest.mark.parametrize("in_tuner", [True, False])
 def test_run_config_in_trainer_and_tuner(
-    ray_start_4_cpus, tmp_path, propagate_logs, caplog
+    propagate_logs, tmp_path, caplog, in_trainer, in_tuner
 ):
+    trainer_run_config = (
+        RunConfig(name="trainer", local_dir=str(tmp_path)) if in_trainer else None
+    )
+    tuner_run_config = (
+        RunConfig(name="tuner", local_dir=str(tmp_path)) if in_tuner else None
+    )
     trainer = DataParallelTrainer(
         lambda config: None,
         backend_config=TestConfig(),
         scaling_config=ScalingConfig(num_workers=1),
-        run_config=RunConfig(name="ignored", local_dir="ignored"),
+        run_config=trainer_run_config,
     )
     with caplog.at_level(logging.INFO, logger="ray.tune.impl.tuner_internal"):
-        Tuner(trainer, run_config=RunConfig(name="used", local_dir=str(tmp_path)))
-    assert list((tmp_path / "used").glob(_TUNER_PKL))
-    assert (
+        tuner = Tuner(trainer, run_config=tuner_run_config)
+
+    both_msg = (
         "`RunConfig` was passed to both the `Tuner` and the `DataParallelTrainer`"
-        in caplog.text
     )
+    if in_trainer and in_tuner:
+        assert list((tmp_path / "tuner").glob(_TUNER_PKL))
+        assert both_msg in caplog.text
+    elif in_trainer and not in_tuner:
+        assert list((tmp_path / "trainer").glob(_TUNER_PKL))
+        assert both_msg not in caplog.text
+    elif not in_trainer and in_tuner:
+        assert list((tmp_path / "tuner").glob(_TUNER_PKL))
+        assert both_msg not in caplog.text
+    else:
+        assert tuner._local_tuner.get_run_config() == RunConfig()
+        assert both_msg not in caplog.text
 
 
 def test_run_config_in_param_space():

python/ray/tune/impl/tuner_internal.py

@@ -83,22 +83,11 @@ def __init__(
         from ray.train.trainer import BaseTrainer
 
         if isinstance(trainable, BaseTrainer):
-            # If no run config was passed to the Tuner directly,
-            # use the one from the Trainer, if available
-            if not run_config:
-                run_config = trainable.run_config
-            if run_config and trainable.run_config != RunConfig():
-                logger.info(
-                    "A `RunConfig` was passed to both the `Tuner` and the "
-                    f"`{trainable.__class__.__name__}`. The run config passed to "
-                    "the `Tuner` is the one that will be used."
-                )
-            if param_space and "run_config" in param_space:
-                raise ValueError(
-                    "`RunConfig` cannot be tuned as part of the `param_space`! "
-                    "Move the run config to be a parameter of the `Tuner`: "
-                    "Tuner(..., run_config=RunConfig(...))"
-                )
+            run_config = self._choose_run_config(
+                tuner_run_config=run_config,
+                trainer=trainable,
+                param_space=param_space,
+            )
 
         self.trainable = trainable
         param_space = param_space or {}
@@ -403,6 +392,49 @@ def _maybe_sync_down_tuner_state(self, restore_path: str) -> Tuple[bool, str]:
         download_from_uri(str(restore_uri / _TUNER_PKL), str(tempdir / _TUNER_PKL))
         return True, str(tempdir)
 
+    def _choose_run_config(
+        self,
+        tuner_run_config: Optional[RunConfig],
+        trainer: "BaseTrainer",
+        param_space: Optional[Dict[str, Any]],
+    ) -> RunConfig:
+        """Chooses which `RunConfig` to use when multiple can be passed in
+        through a Trainer or the Tuner itself.
+
+        Args:
+            tuner_run_config: The run config passed into the Tuner constructor.
+            trainer: The AIR Trainer instance to use with Tune, which may have
+                a RunConfig specified by the user.
+            param_space: The param space passed to the Tuner.
+
+        Raises:
+            ValueError: if the `run_config` is specified as a hyperparameter.
+        """
+        if param_space and "run_config" in param_space:
+            raise ValueError(
+                "`RunConfig` cannot be tuned as part of the `param_space`! "
+                "Move the run config to be a parameter of the `Tuner`: "
+                "Tuner(..., run_config=RunConfig(...))"
+            )
+
+        # Both Tuner RunConfig + Trainer RunConfig --> prefer Tuner RunConfig
+        if tuner_run_config and trainer.run_config != RunConfig():
+            logger.info(
+                "A `RunConfig` was passed to both the `Tuner` and the "
+                f"`{trainer.__class__.__name__}`. The run config passed to "
+                "the `Tuner` is the one that will be used."
+            )
+            return tuner_run_config
+
+        # No Tuner RunConfig -> pass the Trainer config through
+        # This returns either a user-specified config, or the default RunConfig
+        # if nothing was provided to both the Trainer or Tuner.
+        if not tuner_run_config:
+            return trainer.run_config
+
+        # Tuner RunConfig + No Trainer RunConfig --> Use the Tuner config
+        return tuner_run_config
+
     def _process_scaling_config(self) -> None:
         """Converts ``self._param_space["scaling_config"]`` to a dict.