Skip to content

Commit 347daa1

Browse files
committed
feat(sim,training): actuator-keyed policies, vertical-camera fix, RL parity (evaluate/vec/gym/staged_reward)
Sim bug fixes (found by running policies across embodiments, each pinned): - robot_action_keys(): policies were keyed by ALL joint names incl. passive/ mimic followers and missed tendon actuators, so g1/aloha/xarm7/stretch/hands silently no-op'd. Now keyed by the robot's actuators (what send_action resolves). MuJoCo override + SimEngine ABC default (other backends unchanged). - spec_builder._target_quat: returned None for top-down/vertical cameras (forward parallel to world +Z -> degenerate cross). Falls back to +Y up. - mujoco render(output_path=...): persist PNG for verifiable renders, with path-traversal / shell-metachar guard on the LLM-supplied path. - eval_policy: auto-resolves the single robot (API parity with run_policy). - DeclarativeBenchmark.on_episode_start: reset stateful reward terms (no phase leak across episodes). RL training parity (4 gaps closed, CPU-tested, no new required deps): - predicates.staged_reward: declarative monotonic phase-machine reward (StatefulRewardTerm). Curricula authored as DATA via the closed predicate registry - never eval, never shipped task code. - base_algo.evaluate()/load_checkpoint()/_deterministic_action(): deterministic eval peer of train(), frozen normalizer, success_rate; works for PPO + SAC. - SimEnv.n_substeps (default 5): position-target PD needs substeps to track; n_substeps=1 was the "nothing learns" bug. Resets stateful reward terms. - PpoTrainer: vectorized rollout path; num_envs>1 now accepted (was hard- rejected). N=1 path byte-identical (dispatch on _vectorized). - VecSimEnv: N independent SimEnv, one reused thread pool, autoreset with terminal_obs capture (load-bearing for GAE bootstrap), homogeneity guard. - GymSimEnv: gymnasium adapter with correct terminated-vs-truncated split (the SB3 TimeLimit footgun fixed at the source) + critic-obs passthrough. Tests: 38 new CPU tests (vec_env, evaluate, vectorized-ppo, gym_env, staged_reward) + SO-101 e2e wiring/convergence integ. Updated 3 existing tests that encoded the old contracts (num_envs==1 rejection, eval_policy robot_name).
1 parent c991151 commit 347daa1

16 files changed

Lines changed: 2024 additions & 11 deletions

File tree

strands_robots/simulation/benchmark_spec.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -236,6 +236,13 @@ def on_episode_start(self, sim: SimEngine, rng: random.Random) -> None:
236236
raise RuntimeError(
237237
f"DeclarativeBenchmark '{self._name}': load_scene({self._scene!r}) failed: {msg}"
238238
)
239+
# Reset any stateful reward terms (e.g. a staged_reward phase machine)
240+
# so per-episode phase state does not leak across episodes. Stateless
241+
# function terms have no reset() and are skipped.
242+
for term in self._reward_terms:
243+
term_reset = getattr(term, "reset", None)
244+
if callable(term_reset):
245+
term_reset()
239246
super().on_episode_start(sim, rng)
240247

241248
def on_step(self, sim: SimEngine, obs: dict[str, Any], action: dict[str, Any]) -> StepInfo:

strands_robots/simulation/mujoco/rendering.py

Lines changed: 38 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -571,10 +571,21 @@ def _scale_ctrl_for_actuator(model: Any, ai: int, value: float, mj: Any) -> floa
571571
return lo + frac * span
572572

