Skip to content

Commit 42b6e40

Browse files
diegoferigo-raiisaaclab-bot[bot]
authored andcommitted
Label RSL-RL play videos with checkpoint stems (#7392)
# Description When playing an RSL-RL checkpoint with video recording, the clip filenames do not say which checkpoint produced them, so videos from different checkpoints in the same run directory are indistinguishable. This appends the checkpoint stem to the play video filename prefix. The label is only applied to real RSL-RL checkpoints, matched as `model_<N>.pt`, so custom or pretrained checkpoint names that merely contain a digit are not mislabeled. ## Type of change - Bug fix (non-breaking change which fixes an issue) ## Release backport - [x] <!-- backport-active-release --> Backport this pull request to the active release branch after it merges into `develop` ## Checklist - [x] I have read and understood the contribution guidelines - [x] I have run the `pre-commit` checks with `./isaaclab.sh --format` - [x] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [x] I have added a changelog fragment under `source/<pkg>/changelog.d/` for every touched package - [x] I have added my name to the `CONTRIBUTORS.md` or my name already exists there (cherry picked from commit 8fbdc38)
1 parent 1d072ba commit 42b6e40

4 files changed

Lines changed: 71 additions & 2 deletions

File tree

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
Fixed
2+
^^^^^
3+
4+
* Fixed RSL-RL play video filenames missing the numeric checkpoint stem used for playback.

source/isaaclab_rl/isaaclab_rl/entrypoints/backends/play_rsl_rl.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,7 @@ def main(env_cfg: ManagerBasedRLEnvCfg | DirectRLEnvCfg | DirectMARLEnvCfg, agen
164164
log_dir = os.path.dirname(resume_path)
165165

166166
env_cfg.log_dir = log_dir
167-
apply_video_recording(env_cfg, log_dir, args_cli, subdir="play")
167+
apply_video_recording(env_cfg, log_dir, args_cli, subdir="play", checkpoint_path=resume_path)
168168

169169
screen.stage("Creating environment")
170170
env = create_isaaclab_env(

source/isaaclab_rl/isaaclab_rl/entrypoints/common.py

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -791,7 +791,14 @@ def pre_launch_video_config(env_cfg: Any, log_dir: str | None = None, args_cli:
791791
pass
792792

793793

794-
def apply_video_recording(env_cfg: Any, log_dir: str, args_cli: argparse.Namespace, *, subdir: str = "train") -> None:
794+
def apply_video_recording(
795+
env_cfg: Any,
796+
log_dir: str,
797+
args_cli: argparse.Namespace,
798+
*,
799+
subdir: str = "train",
800+
checkpoint_path: str | None = None,
801+
) -> None:
795802
"""Configure internal video recording on the environment config.
796803
797804
Enables recording by ensuring ``env_cfg.video_recorders`` is non-empty, then applies
@@ -817,6 +824,8 @@ def apply_video_recording(env_cfg: Any, log_dir: str, args_cli: argparse.Namespa
817824
args_cli: Parsed command-line arguments.
818825
subdir: Sub-directory name appended to ``<log_dir>/videos/`` for the fallback output
819826
path. Use ``"train"`` for training runs and ``"play"`` for evaluation.
827+
checkpoint_path: Checkpoint loaded by a play run. When set to a ``model_<N>.pt``
828+
path with a numeric id, the checkpoint stem is appended to play video names.
820829
"""
821830
if not getattr(args_cli, "video", False):
822831
return
@@ -958,6 +967,8 @@ def apply_video_recording(env_cfg: Any, log_dir: str, args_cli: argparse.Namespa
958967
cfg.video_length = video_length
959968
if video_interval is not None:
960969
cfg.video_interval = video_interval
970+
if subdir == "play" and (label := _checkpoint_video_label(checkpoint_path)) is not None:
971+
cfg.output_filename_prefix = _checkpoint_video_prefix(cfg.output_filename_prefix, label)
961972

962973
print("[INFO] Video recording enabled.")
963974
for cfg in env_cfg.video_recorders:
@@ -972,6 +983,22 @@ def apply_video_recording(env_cfg: Any, log_dir: str, args_cli: argparse.Namespa
972983
)
973984

974985

986+
def _checkpoint_video_label(checkpoint_path: str | None) -> str | None:
987+
if checkpoint_path is None:
988+
return None
989+
990+
path = Path(checkpoint_path)
991+
if re.fullmatch(r"model_\d+", path.stem) is None or path.suffix != ".pt":
992+
return None
993+
return path.stem
994+
995+
996+
def _checkpoint_video_prefix(prefix: str, label: str) -> str:
997+
if prefix == label or prefix.endswith(f"_{label}"):
998+
return prefix
999+
return f"{prefix}_{label}"
1000+
1001+
9751002
def wrap_record_video(env, log_dir: str, args_cli: argparse.Namespace):
9761003
"""No-op stub kept for backwards compatibility.
9771004

source/isaaclab_rl/test/test_apply_video_recording.py

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,44 @@ def test_apply_video_recording_injects_correct_recorder():
7474
assert rec.video_length == 42 # CLI override applied
7575
assert rec.video_interval == 500 # CLI override applied
7676
assert rec.output_dir == os.path.join("/my/log", "videos", "play")
77+
assert rec.output_filename_prefix == "clip"
78+
79+
80+
@pytest.mark.parametrize(
81+
("existing_prefix", "checkpoint_name", "expected_prefix"),
82+
[
83+
("clip", "model_1200.pt", "clip_model_1200"),
84+
("eval", "model_42.pt", "eval_model_42"),
85+
# Overlapping numeric ids stay distinct tokens instead of substrings.
86+
("clip_model_1200", "model_120.pt", "clip_model_1200_model_120"),
87+
# Non-model or non-numeric stems keep the configured prefix.
88+
("clip", "custom_1200.pt", "clip"),
89+
("clip", "final.pt", "clip"),
90+
],
91+
)
92+
def test_apply_video_recording_labels_play_video_with_checkpoint_stem(
93+
existing_prefix, checkpoint_name, expected_prefix
94+
):
95+
"""Play videos append only a numeric model checkpoint stem, kept as a distinct token."""
96+
from isaaclab.envs.utils.video_recorder_cfg import VideoRecorderCfg
97+
98+
existing = VideoRecorderCfg()
99+
existing.output_filename_prefix = existing_prefix
100+
101+
env_cfg = _env_cfg()
102+
env_cfg.video_recorders = [existing]
103+
apply_video_recording(env_cfg, "/my/log", _args(), subdir="play", checkpoint_path=f"/my/log/{checkpoint_name}")
104+
105+
assert env_cfg.video_recorders[0].output_filename_prefix == expected_prefix
106+
107+
108+
def test_apply_video_recording_leaves_train_video_prefix_unchanged():
109+
"""Checkpoint labels are only applied to play videos, not training videos."""
110+
111+
env_cfg = _env_cfg()
112+
apply_video_recording(env_cfg, "/my/log", _args(), checkpoint_path="/my/log/model_1200.pt")
113+
114+
assert env_cfg.video_recorders[0].output_filename_prefix == "clip"
77115

78116

79117
def test_apply_video_recording_uses_cfg_defaults_when_cli_not_passed():

0 commit comments

Comments
 (0)