Sample the skill-factory parameters in Grow's and Coffee's NSRTs - #201
Merged
Merged
Conversation
Circuit, Laser and Switch looked up their switch joints without physicsClientId, so PyBullet answered from client 0, the first world the process built. Built after any other env (through the RoboDisco wrapper, in a test session, or in scripts/robodisco_getting_started.py) they read the wrong world and failed with "getJointState failed; invalid jointIndex" or "GetJointInfo failed.". The same omission sat in calls that never crash. Laser created its beam bodies in client 0, so the planner's world drew beams into the executing env's world, and its bookkeeping then removed bodies from its own. Coffee joined its cord segments and its plugged-in plug with constraints in client 0, and the Domino fan component read its switch joints from client 0. Each of these calls now names the env's client, as do the few calls that passed it positionally, so tests/test_pybullet_client_ids.py can require physicsClientId= on every PyBullet call that reaches a physics server. Regression tests build Circuit, Laser and Switch after Blocks in one process and check that Laser's beams appear in its own world only. The envs README and the smoke script no longer describe the failure, and the smoke script numbers its seven steps out of seven.
Both oracle approaches crashed on Domino and Fan in the default configuration, for unrelated reasons. Domino's ground-truth processes were written for domino targets (domino_use_domino_blocks_as_target=True, the benchmark setting). By default the targets are hinged flaps and Toppled ranges over them, so the process for a domino falling flat built Toppled(?d1) over a domino and failed LiftedAtom's type check (structs.py:880). That process now adds Toppled only when dominoes are the targets; with hinged targets a fallen domino just stops tilting. No process models a hinged target falling, so both oracles now run and report the goal unreachable. Fan's default scene (the exposed deck since #196) attaches a FanTransferEvaluator to each task. The process planner rewrites the goal BallAtTarget into the grid goal BallAtLoc but kept the evaluator, failing Task's check that an evaluator judges the task's own goal (structs.py:1051). The rewritten task now drops it, as replace_goal_with_alt_goal does; the planner never reads it. oracle_process_planning then runs, but its grid processes cannot reach the goal cell. The oracle approach already ran without crashing. The envs README now lists Domino and Fan among the environments that run but do not solve the test task.
Grow's NSRTs still sampled for the legacy options: no parameters for PickJug and a normalized (x, y) for Place. The default skill-factory options take a grasp height for PickJug and a world (x, y, release_z, yaw) target for Place. Grounding let the empty grasp height through, so Grow's oracle ran PickJug without one and raised "IndexError: index 0 is out of bounds" in pick.py's _descend_pose. Coffee's NSRTs have the same mismatch for PickJug, PlaceJugInMachine and TurnMachineOn; its oracle reaches it once Coffee's planner can read its cups. With the skill-factory options both factories now use the samplers their processes already use, as Bridge's NSRTs do; the legacy options and the 2D coffee env keep theirs. A test grounds every NSRT of both environments and checks the sampled parameters against the option's parameter space. Grow's oracle now runs but times out refining its plan, so Grow still solves only with oracle_process_planning. The module docstrings of Grow's ground-truth models no longer call it the coffee environment.
This was referenced Oct 3, 2026
yichao-liang
changed the base branch from
fix-domino-fan-oracle-default
to
master
October 3, 2026 07:59
8 tasks done
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
1 participant
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.
Summary
The bug.
--approach oracleonpybullet_growraisedIndexError: index 0 is out of bounds for axis 0 with size 0in_descend_pose(skill_factories/pick.py).Grow's NSRTs still sampled for the legacy options: no parameters for
PickJugand a normalized(x, y)forPlace.The default skill-factory options take a grasp height for
PickJugand a world(x, y, release_z, yaw)target forPlace.ParameterizedOption.groundlet the empty grasp height through, because a wrong-shaped array passes its float-tolerance fallback, so the failure only surfaced inside the skill.Coffee's NSRTs have the same mismatch for
PickJug,PlaceJugInMachineandTurnMachineOn; its oracle reaches it once Coffee's planner can read its cups (#202).Sampling every NSRT of the other RoboDisco environments the same way found no other mismatch (Bridge's and Domino's NSRTs need task-specific objects that this check does not build).
The fix.
With the skill-factory options, both factories now use the samplers their processes already use, as Bridge's NSRTs do; the legacy options and the 2D
coffeeenv keep theirs.tests/ground_truth_models/test_nsrt_samplers_fit_options.pygrounds every NSRT of both environments and checks the sampled parameters against the option's parameter space.Grow's
oraclenow runs but times out refining its plan, so Grow still solves only withoracle_process_planningand its README row is unchanged.The module docstrings of Grow's ground-truth models no longer call it the coffee environment.
ParameterizedOption.groundaccepting wrong-shaped parameters is left for a separate change.Test plan
pybullet_grow:oracleruns to completion (0/1, timeout; it crashed before) andoracle_process_planningstill solves (1/1).Stacked on #200.
🤖 Generated with Claude Code