573573
def render(
574-
self, camera_name: str = "default", width: int | None = None, height: int | None = None
574+
self,
575+
camera_name: str = "default",
576+
width: int | None = None,
577+
height: int | None = None,
578+
output_path: str | None = None,
575579
) -> dict[str, Any]:
576580
"""Render a camera view to a PNG image.
577581
582+
When ``output_path`` is given the PNG is ALSO written to that file path
583+
(parent dirs created) and the saved path is reported in the ``json``
584+
block as ``saved_path`` and in the text summary. This lets an agent (or
585+
a human) persist a render for independent verification instead of only
586+
receiving the bytes inline.
587+
588+
578589
Returns an agent-tool dict with ``status`` and a ``content`` list; on
579590
success the content holds an ``image`` block carrying PNG bytes
580591
(``{"image": {"format": "png", "source": {"bytes": ...}}}``) plus a
@@ -662,12 +673,36 @@ def render(
662673
pixel_var = float(_np.var(img))
663674
pixel_mean = float(_np.mean(img))
664675

676+
saved_path: str | None = None
677+
if output_path:
678+
import os as _os
679+
680+
# Validate against shell/path-traversal injection (LLM-supplied).
681+
bad = {";", "|", "$", "`", ">", "<", "\n", "\r", "\x00"}
682+
if any(b in output_path for b in bad) or ".." in output_path.split("/"):
683+
return {
684+
"status": "error",
685+
"content": [{"text": f"render: unsafe output_path {output_path!r}"}],
686+
}
687+
_dir = _os.path.dirname(_os.path.abspath(output_path))
688+
_os.makedirs(_dir, exist_ok=True)
689+
with open(output_path, "wb") as _f:
690+
_f.write(png_bytes)
691+
saved_path = _os.path.abspath(output_path)
692+
693+
summary = f"{w}x{h} from '{label}' at t={self._world.sim_time:.3f}s"
694+
if saved_path:
695+
summary += f" -> saved {saved_path}"
696+
json_block = {"pixel_variance": pixel_var, "pixel_mean": pixel_mean, "camera": label}
697+
if saved_path:
698+
json_block["saved_path"] = saved_path
699+
665700
return {
666701
"status": "success",
667702
"content": [
668-
{"text": f"{w}x{h} from '{label}' at t={self._world.sim_time:.3f}s"},
703+
{"text": summary},
669704
{"image": {"format": "png", "source": {"bytes": png_bytes}}},
670-
{"json": {"pixel_variance": pixel_var, "pixel_mean": pixel_mean, "camera": label}},
705+
{"json": json_block},
671706
],
672707
}
673708
except Exception as e:

strands_robots/simulation/predicates.py

Lines changed: 159 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -554,6 +554,162 @@ def term(_sim: SimEngine) -> float:
554554
return term
555555

556556

557+
# Stateful reward terms (declarative phase machine)
558+
#
559+
# A plain RewardTerm is stateless: ``(SimEngine) -> float``. Some rewards need
560+
# memory across steps - a pick-place curriculum advances Reach -> Grasp ->
561+
# Transport -> Place, awards a one-time bonus on each transition, and only ever
562+
# moves forward. Rather than hardcode any specific task, we expose ONE
563+
# generic primitive, ``staged_reward``, that composes EXISTING registry
564+
# predicates into a phase machine. The task itself is then authored as data
565+
# (a spec dict / YAML) by a human or LLM - never as shipped code, and never via
566+
# ``eval`` (sub-predicates are compiled through :func:`make_predicate`, the same
567+
# closed-registry path as every other DSL call).
568+
569+
570+
class StatefulRewardTerm:
571+
"""A reward term that carries per-episode state and must be ``reset()``.
572+
573+
Duck-typed by consumers: anything with ``__call__(sim) -> float`` AND a
574+
zero-arg ``reset()`` is treated as episode-stateful. ``SimEnv.reset`` and
575+
``DeclarativeBenchmark.on_episode_start`` call ``reset()`` on any reward
576+
term that has it, so stateless plain-function terms are unaffected.
577+
"""
578+
579+
def __call__(self, sim: SimEngine) -> float: # pragma: no cover - interface
580+
raise NotImplementedError
581+
582+
def reset(self) -> None: # pragma: no cover - interface
583+
raise NotImplementedError
584+
585+
586+
class _StagedReward(StatefulRewardTerm):
587+
"""Monotonic multi-stage (phase-machine) reward built from sub-predicates.
588+
589+
Each stage declares:
590+
- ``reward``: a float-valued registry predicate giving the dense
591+
shaping signal while the machine is IN that stage.
592+
- ``advance_when``: a bool-valued registry predicate; the FIRST step it
593+
returns True the machine awards ``bonus`` once and advances to the
594+
next stage. Phases only ever move forward (no regression), matching
595+
curriculum semantics and giving a stable, non-oscillating signal.
596+
- ``bonus``: a one-time scalar added on the transition out of the stage
597+
(default 0.0).
598+
599+
The last stage has no ``advance_when`` gate (the task is "done" there for
600+
reward purposes; episode termination is a separate ``success`` predicate).
601+
Per step the emitted reward is ``current_stage.reward(sim) +
602+
(bonus if this step advanced else 0.0)``.
603+
"""
604+
605+
def __init__(
606+
self,
607+
stages: list[tuple[RewardTerm, BoolPredicate | None, float]],
608+
) -> None:
609+
self._stages = stages
610+
self._phase = 0
611+
612+
def reset(self) -> None:
613+
self._phase = 0
614+
615+
@property
616+
def phase(self) -> int:
617+
"""Current stage index (0-based). Exposed for logging / tests."""
618+
return self._phase
619+
620+
def __call__(self, sim: SimEngine) -> float:
621+
if not self._stages:
622+
return 0.0
623+
phase = min(self._phase, len(self._stages) - 1)
624+
reward_fn, advance_fn, bonus = self._stages[phase]
625+
r = float(reward_fn(sim))
626+
# Advance (and award the one-time bonus) only if there IS a next stage
627+
# and this stage declares a gate that now fires.
628+
if self._phase < len(self._stages) - 1 and advance_fn is not None and bool(advance_fn(sim)):
629+
self._phase += 1
630+
return r + float(bonus)
631+
return r
632+
633+
634+
def _staged_reward(stages: list[Any]) -> RewardTerm:
635+
"""Factory: compile a declared stage list into a :class:`_StagedReward`.
636+
637+
This is the single new primitive that turns the stateless DSL into a
638+
declarative phase machine. It recursively compiles each stage's ``reward``
639+
and ``advance_when`` through :func:`make_predicate`, so the whole thing
640+
stays inside the closed-registry / no-``eval`` safety contract: a spec can
641+
only ever reference predicates that already exist in the registry.
642+
643+
Args:
644+
stages: Ordered list of stage dicts. Each stage::
645+
646+
{
647+
"reward": {"predicate": <float-term name>, **kwargs},
648+
"advance_when": {"predicate": <bool-pred name>, **kwargs}, # omit on last stage
649+
"bonus": <float>, # optional, default 0.0
650+
}
651+
652+
Returns:
653+
A callable+resettable :class:`_StagedReward`.
654+
655+
Raises:
656+
ValueError: stages is not a non-empty list, a stage is malformed, a
657+
non-final stage omits ``advance_when``, or ``bonus`` is non-numeric.
658+
TypeError: surfaced from :func:`make_predicate` for bad sub-kwargs.
659+
"""
660+
if not isinstance(stages, list) or not stages:
661+
raise ValueError("staged_reward: 'stages' must be a non-empty list of stage dicts")
662+
663+
compiled: list[tuple[RewardTerm, BoolPredicate | None, float]] = []
664+
n = len(stages)
665+
for i, stage in enumerate(stages):
666+
if not isinstance(stage, dict):
667+
raise ValueError(f"staged_reward: stage[{i}] must be a dict, got {type(stage).__name__}")
668+
unknown = set(stage.keys()) - {"reward", "advance_when", "bonus"}
669+
if unknown:
670+
raise ValueError(
671+
f"staged_reward: stage[{i}] has unknown keys {sorted(unknown)}; "
672+
"allowed: reward, advance_when, bonus"
673+
)
674+
675+
reward_call = stage.get("reward")
676+
if not isinstance(reward_call, dict) or "predicate" not in reward_call:
677+
raise ValueError(
678+
f"staged_reward: stage[{i}].reward must be a predicate-call dict "
679+
"like {predicate: distance_neg, body_a: ..., body_b: ...}"
680+
)
681+
reward_name = reward_call["predicate"]
682+
reward_kwargs = {k: v for k, v in reward_call.items() if k != "predicate"}
683+
reward_fn = make_predicate(reward_name, **reward_kwargs)
684+
685+
advance_call = stage.get("advance_when")
686+
advance_fn: BoolPredicate | None
687+
if advance_call is None:
688+
if i != n - 1:
689+
raise ValueError(
690+
f"staged_reward: stage[{i}] is not the final stage and must declare "
691+
"'advance_when' (a bool predicate gating the transition to the next stage)"
692+
)
693+
advance_fn = None
694+
else:
695+
if not isinstance(advance_call, dict) or "predicate" not in advance_call:
696+
raise ValueError(
697+
f"staged_reward: stage[{i}].advance_when must be a predicate-call dict "
698+
"like {predicate: distance_less_than, body_a: ..., body_b: ..., threshold: ...}"
699+
)
700+
advance_name = advance_call["predicate"]
701+
advance_kwargs = {k: v for k, v in advance_call.items() if k != "predicate"}
702+
advance_fn = make_predicate(advance_name, **advance_kwargs)
703+
704+
bonus_raw = stage.get("bonus", 0.0)
705+
if isinstance(bonus_raw, bool) or not isinstance(bonus_raw, (int, float)):
706+
raise ValueError(f"staged_reward: stage[{i}].bonus must be a number, got {bonus_raw!r}")
707+
708+
compiled.append((reward_fn, advance_fn, float(bonus_raw)))
709+
710+
return _StagedReward(compiled)
711+
712+
557713
# Registry
558714

559715
PREDICATE_REGISTRY: dict[str, PredicateFactory] = {
@@ -574,6 +730,8 @@ def term(_sim: SimEngine) -> float:
574730
"distance_neg": _distance_neg,
575731
"joint_progress": _joint_progress,
576732
"constant": _constant,
733+
# stateful (phase machine)
734+
"staged_reward": _staged_reward,
577735
}
578736

579737

@@ -634,6 +792,7 @@ def make_predicate(name: str, **kwargs: Any) -> Callable[[SimEngine], Any]:
634792
"BoolPredicate",
635793
"PredicateFactory",
636794
"RewardTerm",
795+
"StatefulRewardTerm",
637796
"make_predicate",
638797
"register_predicate",
639798
]

strands_robots/training/rl/__init__.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@
2222

2323
from strands_robots.training.rl.base_algo import BaseRLAlgo, RLTrainSpec
2424
from strands_robots.training.rl.env import SimEnv
25+
from strands_robots.training.rl.gym_env import GymSimEnv
26+
from strands_robots.training.rl.vec_env import VecSimEnv
2527
from strands_robots.training.rl.fast_sac import FastSacTrainer
2628
from strands_robots.training.rl.normalization import EmpiricalNormalization
2729
from strands_robots.training.rl.ppo import PpoTrainer
@@ -34,5 +36,7 @@
3436
"FastSacTrainer",
3537
"SimpleReplayBuffer",
3638
"SimEnv",
39+
"GymSimEnv",
40+
"VecSimEnv",
3741
"EmpiricalNormalization",
3842
]

0 commit comments

Comments
 (0)