feat(profiler): require explicit local save_path; drop implicit ckpt-derived default

Per review: remove the non-obvious {ckpt_path}/profiler_traces default and the
silent cloud-URI -> ./profiler_traces fallback. The relative fallback would land
traces in the Ray runtime working dir under /tmp/ray, and the ckpt-derived
default was surprising. save_path is now a required, explicit local path,
validated fail-fast at startup (non-empty + not a cloud URI). Updated docs,
config defaults comment, and tests accordingly.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: dyurk-lila

docs/content/docs/configuration/config.mdx

@@ -276,7 +276,7 @@ policy:
   torch_profiler_config:    # torch.profiler-based training-loop profiler (see below)
     enable: false
     ranks: [0]
-    save_path: null         # defaults to {ckpt_path}/profiler_traces
+    save_path: null         # REQUIRED when enable=true; absolute local path
     skip_first: 10           # torch.profiler.schedule
     wait: 0
     warmup: 1
@@ -310,7 +310,7 @@ Which steps are recorded is controlled entirely by [`torch.profiler.schedule`](h
 
 - `policy.torch_profiler_config.enable`: Master switch. When `false` (default), no profiler RPCs are dispatched and there is zero overhead.
 - `policy.torch_profiler_config.ranks`: List of global ranks to profile (e.g. `[0]`). Add a mid-pipeline-stage rank to diagnose pipeline bubbles.
-- `policy.torch_profiler_config.save_path`: Output directory for traces. Defaults to `{ckpt_path}/profiler_traces`.
+- `policy.torch_profiler_config.save_path`: Output directory for traces. **Required** when `enable: true` (there is no implicit default). Use an absolute local path: Ray workers run from a `/tmp/ray/.../working_dir_files` runtime working dir, so a relative path would scatter traces there, and `torch.profiler` cannot write to cloud URIs (`s3://`/`gs://`) — both are rejected at startup.
 - `policy.torch_profiler_config.{skip_first,wait,warmup,active,repeat}`: Passed directly to `torch.profiler.schedule`.
 - `policy.torch_profiler_config.activities`: Subset of `["cpu", "cuda"]` to record.
 - `policy.torch_profiler_config.{record_shapes,profile_memory,with_stack,with_flops,with_modules}`: Passed directly to `torch.profiler.profile`. `with_stack` and `record_shapes` add overhead; `profile_memory` is heavier still and off by default.

skyrl/backends/skyrl_train/utils/profiler.py

@@ -4,8 +4,6 @@
 import torch.distributed
 from loguru import logger
 
-from skyrl.backends.skyrl_train.utils.io.io import is_cloud_path
-
 # Map config activity strings to torch ProfilerActivity members.
 _ACTIVITY_MAP = {
     "cpu": torch.profiler.ProfilerActivity.CPU,
@@ -17,15 +15,13 @@ def build_profiler_from_policy_cfg(trainer_cfg):
     """Construct a :class:`Profiler` from ``policy.torch_profiler_config``, or None.
 
     Returns ``None`` when profiling is disabled, so callers can simply assign the
-    result to ``self.profiler``. The trace ``save_path`` defaults to
-    ``{ckpt_path}/profiler_traces`` (mirrors how memory snapshots default under
-    ``ckpt_path``).
+    result to ``self.profiler``. ``save_path`` is an explicit, required local path
+    (validated at startup) -- there is no implicit ``ckpt_path``-derived default.
     """
     cfg = trainer_cfg.policy.torch_profiler_config
     if not cfg.enable:
         return None
-    default_save_path = os.path.join(trainer_cfg.ckpt_path, "profiler_traces")
-    return Profiler(cfg, default_save_path=default_save_path)
+    return Profiler(cfg)
 
 
 class Profiler:
@@ -51,7 +47,7 @@ class Profiler:
         with_modules, export_type.
     """
 
-    def __init__(self, config, default_save_path: str = None):
+    def __init__(self, config):
         self.enable = config.enable
         self.prof = None
         # Per-window self-device-time kernel summary, refreshed by on_trace_ready
@@ -62,17 +58,9 @@ def __init__(self, config, default_save_path: str = None):
         if not config.enable:
             return
         self.config = config
-        self.save_path = config.save_path or default_save_path or "./profiler_traces"
-        # torch.profiler writes traces with the local filesystem only. ``save_path``
-        # commonly defaults to ``{ckpt_path}/profiler_traces``, and ckpt_path can be a
-        # cloud URI (s3://, gs://) -- which torch can't write to, so the trace would be
-        # silently lost. Fall back to a local dir so profiling still produces output.
-        if is_cloud_path(self.save_path):
-            logger.warning(
-                f"[Profiler] cloud save_path {self.save_path!r} is not writable by torch.profiler; "
-                f"falling back to local './profiler_traces'."
-            )
-            self.save_path = "./profiler_traces"
+        # `save_path` is a required, explicit local path -- validated by
+        # TorchProfilerConfig.validate() at startup (non-empty + not a cloud URI).
+        self.save_path = config.save_path
         self.ranks = list(config.ranks)
         self.export_type = getattr(config, "export_type", "chrome_trace")
         self.rank = torch.distributed.get_rank() if torch.distributed.is_initialized() else 0

skyrl/train/config/config.py

@@ -180,7 +180,10 @@ class TorchProfilerConfig(BaseConfig):
     enable: bool = False
     ranks: List[int] = field(default_factory=lambda: [0])
     save_path: Optional[str] = None
-    """Trace output dir. Defaults to ``{ckpt_path}/profiler_traces`` when None."""
+    """Trace output dir. Required when ``enable=True``; use an absolute local path.
+    Ray workers run from a ``/tmp/ray/.../working_dir_files`` runtime dir, so a
+    relative path would scatter traces there -- and ``torch.profiler`` cannot write
+    to cloud URIs (``s3://``/``gs://``)."""
 
     # torch.profiler.schedule -- one cycle = wait + warmup + active steps.
     skip_first: int = 10
@@ -216,6 +219,22 @@ def validate(self) -> None:
             return
         if not self.ranks:
             raise ValueError("`torch_profiler_config.ranks` must be non-empty when profiling is enabled.")
+        # `save_path` is a required, explicit, local path -- no implicit default. Ray
+        # workers run from a /tmp/ray runtime working dir, so a relative path would
+        # scatter traces there, and torch.profiler can't write cloud URIs.
+        if not self.save_path:
+            raise ValueError(
+                "`torch_profiler_config.save_path` must be set when profiling is enabled. "
+                "Use an absolute local path -- Ray workers run from a /tmp/ray runtime "
+                "working dir, so a relative path would write traces there."
+            )
+        from skyrl.backends.skyrl_train.utils.io.io import is_cloud_path
+
+        if is_cloud_path(self.save_path):
+            raise ValueError(
+                f"`torch_profiler_config.save_path` must be a local path; got cloud URI "
+                f"{self.save_path!r}. torch.profiler cannot write to cloud storage."
+            )
         # An empty `activities` passes the membership check below vacuously, but
         # `torch.profiler.profile(activities=[])` records nothing -- fail fast instead.
         if not self.activities:

tests/backends/skyrl_train/utils/test_profiler.py

@@ -44,7 +44,7 @@ def _run_loop(prof: Profiler, n_steps: int) -> None:
 
 
 def test_disabled_is_noop(tmp_path):
-    prof = Profiler(_ProfCfg(enable=False), default_save_path=str(tmp_path))
+    prof = Profiler(_ProfCfg(enable=False, save_path=str(tmp_path)))
     assert prof.prof is None
     assert prof.check() is False
     _run_loop(prof, 5)  # must not raise
@@ -54,16 +54,15 @@ def test_disabled_is_noop(tmp_path):
 def test_rank_not_selected_is_noop(tmp_path):
     # is_initialized() is False in this process -> rank resolves to 0, which is
     # not in ranks=[1], so the profiler must not arm.
-    prof = Profiler(_ProfCfg(ranks=[1]), default_save_path=str(tmp_path))
+    prof = Profiler(_ProfCfg(ranks=[1], save_path=str(tmp_path)))
     assert prof.prof is None
     _run_loop(prof, 3)
     assert glob.glob(os.path.join(str(tmp_path), "*")) == []
 
 
 def test_single_window_writes_one_trace(tmp_path):
     prof = Profiler(
-        _ProfCfg(skip_first=0, wait=0, warmup=0, active=1, repeat=1),
-        default_save_path=str(tmp_path),
+        _ProfCfg(skip_first=0, wait=0, warmup=0, active=1, repeat=1, save_path=str(tmp_path)),
     )
     assert prof.check() is True
     _run_loop(prof, 3)
@@ -75,8 +74,7 @@ def test_single_window_writes_one_trace(tmp_path):
 def test_repeat_writes_multiple_windows(tmp_path):
     # Two cycles of (warmup=1 + active=1); needs >= 2*(1+1) steps to fire twice.
     prof = Profiler(
-        _ProfCfg(skip_first=0, wait=0, warmup=1, active=1, repeat=2),
-        default_save_path=str(tmp_path),
+        _ProfCfg(skip_first=0, wait=0, warmup=1, active=1, repeat=2, save_path=str(tmp_path)),
     )
     _run_loop(prof, 8)
     traces = glob.glob(os.path.join(str(tmp_path), "*.pt.trace.json*"))
@@ -86,51 +84,35 @@ def test_repeat_writes_multiple_windows(tmp_path):
 def test_skip_first_defers_recording(tmp_path):
     # With skip_first larger than the loop, no window should ever open.
     prof = Profiler(
-        _ProfCfg(skip_first=100, wait=0, warmup=0, active=1, repeat=1),
-        default_save_path=str(tmp_path),
+        _ProfCfg(skip_first=100, wait=0, warmup=0, active=1, repeat=1, save_path=str(tmp_path)),
     )
     _run_loop(prof, 5)
     assert glob.glob(os.path.join(str(tmp_path), "*.pt.trace.json*")) == []
 
 
-def test_save_path_overrides_default(tmp_path):
-    explicit = tmp_path / "explicit"
-    prof = Profiler(
-        _ProfCfg(save_path=str(explicit)),
-        default_save_path=str(tmp_path / "default"),
-    )
-    assert prof.save_path == str(explicit)
-
-
-def test_default_save_path_used_when_none(tmp_path):
-    default = str(tmp_path / "default")
-    prof = Profiler(_ProfCfg(save_path=None), default_save_path=default)
-    assert prof.save_path == default
-
-
-def test_cloud_save_path_falls_back_to_local(tmp_path):
-    # ckpt_path (and thus the derived default save_path) can be a cloud URI, which
-    # torch.profiler can't write to. The wrapper must fall back to a local dir so
-    # the trace isn't silently lost.
-    prof = Profiler(_ProfCfg(save_path="s3://my-bucket/run/profiler_traces"), default_save_path=str(tmp_path))
-    assert prof.save_path == "./profiler_traces"
+def test_save_path_is_taken_verbatim(tmp_path):
+    # `save_path` is an explicit, required local path -- the wrapper uses it as-is
+    # (no implicit default, no rewriting). Cloud-URI/empty rejection lives in
+    # TorchProfilerConfig.validate(), exercised in tests/train/test_config.py.
+    explicit = str(tmp_path / "explicit")
+    prof = Profiler(_ProfCfg(save_path=explicit))
+    assert prof.save_path == explicit
 
 
 def test_kernel_summary_none_when_disabled(tmp_path):
-    prof = Profiler(_ProfCfg(enable=False), default_save_path=str(tmp_path))
+    prof = Profiler(_ProfCfg(enable=False, save_path=str(tmp_path)))
     assert prof.get_kernel_summary() is None
 
 
 def test_kernel_summary_empty_before_first_window(tmp_path):
-    prof = Profiler(_ProfCfg(skip_first=0, wait=0, warmup=0, active=1), default_save_path=str(tmp_path))
+    prof = Profiler(_ProfCfg(skip_first=0, wait=0, warmup=0, active=1, save_path=str(tmp_path)))
     summary = prof.get_kernel_summary()
     assert summary == {"window_count": 0, "pairs": []}
 
 
 def test_kernel_summary_populated_after_window(tmp_path):
     prof = Profiler(
-        _ProfCfg(skip_first=0, wait=0, warmup=0, active=1, repeat=1, activities=["cpu"]),
-        default_save_path=str(tmp_path),
+        _ProfCfg(skip_first=0, wait=0, warmup=0, active=1, repeat=1, activities=["cpu"], save_path=str(tmp_path)),
     )
     prof.start()
     for _ in range(3):
@@ -157,14 +139,14 @@ def test_kernel_summary_populated_after_window(tmp_path):
 def test_activities_threaded_to_torch(tmp_path):
     import torch
 
-    prof = Profiler(_ProfCfg(activities=["cpu"]), default_save_path=str(tmp_path))
+    prof = Profiler(_ProfCfg(activities=["cpu"], save_path=str(tmp_path)))
     # The underlying torch profiler was constructed with the CPU activity only.
     assert torch.profiler.ProfilerActivity.CPU in prof.prof.activities
     assert torch.profiler.ProfilerActivity.CUDA not in prof.prof.activities
 
 
 def test_step_failure_disables_without_raising(tmp_path):
-    prof = Profiler(_ProfCfg(), default_save_path=str(tmp_path))
+    prof = Profiler(_ProfCfg(save_path=str(tmp_path)))
     prof.start()
 
     class _Boom:
@@ -241,42 +223,33 @@ def test_rpcs_drive_profiler_when_present(self):
 
 class TestBuildProfilerFromPolicyCfg:
     """``build_profiler_from_policy_cfg`` is the production entry both worker
-    backends call in ``init_model``. It gates on ``enable`` and composes the
-    default ``{ckpt_path}/profiler_traces`` save path when none is set."""
+    backends call in ``init_model``. It gates on ``enable`` and passes the
+    explicit, validated ``save_path`` straight through (no implicit default)."""
 
     @staticmethod
-    def _trainer_cfg(prof_cfg, ckpt_path):
+    def _trainer_cfg(prof_cfg):
         from types import SimpleNamespace
 
-        return SimpleNamespace(ckpt_path=ckpt_path, policy=SimpleNamespace(torch_profiler_config=prof_cfg))
+        return SimpleNamespace(policy=SimpleNamespace(torch_profiler_config=prof_cfg))
 
     def test_returns_none_when_disabled(self, tmp_path):
         from skyrl.backends.skyrl_train.utils.profiler import (
             build_profiler_from_policy_cfg,
         )
 
-        cfg = self._trainer_cfg(_ProfCfg(enable=False), str(tmp_path))
+        cfg = self._trainer_cfg(_ProfCfg(enable=False, save_path=str(tmp_path)))
         assert build_profiler_from_policy_cfg(cfg) is None
 
-    def test_builds_profiler_with_ckpt_default_save_path(self, tmp_path):
+    def test_builds_profiler_with_explicit_save_path(self, tmp_path):
         from skyrl.backends.skyrl_train.utils.profiler import (
             Profiler,
             build_profiler_from_policy_cfg,
         )
 
-        cfg = self._trainer_cfg(_ProfCfg(enable=True, save_path=None), str(tmp_path))
-        prof = build_profiler_from_policy_cfg(cfg)
-        assert isinstance(prof, Profiler)
-        assert prof.save_path == os.path.join(str(tmp_path), "profiler_traces")
-
-    def test_explicit_save_path_wins_over_ckpt_default(self, tmp_path):
-        from skyrl.backends.skyrl_train.utils.profiler import (
-            build_profiler_from_policy_cfg,
-        )
-
         explicit = str(tmp_path / "explicit")
-        cfg = self._trainer_cfg(_ProfCfg(enable=True, save_path=explicit), str(tmp_path))
+        cfg = self._trainer_cfg(_ProfCfg(enable=True, save_path=explicit))
         prof = build_profiler_from_policy_cfg(cfg)
+        assert isinstance(prof, Profiler)
         assert prof.save_path == explicit
 
 

tests/train/test_config.py

@@ -268,13 +268,18 @@ class TestTorchProfilerConfigValidation:
     def _cfg(**overrides):
         from skyrl.train.config.config import TorchProfilerConfig
 
+        # `save_path` is required when enabled; supply a valid local default so
+        # tests targeting *other* fields don't trip the save_path guard.
+        overrides.setdefault("save_path", "/tmp/skyrl_prof_test")
         return TorchProfilerConfig(enable=True, **overrides)
 
     def test_disabled_skips_all_checks(self):
         from skyrl.train.config.config import TorchProfilerConfig
 
         # Garbage values must be tolerated while disabled (the default state).
-        TorchProfilerConfig(enable=False, export_type="bogus", activities=["gpu"], ranks=[], active=0).validate()
+        TorchProfilerConfig(
+            enable=False, export_type="bogus", activities=["gpu"], ranks=[], active=0, save_path=None
+        ).validate()
 
     def test_defaults_are_valid_when_enabled(self):
         self._cfg().validate()  # must not raise
@@ -283,6 +288,19 @@ def test_empty_ranks_rejected(self):
         with pytest.raises(ValueError, match=r"ranks.*non-empty"):
             self._cfg(ranks=[]).validate()
 
+    def test_missing_save_path_rejected(self):
+        # save_path has no implicit default; an enabled run must set it explicitly.
+        with pytest.raises(ValueError, match=r"save_path.*must be set"):
+            self._cfg(save_path=None).validate()
+        with pytest.raises(ValueError, match=r"save_path.*must be set"):
+            self._cfg(save_path="").validate()
+
+    def test_cloud_save_path_rejected(self):
+        # torch.profiler can only write the local filesystem; cloud URIs fail fast.
+        for uri in ("s3://bucket/run/traces", "gs://bucket/run/traces", "gcs://bucket/run/traces"):
+            with pytest.raises(ValueError, match=r"save_path.*local path"):
+                self._cfg(save_path=uri).validate()
+
     def test_unknown_activity_rejected(self):
         with pytest.raises(ValueError, match=r"activities"):
             self._cfg(activities=["cpu", "gpu"]).validate()