Skip to content

Let Domino's and Fan's oracles run in the default configuration - #200

Merged
yichao-liang merged 2 commits into
masterfrom
fix-domino-fan-oracle-default
Oct 3, 2026
Merged

yichao-liang merged 2 commits into
masterfrom
fix-domino-fan-oracle-default

Conversation

@yichao-liang

Copy link
Copy Markdown
Collaborator

Summary

Both oracle approaches crashed on Domino and Fan in the default configuration, for unrelated reasons.

Domino.
The 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 (DominoTiltingDelete) built Toppled(?d1) over a domino and failed LiftedAtom's type check at structs.py:880 (the type assertion on that line, not the arity check).
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.
oracle never crashed: its NSRTs come from the grid processes, and only the process planner adds the grid, so it reports the physical goal unreachable.
oracle_process_planning crashed for a different reason.
The default scene (the exposed deck since #196) attaches a FanTransferEvaluator to each task, and the 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 the evaluator, as replace_goal_with_alt_goal does; the planner never reads it.
The planner then runs, but its grid processes cannot reach the goal cell.

The envs README now lists Domino and Fan among the environments that run but do not solve the test task.

Test plan

  • The README command with --approach oracle and --approach oracle_process_planning on pybullet_domino and pybullet_fan: all four run to completion (0/1 solved); three of them crashed before.
  • The new tests fail on the old code and pass here: tests/ground_truth_models/test_process_derived_nsrts.py::test_domino_gt_processes_build_in_both_target_modes (both target modes) and tests/envs/test_pybullet_fan_transfer.py::test_grid_task_drops_the_evaluator_of_the_replaced_goal.
  • yapf 0.32.0, isort 5.10.1, docformatter 1.4, mypy 1.8.0 and pylint on the whole repo.
  • The full test suite on the top of this stack (Keep each PyBullet world's body ids on its own Objects #202).

Stacked on #199.

🤖 Generated with Claude Code

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.
@yichao-liang
yichao-liang changed the base branch from fix-robodisco-cross-env-leak to master October 3, 2026 07:59
@yichao-liang
yichao-liang merged commit e351d02 into master Oct 3, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant