diff --git a/scripts/benchmarks/measure_synthetic_throughput.py b/scripts/benchmarks/measure_synthetic_throughput.py new file mode 100644 index 0000000000..e97d44f79f --- /dev/null +++ b/scripts/benchmarks/measure_synthetic_throughput.py @@ -0,0 +1,151 @@ +"""Stress-test SceneEntityCfg ID indexing with a synthetic many-term env. + +Builds a cartpole env inflated with N extra observation and reward terms, each +using a single-joint ``SceneEntityCfg`` so ``joint_ids`` resolves to a real +``list[int]`` (not the optimized ``slice(None)``). This is a focused microbench +for the ``tensor[:, cfg.joint_ids]`` implicit-CUDA-sync question raised in +issue #1019: light physics keeps manager overhead as the dominant signal, and +every extra term hits the suspected hot path. + +Reuses the timing helpers from ``measure_throughput`` for apples-to-apples +comparison with the regression benchmark. +""" + +from __future__ import annotations + +import statistics +import sys +from dataclasses import dataclass, field +from pathlib import Path + +import torch +import tyro + +import mjlab +from mjlab.envs import ManagerBasedRlEnv +from mjlab.envs.mdp import joint_pos_rel, joint_vel_rel +from mjlab.managers.observation_manager import ObservationTermCfg +from mjlab.managers.reward_manager import RewardTermCfg +from mjlab.managers.scene_entity_config import SceneEntityCfg +from mjlab.tasks.cartpole.cartpole_env_cfg import ( + cartpole_balance_env_cfg, + pole_angle_cos_sin, +) + +sys.path.insert(0, str(Path(__file__).parent)) +from measure_throughput import measure_env_sps, measure_physics_sps # noqa: E402 + + +def _synthetic_reward( + env: ManagerBasedRlEnv, + asset_cfg: SceneEntityCfg, +) -> torch.Tensor: + """Indexing-heavy reward that mirrors the joint_pos_rel access pattern.""" + asset = env.scene[asset_cfg.name] + jp = asset.data.joint_pos[:, asset_cfg.joint_ids] + jv = asset.data.joint_vel[:, asset_cfg.joint_ids] + return -(jp.pow(2).sum(-1) + 0.1 * jv.pow(2).sum(-1)) + + +def _inflate(cfg, extra_terms: int) -> None: + """Add ``extra_terms`` obs + reward terms that all do list[int] indexing. + + Alternates between the slider and hinge joints; each picks exactly one joint + so ``joint_ids`` cannot collapse to ``slice(None)``. + """ + if extra_terms <= 0: + return + joint_names = ("slider", "hinge_1") + obs_funcs = (joint_pos_rel, joint_vel_rel, pole_angle_cos_sin) + for i in range(extra_terms): + joint = joint_names[i % 2] + entity_cfg = SceneEntityCfg("cartpole", joint_names=(joint,)) + obs_func = obs_funcs[i % len(obs_funcs)] + cfg.observations["actor"].terms[f"synth_obs_{i}"] = ObservationTermCfg( + func=obs_func, + params={"asset_cfg": entity_cfg}, + ) + cfg.observations["critic"].terms[f"synth_obs_{i}"] = ObservationTermCfg( + func=obs_func, + params={"asset_cfg": entity_cfg}, + ) + cfg.rewards[f"synth_reward_{i}"] = RewardTermCfg( + func=_synthetic_reward, + weight=0.0, + params={"asset_cfg": entity_cfg}, + ) + + +@dataclass +class SyntheticConfig: + num_envs: int = 4096 + num_steps: int = 200 + warmup_steps: int = 50 + reps: int = 5 + device: str = "cuda:0" + extra_terms: list[int] = field(default_factory=lambda: [0, 10, 50, 100]) + """Sweep of extra-term counts to benchmark.""" + + +def benchmark_one(cfg: SyntheticConfig, extra_terms: int) -> dict: + print(f"\nSynthetic cartpole with {extra_terms} extra terms...") + env_cfg = cartpole_balance_env_cfg() + env_cfg.scene.num_envs = cfg.num_envs + _inflate(env_cfg, extra_terms) + + env = ManagerBasedRlEnv(cfg=env_cfg, device=cfg.device) + env.reset() + + action_dim = sum(env.action_manager.action_term_dim) + action = torch.zeros(env.num_envs, action_dim, device=env.device) + for _ in range(cfg.warmup_steps): + env.step(action) + torch.cuda.synchronize() + + physics_samples: list[float] = [] + env_samples: list[float] = [] + for _ in range(cfg.reps): + physics_samples.append(measure_physics_sps(env, cfg.num_steps)) + env.reset() + torch.cuda.synchronize() + env_samples.append(measure_env_sps(env, cfg.num_steps)) + env.reset() + torch.cuda.synchronize() + + env.close() + + physics_med = statistics.median(physics_samples) + env_med = statistics.median(env_samples) + return { + "extra_terms": extra_terms, + "physics_sps": physics_med, + "env_sps": env_med, + "overhead_pct": 100 * (1 - env_med / physics_med), + "env_sps_min": min(env_samples), + "env_sps_max": max(env_samples), + } + + +def main(cfg: SyntheticConfig) -> None: + print("Synthetic Many-Term Throughput Benchmark (cartpole base)") + print(f" Envs: {cfg.num_envs} Steps: {cfg.num_steps} Reps: {cfg.reps}") + + results = [benchmark_one(cfg, n) for n in cfg.extra_terms] + + print("\n" + "=" * 72) + print( + f"{'Extra terms':>12} {'Physics SPS':>14} {'Env SPS':>14} " + f"{'Env min/max':>20} {'Overhead':>10}" + ) + print("-" * 72) + for r in results: + minmax = f"{r['env_sps_min']:,.0f}/{r['env_sps_max']:,.0f}" + print( + f"{r['extra_terms']:>12} {r['physics_sps']:>14,.0f} " + f"{r['env_sps']:>14,.0f} {minmax:>20} {r['overhead_pct']:>9.1f}%" + ) + + +if __name__ == "__main__": + cfg = tyro.cli(SyntheticConfig, config=mjlab.TYRO_FLAGS) + main(cfg) diff --git a/scripts/benchmarks/measure_throughput.py b/scripts/benchmarks/measure_throughput.py index e2ef7af517..d23c031740 100644 --- a/scripts/benchmarks/measure_throughput.py +++ b/scripts/benchmarks/measure_throughput.py @@ -7,6 +7,7 @@ from __future__ import annotations import json +import statistics import subprocess import time from dataclasses import asdict, dataclass, field @@ -35,13 +36,23 @@ class BenchmarkResult: physics_sps: float env_sps: float overhead_pct: float + reps: int = 1 + physics_sps_min: float | None = None + physics_sps_max: float | None = None + env_sps_min: float | None = None + env_sps_max: float | None = None def __str__(self) -> str: - return ( - f"{self.task} (dec={self.decimation}):\n" - f" Physics SPS: {self.physics_sps:,.0f}\n" - f" Env SPS: {self.env_sps:,.0f}\n" - f" Overhead: {self.overhead_pct:.1f}%" + physics_line = f" Physics SPS: {self.physics_sps:,.0f}" + env_line = f" Env SPS: {self.env_sps:,.0f}" + if self.reps > 1: + physics_line += ( + f" [min {self.physics_sps_min:,.0f} / max {self.physics_sps_max:,.0f}]" + ) + env_line += f" [min {self.env_sps_min:,.0f} / max {self.env_sps_max:,.0f}]" + header = f"{self.task} (dec={self.decimation}, reps={self.reps}):" + return "\n".join( + [header, physics_line, env_line, f" Overhead: {self.overhead_pct:.1f}%"] ) def to_dict(self) -> dict: @@ -61,6 +72,10 @@ class ThroughputConfig: warmup_steps: int = 50 """Number of warmup steps before measuring.""" + reps: int = 1 + """Number of measurement reps per task. Reported value is the median; min/max + are recorded alongside. Warmup runs once at the start of each task.""" + device: str = "cuda:0" """Device to run on.""" @@ -76,6 +91,10 @@ class ThroughputConfig: tracking_motion: str = "rll_humanoid/wandb-registry-Motions/lafan_cartwheel:latest" """W&B artifact path for tracking task motion (entity/project/name:alias).""" + tracking_motion_file: str | None = None + """Optional local motion.npz path. When set, skips W&B and uses this file + for any MotionCommandCfg in the task config.""" + output_dir: Path | None = None """Output directory for JSON results. If None, results are only printed.""" @@ -130,10 +149,13 @@ def benchmark_task(task: str, cfg: ThroughputConfig) -> BenchmarkResult: if len(env_cfg.commands) > 0: motion_cmd = env_cfg.commands.get("motion") if isinstance(motion_cmd, MotionCommandCfg): - api = wandb.Api() - artifact = api.artifact(cfg.tracking_motion) - motion_dir = artifact.download() - motion_cmd.motion_file = str(Path(motion_dir) / "motion.npz") + if cfg.tracking_motion_file is not None: + motion_cmd.motion_file = cfg.tracking_motion_file + else: + api = wandb.Api() + artifact = api.artifact(cfg.tracking_motion) + motion_dir = artifact.download() + motion_cmd.motion_file = str(Path(motion_dir) / "motion.npz") env = ManagerBasedRlEnv(cfg=env_cfg, device=cfg.device) env.reset() @@ -146,13 +168,21 @@ def benchmark_task(task: str, cfg: ThroughputConfig) -> BenchmarkResult: torch.cuda.synchronize() decimation = env.cfg.decimation - physics_sps = measure_physics_sps(env, cfg.num_steps) - - env.reset() - torch.cuda.synchronize() - - env_sps = measure_env_sps(env, cfg.num_steps) + physics_samples: list[float] = [] + env_samples: list[float] = [] + for rep in range(cfg.reps): + if cfg.reps > 1: + print(f" rep {rep + 1}/{cfg.reps}") + physics_samples.append(measure_physics_sps(env, cfg.num_steps)) + env.reset() + torch.cuda.synchronize() + env_samples.append(measure_env_sps(env, cfg.num_steps)) + env.reset() + torch.cuda.synchronize() + + physics_sps = statistics.median(physics_samples) + env_sps = statistics.median(env_samples) overhead_pct = 100 * (1 - env_sps / physics_sps) env.close() @@ -165,6 +195,11 @@ def benchmark_task(task: str, cfg: ThroughputConfig) -> BenchmarkResult: physics_sps=physics_sps, env_sps=env_sps, overhead_pct=overhead_pct, + reps=cfg.reps, + physics_sps_min=min(physics_samples) if cfg.reps > 1 else None, + physics_sps_max=max(physics_samples) if cfg.reps > 1 else None, + env_sps_min=min(env_samples) if cfg.reps > 1 else None, + env_sps_max=max(env_samples) if cfg.reps > 1 else None, ) diff --git a/scripts/benchmarks/profile_env_step.py b/scripts/benchmarks/profile_env_step.py new file mode 100644 index 0000000000..47a54adb9a --- /dev/null +++ b/scripts/benchmarks/profile_env_step.py @@ -0,0 +1,149 @@ +"""Profile env.step() to localize the cost of SceneEntityCfg indexing. + +Wraps a small window of ``env.step()`` calls in ``torch.profiler`` and +surfaces: + +- Top operators by CUDA self time. +- Per-step counts of ``aten::index`` / ``aten::index_put_`` — the operators + that fire on every ``tensor[:, cfg.joint_ids]`` with a Python list index. +- Per-step count of host-to-device ``Memcpy`` events, a proxy for the + list-of-int → CUDA-tensor copies issue #1019 is about. + +Supports two modes: + +- ``--task `` (e.g. ``Mjlab-Tracking-Flat-Unitree-G1``). + Pass ``--motion-file`` to skip wandb when the task needs one. +- ``--synthetic-extra-terms N``: cartpole inflated with N single-joint + obs+reward terms (each with an explicit ``joint_ids`` list, defeating the + ``slice(None)`` optimization). Use 0 as the control. +""" + +from __future__ import annotations + +import sys +from dataclasses import dataclass +from pathlib import Path + +import torch +import tyro +from torch.profiler import ProfilerActivity, profile + +import mjlab +import mjlab.tasks # noqa: F401 - registers tasks +from mjlab.envs import ManagerBasedRlEnv +from mjlab.tasks.cartpole.cartpole_env_cfg import cartpole_balance_env_cfg +from mjlab.tasks.registry import load_env_cfg +from mjlab.tasks.tracking.mdp.commands import MotionCommandCfg + +sys.path.insert(0, str(Path(__file__).parent)) +from measure_synthetic_throughput import _inflate # noqa: E402 + + +@dataclass +class ProfileConfig: + task: str | None = None + """Registered task name. Mutually exclusive with synthetic mode.""" + + synthetic_extra_terms: int | None = None + """If set, profile cartpole inflated with this many extra terms.""" + + motion_file: str | None = None + """Local path to motion.npz, used when task needs one (skips wandb).""" + + num_envs: int = 4096 + num_steps: int = 20 + """Number of env steps to profile. Keep small — profiler overhead is real.""" + + warmup_steps: int = 50 + device: str = "cuda:0" + trace_dir: Path = Path("profile_traces") + top_n: int = 25 + + +def _build_env(cfg: ProfileConfig) -> tuple[ManagerBasedRlEnv, str]: + if (cfg.task is None) == (cfg.synthetic_extra_terms is None): + raise SystemExit("Set exactly one of --task / --synthetic-extra-terms.") + + if cfg.synthetic_extra_terms is not None: + label = f"synthetic-cartpole-{cfg.synthetic_extra_terms}" + env_cfg = cartpole_balance_env_cfg() + env_cfg.scene.num_envs = cfg.num_envs + _inflate(env_cfg, cfg.synthetic_extra_terms) + else: + assert cfg.task is not None + label = cfg.task + env_cfg = load_env_cfg(cfg.task) + env_cfg.scene.num_envs = cfg.num_envs + motion_cmd = env_cfg.commands.get("motion") if len(env_cfg.commands) > 0 else None + if isinstance(motion_cmd, MotionCommandCfg): + if cfg.motion_file is None: + raise SystemExit( + f"Task {cfg.task} needs --motion-file (local motion.npz path)." + ) + motion_cmd.motion_file = cfg.motion_file + + env = ManagerBasedRlEnv(cfg=env_cfg, device=cfg.device) + return env, label + + +def _summarize(prof: profile, num_steps: int, top_n: int) -> None: + events = prof.key_averages() + + print(f"\nTop {top_n} ops by CUDA self time:") + print( + events.table( + sort_by="self_cuda_time_total", + row_limit=top_n, + max_name_column_width=55, + ) + ) + + def _count(name: str) -> int: + return sum(int(e.count) for e in events if e.key == name) + + per_step_index = _count("aten::index") / num_steps + per_step_index_put = _count("aten::index_put_") / num_steps + per_step_to_copy = _count("aten::_to_copy") / num_steps + per_step_index_select = _count("aten::index_select") / num_steps + + print("\nPer-step op counts (the implicit-sync suspects):") + print(f" aten::index : {per_step_index:.1f}") + print(f" aten::index_put_ : {per_step_index_put:.1f}") + print(f" aten::index_select : {per_step_index_select:.1f}") + print(f" aten::_to_copy : {per_step_to_copy:.1f}") + + +def main(cfg: ProfileConfig) -> None: + env, label = _build_env(cfg) + env.reset() + + action_dim = sum(env.action_manager.action_term_dim) + action = torch.zeros(env.num_envs, action_dim, device=env.device) + for _ in range(cfg.warmup_steps): + env.step(action) + torch.cuda.synchronize() + + print(f"\nProfiling {label} for {cfg.num_steps} steps (num_envs={cfg.num_envs})...") + + with profile( + activities=[ProfilerActivity.CPU, ProfilerActivity.CUDA], + record_shapes=False, + with_stack=False, + ) as prof: + for _ in range(cfg.num_steps): + env.step(action) + torch.cuda.synchronize() + + cfg.trace_dir.mkdir(parents=True, exist_ok=True) + trace_path = cfg.trace_dir / f"{label}.json" + prof.export_chrome_trace(str(trace_path)) + print(f"Chrome trace: {trace_path}") + + _summarize(prof, cfg.num_steps, cfg.top_n) + + env.close() + + +if __name__ == "__main__": + cfg = tyro.cli(ProfileConfig, config=mjlab.TYRO_FLAGS) + main(cfg) diff --git a/src/mjlab/entity/data.py b/src/mjlab/entity/data.py index b20f930517..f74bfe74ea 100644 --- a/src/mjlab/entity/data.py +++ b/src/mjlab/entity/data.py @@ -136,7 +136,7 @@ def write_joint_state( self, position: torch.Tensor, velocity: torch.Tensor, - joint_ids: torch.Tensor | slice | None = None, + joint_ids: Sequence[int] | torch.Tensor | slice | None = None, env_ids: torch.Tensor | slice | None = None, ) -> None: if not self.is_articulated: @@ -148,7 +148,7 @@ def write_joint_state( def write_joint_position( self, position: torch.Tensor, - joint_ids: torch.Tensor | slice | None = None, + joint_ids: Sequence[int] | torch.Tensor | slice | None = None, env_ids: torch.Tensor | slice | None = None, ) -> None: if not self.is_articulated: @@ -162,7 +162,7 @@ def write_joint_position( def write_joint_velocity( self, velocity: torch.Tensor, - joint_ids: torch.Tensor | slice | None = None, + joint_ids: Sequence[int] | torch.Tensor | slice | None = None, env_ids: torch.Tensor | slice | None = None, ) -> None: if not self.is_articulated: @@ -177,7 +177,7 @@ def write_external_wrench( self, force: torch.Tensor | None, torque: torch.Tensor | None, - body_ids: Sequence[int] | slice | None = None, + body_ids: Sequence[int] | torch.Tensor | slice | None = None, env_ids: torch.Tensor | slice | None = None, ) -> None: env_ids = self._resolve_env_ids(env_ids) diff --git a/src/mjlab/entity/entity.py b/src/mjlab/entity/entity.py index 7582a02c05..56a15c1dd5 100644 --- a/src/mjlab/entity/entity.py +++ b/src/mjlab/entity/entity.py @@ -933,7 +933,7 @@ def write_joint_state_to_sim( self, position: torch.Tensor, velocity: torch.Tensor, - joint_ids: torch.Tensor | slice | None = None, + joint_ids: Sequence[int] | torch.Tensor | slice | None = None, env_ids: torch.Tensor | slice | None = None, ): """Set the joint state into the simulation. @@ -1125,7 +1125,7 @@ def write_external_wrench_to_sim( forces: torch.Tensor, torques: torch.Tensor, env_ids: torch.Tensor | slice | None = None, - body_ids: Sequence[int] | slice | None = None, + body_ids: Sequence[int] | torch.Tensor | slice | None = None, ) -> None: """Apply external wrenches to bodies in the simulation. diff --git a/src/mjlab/envs/mdp/dr/actuator.py b/src/mjlab/envs/mdp/dr/actuator.py index 09c0546a2d..37a586a4aa 100644 --- a/src/mjlab/envs/mdp/dr/actuator.py +++ b/src/mjlab/envs/mdp/dr/actuator.py @@ -60,12 +60,13 @@ def pd_gains( else: env_ids = env_ids.to(env.device, dtype=torch.int) - if isinstance(asset_cfg.actuator_ids, list): - actuators = [asset.actuators[i] for i in asset_cfg.actuator_ids] - elif isinstance(asset_cfg.actuator_ids, slice): - actuators = asset.actuators[asset_cfg.actuator_ids] + actuator_ids = asset_cfg.actuator_ids + if isinstance(actuator_ids, slice): + actuators = asset.actuators[actuator_ids] else: - actuators = [asset.actuators[asset_cfg.actuator_ids]] + if isinstance(actuator_ids, torch.Tensor): + actuator_ids = actuator_ids.tolist() + actuators = [asset.actuators[i] for i in actuator_ids] for actuator in actuators: ctrl_ids = actuator.global_ctrl_ids @@ -219,13 +220,13 @@ def effort_limits( else: env_ids = env_ids.to(env.device, dtype=torch.int) - if isinstance(asset_cfg.actuator_ids, list): - actuators = [asset.actuators[i] for i in asset_cfg.actuator_ids] + actuator_ids = asset_cfg.actuator_ids + if isinstance(actuator_ids, slice): + actuators = asset.actuators[actuator_ids] else: - actuators = asset.actuators[asset_cfg.actuator_ids] - - if not isinstance(actuators, list): - actuators = [actuators] + if isinstance(actuator_ids, torch.Tensor): + actuator_ids = actuator_ids.tolist() + actuators = [asset.actuators[i] for i in actuator_ids] for actuator in actuators: ctrl_ids = actuator.global_ctrl_ids diff --git a/src/mjlab/envs/mdp/events.py b/src/mjlab/envs/mdp/events.py index 0872ec7141..5de208e096 100644 --- a/src/mjlab/envs/mdp/events.py +++ b/src/mjlab/envs/mdp/events.py @@ -301,15 +301,11 @@ def reset_joints_by_offset( joint_vel = default_joint_vel[env_ids][:, asset_cfg.joint_ids].clone() joint_vel += sample_uniform(*velocity_range, joint_vel.shape, env.device) - joint_ids = asset_cfg.joint_ids - if isinstance(joint_ids, list): - joint_ids = torch.tensor(joint_ids, device=env.device) - asset.write_joint_state_to_sim( joint_pos.view(len(env_ids), -1), joint_vel.view(len(env_ids), -1), env_ids=env_ids, - joint_ids=joint_ids, + joint_ids=asset_cfg.joint_ids, ) @@ -371,9 +367,9 @@ def apply_external_force_torque( env_ids = resolve_env_ids(env, env_ids) asset: Entity = env.scene[asset_cfg.name] num_bodies = ( - len(asset_cfg.body_ids) - if isinstance(asset_cfg.body_ids, list) - else asset.num_bodies + asset.num_bodies + if isinstance(asset_cfg.body_ids, slice) + else len(asset_cfg.body_ids) ) size = (len(env_ids), num_bodies, 3) forces = sample_uniform(*force_range, size, env.device) @@ -448,9 +444,9 @@ def __init__(self, cfg, env: ManagerBasedRlEnv): ) self._num_bodies = ( - len(self._body_ids) - if isinstance(self._body_ids, list) - else self._asset.num_bodies + self._asset.num_bodies + if isinstance(self._body_ids, slice) + else len(self._body_ids) ) self._cooldown_s: tuple[float, float] = cfg.params["cooldown_s"] diff --git a/src/mjlab/managers/scene_entity_config.py b/src/mjlab/managers/scene_entity_config.py index 93a44e875a..83ed0fec12 100644 --- a/src/mjlab/managers/scene_entity_config.py +++ b/src/mjlab/managers/scene_entity_config.py @@ -3,6 +3,8 @@ from dataclasses import dataclass, field from typing import NamedTuple +import torch + from mjlab.entity import Entity from mjlab.scene import Scene @@ -51,13 +53,19 @@ class _FieldConfig(NamedTuple): ] +_IdsField = list[int] | torch.Tensor | slice + + @dataclass class SceneEntityCfg: """Configuration for a scene entity that is used by the manager's term. - This configuration allows flexible specification of entity components either by name - or by ID. During resolution, it ensures consistency between names and IDs, and can - optimize to slice(None) when all components are selected. + Users specify components by name (str/tuple) or by raw IDs (``list[int]``). + After :meth:`resolve`, every ``*_ids`` field is either a ``slice`` (when all + components are selected) or a ``torch.long`` tensor on ``scene.device``. + Indexing GPU tensors with these resolved IDs avoids the implicit H2D copy + and stream synchronization that a Python ``list[int]`` would cause on every + ``tensor[:, ids]`` access. """ name: str @@ -66,62 +74,72 @@ class SceneEntityCfg: joint_names: str | tuple[str, ...] | None = None """Names of joints to include. Can be a single string or tuple.""" - joint_ids: list[int] | slice = field(default_factory=lambda: slice(None)) - """IDs of joints to include. Can be a list or slice.""" + joint_ids: _IdsField = field(default_factory=lambda: slice(None)) + """IDs of joints to include. After resolve(), a tensor on ``scene.device`` + or :class:`slice` for "all".""" body_names: str | tuple[str, ...] | None = None """Names of bodies to include. Can be a single string or tuple.""" - body_ids: list[int] | slice = field(default_factory=lambda: slice(None)) - """IDs of bodies to include. Can be a list or slice.""" + body_ids: _IdsField = field(default_factory=lambda: slice(None)) + """IDs of bodies to include. After resolve(), a tensor on ``scene.device`` + or :class:`slice` for "all".""" geom_names: str | tuple[str, ...] | None = None """Names of geometries to include. Can be a single string or tuple.""" - geom_ids: list[int] | slice = field(default_factory=lambda: slice(None)) - """IDs of geometries to include. Can be a list or slice.""" + geom_ids: _IdsField = field(default_factory=lambda: slice(None)) + """IDs of geometries to include. After resolve(), a tensor on + ``scene.device`` or :class:`slice` for "all".""" site_names: str | tuple[str, ...] | None = None """Names of sites to include. Can be a single string or tuple.""" - site_ids: list[int] | slice = field(default_factory=lambda: slice(None)) - """IDs of sites to include. Can be a list or slice.""" + site_ids: _IdsField = field(default_factory=lambda: slice(None)) + """IDs of sites to include. After resolve(), a tensor on ``scene.device`` + or :class:`slice` for "all".""" actuator_names: str | list[str] | None = None """Names of actuators to include. Can be a single string or list.""" - actuator_ids: list[int] | slice = field(default_factory=lambda: slice(None)) - """IDs of actuators to include. Can be a list or slice.""" + actuator_ids: _IdsField = field(default_factory=lambda: slice(None)) + """IDs of actuators to include. After resolve(), a tensor on + ``scene.device`` or :class:`slice` for "all".""" tendon_names: str | tuple[str, ...] | None = None """Names of tendons to include. Can be a single string or tuple.""" - tendon_ids: list[int] | slice = field(default_factory=lambda: slice(None)) - """IDs of tendons to include. Can be a list or slice.""" + tendon_ids: _IdsField = field(default_factory=lambda: slice(None)) + """IDs of tendons to include. After resolve(), a tensor on ``scene.device`` + or :class:`slice` for "all".""" camera_names: str | tuple[str, ...] | None = None """Names of cameras to include. Can be a single string or tuple.""" - camera_ids: list[int] | slice = field(default_factory=lambda: slice(None)) - """IDs of cameras to include. Can be a list or slice.""" + camera_ids: _IdsField = field(default_factory=lambda: slice(None)) + """IDs of cameras to include. After resolve(), a tensor on ``scene.device`` + or :class:`slice` for "all".""" light_names: str | tuple[str, ...] | None = None """Names of lights to include. Can be a single string or tuple.""" - light_ids: list[int] | slice = field(default_factory=lambda: slice(None)) - """IDs of lights to include. Can be a list or slice.""" + light_ids: _IdsField = field(default_factory=lambda: slice(None)) + """IDs of lights to include. After resolve(), a tensor on ``scene.device`` + or :class:`slice` for "all".""" material_names: str | tuple[str, ...] | None = None """Names of materials to include. Can be a single string or tuple.""" - material_ids: list[int] | slice = field(default_factory=lambda: slice(None)) - """IDs of materials to include. Can be a list or slice.""" + material_ids: _IdsField = field(default_factory=lambda: slice(None)) + """IDs of materials to include. After resolve(), a tensor on + ``scene.device`` or :class:`slice` for "all".""" pair_names: str | tuple[str, ...] | None = None """Names of contact pairs to include. Can be a single string or tuple.""" - pair_ids: list[int] | slice = field(default_factory=lambda: slice(None)) - """IDs of contact pairs to include. Can be a list or slice.""" + pair_ids: _IdsField = field(default_factory=lambda: slice(None)) + """IDs of contact pairs to include. After resolve(), a tensor on + ``scene.device`` or :class:`slice` for "all".""" preserve_order: bool = False """If True, maintains the order of components as specified.""" @@ -135,6 +153,9 @@ def resolve(self, scene: Scene) -> None: 2. Only names provided: Computes IDs (optimizes to slice(None) if all selected) 3. Only IDs provided: Computes names + After resolution, ``*_ids`` fields are materialized as ``torch.long`` + tensors on ``scene.device`` (or kept as ``slice(None)`` when all selected). + Args: scene: The scene containing the entity to resolve against. @@ -147,6 +168,18 @@ def resolve(self, scene: Scene) -> None: for config in _FIELD_CONFIGS: self._resolve_field(entity, config) + # Materialize list ids onto the scene device so per-step indexing avoids + # the implicit H2D copy + stream sync that a list[int] triggers. + device = torch.device(scene.device) + for config in _FIELD_CONFIGS: + ids = getattr(self, config.ids_attr) + if isinstance(ids, list): + setattr( + self, + config.ids_attr, + torch.as_tensor(ids, dtype=torch.long, device=device), + ) + def _resolve_field(self, entity: Entity, config: _FieldConfig) -> None: """Resolve a single field's names and IDs. diff --git a/src/mjlab/tasks/velocity/mdp/rewards.py b/src/mjlab/tasks/velocity/mdp/rewards.py index 12fc9d9ad9..cff8605bde 100644 --- a/src/mjlab/tasks/velocity/mdp/rewards.py +++ b/src/mjlab/tasks/velocity/mdp/rewards.py @@ -90,7 +90,7 @@ def __call__( ) -> torch.Tensor: asset: Entity = env.scene[asset_cfg.name] - if asset_cfg.body_ids: + if not isinstance(asset_cfg.body_ids, slice): body_quat_w = asset.data.body_link_quat_w[:, asset_cfg.body_ids, :] # [B, N, 4] body_quat_w = body_quat_w.squeeze(1) # [B, 4] else: @@ -124,7 +124,7 @@ def debug_vis(self, visualizer: DebugVisualizer) -> None: return terrain_normal = terrain_normal_from_sensors(env, self._terrain_sensor_names) - if self._asset_cfg.body_ids: + if not isinstance(self._asset_cfg.body_ids, slice): body_quat_w = asset.data.body_link_quat_w[:, self._asset_cfg.body_ids, :].squeeze( 1 ) diff --git a/tests/test_manager_config_immutability.py b/tests/test_manager_config_immutability.py index 963e5bda2b..6a7744b9f2 100644 --- a/tests/test_manager_config_immutability.py +++ b/tests/test_manager_config_immutability.py @@ -150,7 +150,13 @@ def test_scene_entity_cfg_in_params_not_mutated(device): env.num_envs = 2 env.device = device env.max_episode_length_s = 10.0 - env.scene = {"robot": entity} + + class _FakeScene(dict): + def __init__(self, items, device: str) -> None: + super().__init__(items) + self.device = device + + env.scene = _FakeScene({"robot": entity}, device=device) # Select only joint1 --> joint_ids will mutate from slice(None) to [0]. asset_cfg = SceneEntityCfg(name="robot", joint_names=("joint1",)) diff --git a/tests/test_scene_entity_config.py b/tests/test_scene_entity_config.py index 18b2eff358..0fe8bf5ae7 100644 --- a/tests/test_scene_entity_config.py +++ b/tests/test_scene_entity_config.py @@ -3,6 +3,7 @@ from dataclasses import dataclass import pytest +import torch from mjlab.managers.scene_entity_config import SceneEntityCfg @@ -71,10 +72,15 @@ def fake_entity() -> _FakeEntity: ) +class _FakeScene(dict): + """Minimal scene-like mapping with a ``device`` attribute.""" + + device = "cpu" + + @pytest.fixture def fake_scene(fake_entity: _FakeEntity): - # Minimal scene-like mapping. - return {fake_entity.name: fake_entity} + return _FakeScene({fake_entity.name: fake_entity}) @pytest.mark.parametrize( @@ -155,7 +161,8 @@ def test_tuple_names_with_consistent_ids(fake_scene): cfg.resolve(fake_scene) assert cfg.joint_names == ["a", "b"] - assert cfg.joint_ids == [0, 1] + assert isinstance(cfg.joint_ids, torch.Tensor) + assert cfg.joint_ids.tolist() == [0, 1] @pytest.mark.parametrize( @@ -180,7 +187,7 @@ def test_names_reordered_to_match_ids_when_not_preserving_order( cfg.resolve(fake_scene) # IDs and names must both be in internal order: a=0, c=2. - assert getattr(cfg, ids_attr) == [0, 2] + assert getattr(cfg, ids_attr).tolist() == [0, 2] assert getattr(cfg, names_attr) == ["a", "c"] @@ -203,5 +210,5 @@ def test_names_preserve_user_order_when_preserving_order(fake_scene, field_names cfg.resolve(fake_scene) # User order preserved: c=2, a=0. - assert getattr(cfg, ids_attr) == [2, 0] + assert getattr(cfg, ids_attr).tolist() == [2, 0] assert getattr(cfg, names_attr) == ["c", "a"] diff --git a/tests/test_velocity_rewards.py b/tests/test_velocity_rewards.py index 579bc08a0e..03776aaf39 100644 --- a/tests/test_velocity_rewards.py +++ b/tests/test_velocity_rewards.py @@ -51,13 +51,13 @@ def _make_env_and_reward( if body_quat_w is None: body_quat_w = _identity_quat(B) - # Mock asset data. Use explicit asset_cfg with no body_names so - # body_ids stays None and the reward uses root_link_quat_w. + # Mock asset data. Default asset_cfg leaves body_ids as slice(None) so the + # reward uses root_link_quat_w. asset = MagicMock() asset.data.root_link_quat_w = body_quat_w asset.data.root_link_pos_w = torch.zeros(B, 3) asset.data.gravity_vec_w = torch.tensor([0.0, 0.0, -1.0]).expand(B, 3) - asset_cfg = SceneEntityCfg("robot", body_names=None, body_ids=[]) + asset_cfg = SceneEntityCfg("robot") # Mock terrain sensor if needed. sensors: dict = {"robot": asset}