Decouple camera observable refresh from video recording - #26
Open
RuiyangSi wants to merge 1 commit into
Open
Conversation
`_do_robosuite_step` gated robosuite's observable refresh on `_record_frames`, so `--record-video` decided whether image/depth observables were updated during motion. The affected `get_observation()` implementations call `_get_observations()` without `force_update=True` and therefore return cached values, which meant that with recording off (the default, and what run_capbench_sweep.py uses) perception reused the RGB-D captured at reset() for the whole episode. Programs that re-perceive after moving an object grasped stale poses and failed deterministically. Physics was never affected -- both branches reach sim.step(). The divergence is closed-loop: stale perception -> wrong object pose -> wrong IK target. Stepping no longer depends on recording, and the four affected simulators refresh cameras on demand via force_update=True. This keeps the optimization (cameras render only when an observation is requested). The equivalent branches in the handover env are removed for consistency; it already forced updates, so its behaviour is unchanged. cube_restack, 20 trials, avg reward / task completed: --record-video False 0.002 / 0 -> 0.502 / 10 --record-video True 0.454 / 9 -> 0.452 / 9 cube_lifting (0.977 / 19) and spill_wipe (1.000 / 20) are unchanged, as their oracles only perceive before motion. Refs capgym#25 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Decouples Robosuite camera observation refresh from video recording so perception uses current simulation state.
Changes:
- Refreshes camera observations on demand in four simulators.
- Removes recording-dependent stepping branches.
- Preserves read-only video rendering and Viser debugging.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Summary |
|---|---|
capx/envs/simulators/robosuite_spill_wipe.py |
Forces fresh camera observations. |
capx/envs/simulators/robosuite_handover.py |
Removes recording-gated render branches. |
capx/envs/simulators/robosuite_cubes.py |
Forces fresh camera observations. |
capx/envs/simulators/robosuite_cubes_restack.py |
Forces fresh camera observations. |
capx/envs/simulators/robosuite_cube_lift.py |
Forces fresh camera observations. |
capx/envs/simulators/robosuite_base.py |
Decouples simulation stepping from recording. |
Suppressed comments (1)
capx/envs/simulators/robosuite_base.py:145
- When
viser_serveris present, this normalstep()already refreshes the camera observables, and_update_viser_server()then immediately callsget_observation(), which now force-updates them again. Each debug update therefore renders the RGB-D/segmentation sensors twice and discards the first result; keep the step's image skip (as the handover paths do) or pass the step observation through to the viewer.
if hasattr(self, "viser_server"):
self.robosuite_env.step(sliced)
else:
self.robosuite_env.step(sliced, skip_render_images=True)
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #25.
Problem
--record-videochanged task outcomes, not just whether media was saved._do_robosuite_stepgated robosuite's observable refresh on_record_frames:skip_render_images=Truemakes robosuite skip every_image/_depth/_segmentation_instanceobservable. The affectedget_observation()methodsthen call
_get_observations()withoutforce_update=True, so they return thecached
_current_observed_valueinstead of invoking the sensor.With recording off — the default, and what
scripts/run_capbench_sweep.pyuses —perception therefore reused the RGB-D captured at
reset()for the entireepisode. Programs that re-perceive after moving an object grasped stale poses
and failed deterministically.
Physics was never affected: both branches reach
sim.step(). The divergence isclosed-loop (stale perception -> wrong object pose -> wrong IK target).
Measured on
cube_restack, after the program relocates the green cube andre-queries its pose:
Change
_do_robosuite_stepno longer looks at_record_frames; only the viser debugview still forces a full step.
cube_lift,cubes,cubes_restack,spill_wipe) refresh cameras on demand via_get_observations(force_update=True).robosuite_handover.pyare removed forconsistency. That env already forced updates, so its behaviour is unchanged.
This keeps the original optimization — cameras render only when an observation is
actually requested — while guaranteeing perception sees current state.
_record_frame()is untouched; it uses the read-onlysim.render().Not affected, and deliberately left alone:
robosuite_nut_assembly.pyandrobosuite_handover.pyalready passforce_update=True;robosuite_two_arm_lift.pysteps independently of recording; privileged (S1)APIs read state observables, which
skip_render_imagesnever skips.Verification
cube_restack, 20 trials, avg reward / task completed:--record-video False--record-video TrueThe two modes now agree, which is the actual correctness criterion here.
Unchanged, as expected (their oracles only perceive before motion):
cube_lifting0.977 / 19,spill_wipe1.000 / 20.tests/test_environments.pygives identical results before and after thischange on my setup (the pre-existing failures there are unrelated —
LIBERO/Isaac deps).
Impact on published numbers
Benchmark cells run with recording off are affected: 4 tasks x the 8
non-privileged tiers.
cube_restackis definitively invalidated (its near-zerovision scores were this bug, not a perception-grounding ceiling);
cube_stackis likely materially affected.
cube_liftingandspill_wipesingle-turnoracles are not, though generated multi-turn programs that re-perceive are.