Score the friction fit against the markerless pose track - #135
Merged
Conversation
Step 3, and the point of the previous two. Under open-loop execution nothing corrects the twin, so every recorded state is the twin's own PyBullet simulation -- a per-step SSE over them recovers the twin's friction by construction, which is exactly why the Stage-6 run could not learn one. The track the cameras produce is the only real evidence in the episode. code_sim_learning_rollout_score_observed_only (off by default) replaces the per-step residuals with propagation intervals: when each domino started to fall, relative to the first. The per-step loop is SKIPPED rather than added to -- keeping it would let thousands of twin-against-twin terms outvote a handful of real ones. Intervals rather than poses, for two independently measured reasons. Absolute base-frame position is 25-38 mm off while measured displacements are accurate to ~1 mm, so scoring poses would score the extrinsics. And an interval is invariant to however the track's clock is offset from the robot's -- which is what lets alignment be a detected event rather than a clock reading, and is why 3.2 of the plan needed no code at all. A test asserts that invariance. Onsets are confirmed and then backdated because two spurious-fall mechanisms have been measured on real takes and a naive threshold fires on both: gripper occlusion produced a confident 29 deg topple 15 frames early, and orientation drift wandered an untouched domino from 4.4 to 12.9 deg, across the 10 deg the twin calls toppled. Neither reaches an unambiguous fall, so a fall is believed only past 45 deg held for 3 samples -- the shape cascade_certificate._topple_ onset already uses -- and the onset is then walked back from there. Both traces are tests, including the two combined, where the artifact must not drag the true onset 15 frames early. Notably NOT a rate or jump gate: those were measured upstream and rejected, because during a real cascade the other dominoes translate 22-36 mm/frame, overlapping the 48-67 mm of the artifacts. Speed cannot separate them; visibility can, and the pipeline gates on it upstream. Track ids are MATCHED to objects, not assumed. The ids are the order the initialization boxes were drawn, which nothing makes agree with the env's numbering. Every twin-point/track-point pairing is treated as a candidate calibration offset and the one bringing the most dominoes within 40 mm wins. Centroid cancellation was tried first and is wrong: one bad detection drags the centroid and then NO pair matches, which a test pins. A cascade that stalls on one side is penalised at the track's full duration rather than skipped -- skipping would make a friction that stops the cascade early look BETTER than one that reproduces it, by having fewer terms. 3.1: code_sim_learning_rollout_scope_types narrows the scored scope, empty by default so the fidelity report is untouched. That report deliberately scores "everything that moves"; identifying ONE parameter is a different question, where the commanded arm reproduces at every candidate and can only dilute -- and with it in scope nothing ever rests, so rest-point segmentation can never cut. _moving_feature_scope is now module-level (it captured nothing from its closure) so the narrowing can be tested. Flag on with no track: one WARNING and per-step scoring. Scoring zero residuals would make every theta equally good and hand back the prior centre with a confident-looking identifiability report.
…it time The bridge between recording and scoring. Step 2 wrote .svo takes and Step 3 reads tracks; nothing turned one into the other, so the track path had to be set by hand and could only ever name one episode. real_robot_process_takes launches the markerless pipeline over each take as it closes, detached, and joins the outstanding jobs when the run ends. Detached because the pipeline runs about 3x the length of the take: inline it would serialise post-processing into the episode loop and undo open-loop batching. Launched per take rather than in one batch at the end because it parallelises across takes at ~2 GB of a 24 GB card, so the work overlaps the next episode's human scene reset instead of accumulating. A manifest names each episode's take, its track, and whether the take was usable. Rewritten in full as every take closes, so a run killed halfway leaves a valid document rather than a truncated one. An unusable take is recorded and marked, and deliberately NOT processed: a track fitted to a recording that lost a camera mid-episode is a well-formed track of the wrong thing. **The fit waits for the tracks the manifest promised**, up to code_sim_learning_track_wait_s (15 min). This is the part that makes the flag mean what it says. The manifest is written synchronously so it is always there, but the tracks it points at are minutes behind, and the online loop fits as soon as an episode ends -- so without the wait the fit would find nothing, warn, and fall back to per-step scoring, silently reinstating the defect all of this exists to remove. The distinction is that open-loop execution exists to stop the ROBOT waiting on perception; the learner has no such excuse, because the track is its data. The wait expires rather than hangs, and says the fit saw less evidence than the run recorded. Tracks are cached per path: a sweep evaluates the objective dozens of times, and without it each candidate friction would re-parse a multi-megabyte JSON and re-enter the wait. z-mode is "contact" for episode tracks as well as scene captures. The scored quantity is when each domino STARTS to fall, and at that moment it is still standing on the table, so the mode that constrains z there is the right one. A contract test pins run_markerless.sh's positional interface and the env vars passed to it -- a shell contract with no type checker behind it, where a rename would otherwise surface as a background job that fails silently and a track that never appears.
amburger66
force-pushed
the
domino-openloop-step3
branch
from
August 17, 2026 17:44
793797e to
9625f8b
Compare
amburger66
commented
Aug 17, 2026
Comment on lines
+1853
to
+1864
| # Restrict the scored feature scope to these object types; empty keeps | ||
| # every type, which is what the global-fidelity report wants. ["domino"] | ||
| # for the friction experiment: the arm is commanded, so it reproduces at | ||
| # every friction and can only dilute -- and with it in scope the episode | ||
| # never rests, so rest-point segmentation can never cut. | ||
| code_sim_learning_rollout_scope_types: List[str] = [] | ||
| # Features that cannot carry physics signal, dropped from the scored scope | ||
| # when scope_types is set. A colour channel does not move; a settle | ||
| # tolerance is not meaningful applied to a boolean. | ||
| code_sim_learning_rollout_nonkinematic_features: List[str] = [ | ||
| "r", "g", "b", "is_held" | ||
| ] |
Author
There was a problem hiding this comment.
not sure if this makes sense?
amburger66
marked this pull request as ready for review
August 17, 2026 18:36
… job Two things that stood between the post-processing path and actually being runnable. **Boxes, once per run rather than once per take.** Stage 2 needs one box per domino and is human-driven, which is the last thing gating unattended operation. real_robot_pick_boxes_at_start takes a 5-frame snapshot when the cameras open -- before any episode, while a human is still at the bench -- runs stages 1-2 with the drag window, and hands the boxes to the processor for every take afterwards. Previously the only option was producing a boxes.json out of band beforehand, which is a separate manual step nobody would remember. This is valid because a fixed-plan replay trains and tests on ONE arrangement, so boxes drawn on the scene as it stands are the right ones for every episode. And it is self-checking rather than merely assumed: if the layout later shifts far enough that a box no longer sits on its domino, stage 3's frame-0 identity check aborts that take instead of tracking the wrong object. A failed draw is not fatal -- the takes are still recorded for processing by hand. Skipped entirely when real_robot_snapshot_boxes_json names an earlier run's boxes, which is what makes a repeat run unattended. **A log per job.** The pipeline runs detached, so its output had nowhere to go and went to DEVNULL: a failed stage was a missing track and no reason, noticed minutes later when the fit found nothing. Each job now writes stdout and stderr to <bundle>/pipeline.log, and the failure message names that path rather than just an exit code. The log handle is attached to the process object rather than closed at launch: the child writes to it for minutes after launch returns, so letting it be collected would close the descriptor out from under a running stage. It is closed when the job is joined. Four tests. The logging one spawns a real script that writes to stderr and exits non-zero, rather than stubbing the launcher -- which would have tested nothing about the redirection. Full suite 1559 passed, mypy clean, pylint 10.00/10.
…cution Points exp_domino_real at everything the last three commits built, so the experiment exercises record -> post-process -> fit end to end with the fixed-plan explorer skipping straight to a real cascade. The changes and why each is forced: * open_loop_episode on, observe_at_option_boundary off. Mutually exclusive and asserted at construction -- a boundary look has to happen BETWEEN two options, and batching the episode leaves no such moment. * perception "zed" -> "scene_file". "zed" is the MARKER pipeline, whose 20mm tags do not resolve at this camera distance (1 of ~7 on one camera, 0 on the other), and it would also fight the recorder for the same cameras. The captured layout is what a fixed-plan replay wants anyway: the plan names specific objects and a rebuild could renumber them. * human_reset off, for the same reason. * record_episodes and process_takes on, with pick_boxes_at_start so the one human interaction happens at the start of the run. * score_observed_only on. Without it the two flags above make the fit WORSE rather than better: under open-loop nothing corrects the twin, so the recorded states are the twin's own simulation and a per-step SSE over them recovers the twin's friction by construction. Zero real observations instead of six. * scope_types ["domino"], dropping the commanded arm and the colour channels. * track_path at the run manifest, and a 900s wait for tracks still being post-processed. num_online_learning_cycles 2 -> 1 for the first integration run: with human_reset off there is no prompt between episodes, so a second cycle would start on whatever the first left behind. Raise it once the path works, at which point real_robot_snapshot_rebuild is what restores the between-episode prompt without reopening the camera conflict. Drops code_sim_learning_num_mcmc_steps: it does nothing for the rollout sysID. The emcee branch was removed in 2026-07 and the flag is now read only by fitting.py, so its comment here claimed an effect it has not had for a month.
run_20260817_160904 simulated all six of its options and shipped none of them.
The arm could not move at all under open-loop, in any configuration.
A fixed plan ends by raising OptionExecutionFailure("Option plan exhausted!"),
and that type is in the exploration loop's exceptions_to_break_on. The
end-of-episode verdict added for open-loop treated every break_on exception as
"this episode did not run to completion" -- so the normal terminus of every
fixed-plan episode was indistinguishable from an abort, and an executor that
defers its motion to the end of a completed episode discarded all of it:
Option plan exhausted after 6 options.
[CogMan] Finishing episode.
WARNING: real robot: dropping 6 buffered option(s) unshipped -- the
episode did not run to completion
The root cause is that one exception type carries two opposite meanings. So the
normal terminus is flagged where it is raised -- info={"plan_exhausted": True},
at both sites -- and only an unflagged break_on exception marks the episode
incomplete. Structural rather than matching on the message.
Why it took a hardware run to surface: with per-boundary shipping the motion
reached the arm DURING the episode, so the end-of-episode verdict decided
nothing. Open-loop is the first mode where that flag decides whether anything
moves, and no test covered a plan ending by exhaustion -- only by exception or
by step limit. The regression test does, and it reproduces the run: reverted,
it fails [False] == [True].
Also quiets zerorpc, the transport the arm is driven over. It logs a line per
channel at DEBUG, which at loglevel=DEBUG buries a hardware run's own output in
"--> new channel <uuid>". WARNING still surfaces a transport that is failing.
Three things the first hardware runs found.
Stage 2 died on int('id'). init_boxes.py WRITES records -- "id", "box",
"label" under a "boxes" key -- but the BOXES env it READS expects a bare list
of four-number lists. The records were handed over unchanged, so stage 2
iterated a dict and crashed minutes into run_20260817_162250, after the arm had
already executed the whole episode. The coordinates are unwrapped now, in list
order, which is the id order the writer enumerates. Verified against the
boxes.json that failed; both shapes are tested.
The take recorded the twin simulating. Under open-loop the arm does nothing
between the reset and the ship, so a take opened at the reset captures a static
scene -- 105 s of a 258 s take on run_20260817_165815. Trimming cannot recover
it: --trim-motion keeps everything between the first and last movement, and the
arm homing at the reset opens that window, leaving the still period in the
middle. The take is opened at the ship instead. Measured on the next run: 0.6 s
of dead air against 289 s of motion.
Per-boundary shipping still records from the reset, because there the motion is
spread through the episode. One existing test changed meaning rather than
behaviour and was rewritten to match: an aborted open-loop episode now leaves
nothing recording because it never STARTED a take, where before the stop was
what closed it. Same invariant; a companion test keeps the old assertion for
the per-boundary path.
The camera was whichever came first. real_robot_track_camera picks it, and
30264679 is the default: markerless is single-camera and the two are not
interchangeable -- on hand-measured ground truth this one is 6x better on
orientation (1.03 deg median against 6.29), which is what the topple onsets are
read off. Naming a camera the session does not record now raises up front
rather than failing per-episode with a missing file.
Also wires TRIM=1 (BabyRobotPredicator #78) so stage 1 drops the still lead-in
and tail. Worth having even though the take is now tight, and free: both of the
scan's failure modes keep frames rather than lose them. A driver that predates
the flag ignores it, so a contract test says which of the two is in front of us
instead of letting a silent no-op look like a working one.
Full suite 1572 passed, mypy clean over 539 files, pylint 10.00/10.
…odule Submodule b45ac97 -> 603e4d4, two PRs: #77 stops stage 4 deadlocking on fork, defaults -j 16, and makes NEURAL the default depth model. The deadlock is in the exact path the background post-processing drives, so this is the one that matters. NEURAL comes along with it -- predicators cannot select a depth mode, because run_markerless.sh forwards nothing to stage 1 for it, so the mode is whatever stage 1 defaults to. #78 adds --trim-motion, which the previous commit wires up. The experiment config follows the scene the bench is actually set up for: domino_row_20260817.json with four dominoes, capture id 3 as the green start and id 0 as the purple target. envs/all.yaml's 6 / 5 are domino_straight.json's ids and appear nowhere in this scene -- left in place the task has no target at all and _task_from_perceived asserts on it. The fixed plan follows the same scene. domino_straight_bridge2_scene_roles.txt picks domino_4 and domino_5 and pushes domino_1, none of which mean the same thing here: this scene has only domino_0..3, its movables are domino_1 and domino_2, and its start is domino_3. The sketches are checked in beside the config that names them, so the config does not reference a file that only exists on one bench.
The suite clobbered a live run's manifest. EpisodeRecorder's default track directory is the RELATIVE "logs/zed_tracks", and it rewrites tracks.json there whenever a take closes, so a test that constructs a recorder without passing track_dir writes into the repo -- and, if a run is in flight, over its manifest. That is not hypothetical. run_20260817_171402 recorded its episode, stage 4 produced a good track (60455 records over 15695 frames), and the fit then reported "episode 1 still has no track at logs/zed_tracks/take_20260817_174618 _train0_ep001/dominoes_traj.json" and fell back to per-step scoring. The take_ prefix is _StubSession.start_take's return value, the timestamp is when the suite ran, and the serial is the camera the run was not even using. The experiment lost its evidence to its own test suite. Isolating the working directory fixes every present and future test at once, where passing track_dir at each call site fixes only the ones someone remembers. Verified by checksumming the restored manifest across a full run of both affected suites: unchanged.
_persist_fit_trajectories sat inside the physical sysID fit, so the branch that never ran was the branch whose data mattered. A cycle where the agent DECLINES to fit is exactly the one worth a post-mortem, and it left nothing behind. run_20260817_171402 is the cost. Its sweep reported one identical SSE (5.418e+06) for every value of five different physical parameters, against a baseline of 39.37 with no override applied -- a five-order-of-magnitude discontinuity that appears the moment an override is applied at all, and does not vary with the value. Three of its five segments were already pinned at RMS 601, a saturation value rather than a measurement. The agent read that as "this data cannot constrain it" and declined, which was the right call on the evidence in front of it. Diagnosing WHY needs the trajectories, and they died with the process. So the dump now hangs off _learn_simulator, where a cycle's data arrives, whether or not anything is fitted afterwards. The label in the filename says which moment produced it: "recorded" for the data as it arrived, "fitted" for the existing post-fit dump, whose payload's identified params only mean something there. The test asserts the wiring as well as the function. Written the obvious way it passed with the call deleted -- it called _persist_fit_trajectories directly, which is a test of the dump and not of the thing that was broken. Calling _learn_simulator for real needs a whole synthesis session, so the wiring is pinned on its source instead; reverted, the test fails. Full suite 1576 passed, mypy clean, pylint 10.00/10.
Every Pick shipped close, open, close, and the hand visibly opened around the domino it had just taken. Grasp commands closed_fingers - 0.01 = 0.00000, deliberately past the block. The fingers STALL on it at 0.00658 -- #131 gave them a finite motor force so they rest at the faces instead of closing through -- and the carry phases that follow nudge from the ACHIEVED width, commanding 0.00658 - 0.001 = 0.00558. The splitter only ever sees commands, never achieved positions, so it compares that against the grasp COMMAND of 0.00000: a 5.58mm rise, which cleared the 5mm release epsilon by 0.58mm and became a spurious "open", followed by a re-"close" on the next step. Pre-existing; #131 exposed it by changing where the fingers come to rest. NOT caused by open-loop batching: the gripper dedup is session-wide state applied per segment, and a test added earlier proves the per-chunk segments are identical batched or shipped one at a time. This raises the epsilon to 0.008, which is a FITTED constant and is commented as one. A carried object measures 0.00558 on this bench and a genuine release measures 0.0122, so 0.008 is simply a value between them. It cannot be derived, and three attempts at a principled fix are why: * Requiring the release to leave the closed band broke test_split_actions_ignores_wobble_while_closing, which encodes a real release at 0.0122 against a closed of 0.02 -- inside the band. * Clamping the carry nudge to the tightest command so far was inert: the rebound crosses the Grasp -> carry phase boundary, and Grasp is a CHANGE_FINGERS phase that never runs that code. * Seeding that clamp from Grasp's command fixed the symptom but pinned the command at 0.00000 forever, so the fingers kept squeezing (achieved drifted 0.00485 -> 0.00447). That undoes what #131 built and broke test_oracle_process_planning_solves_bridge_task. The two cases are indistinguishable from the command stream -- both are "widen after a grasp", differing only in magnitude. The real fix is to stop inferring intent and carry the skill's own finger_status through on Action.extra_info (unused today, and documented for exactly this), leaving this constant to guard only actions that arrive without a stamp. Three tests pin what the constant was fitted to, so a retune of the grasp depth or the finger force names the two numbers to re-measure. Verified by rolling a Pick out in the twin: gripper commands go ['open','close','open'] -> ['open','close'], with the commanded widths unchanged.
yichao-liang
approved these changes
Aug 18, 2026
yichao-liang
left a comment
yichao-liang
left a comment
Collaborator
There was a problem hiding this comment.
Looks good, great work!
This was referenced Aug 18, 2026
Open
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
2 participants
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.
What this is
Step 3, and the point of the previous two: the friction fit finally scores against real measurements instead of the twin's own simulation.
Off by default. With
code_sim_learning_rollout_score_observed_onlyoff, behaviour is unchanged.Why
Under open-loop execution nothing corrects the twin, so every recorded state is the twin's own PyBullet simulation. A per-step SSE over them recovers the twin's friction by construction — which is why the Stage-6 run could not learn one, and why turning on Steps 1–2 without this would make the experiment worse (zero real observations instead of six).
The track the cameras produce is the only real evidence in an episode. This routes it into the objective, not the trajectory — the poses arrive minutes after the episode ends, so they can never be in the
obsstream.The residual: propagation intervals
Not poses, for two independently measured reasons:
Onset detection: confirm, then backdate
Two spurious-fall mechanisms are measured on real takes, and a naive threshold fires on both:
ToppledthresholdNeither reaches an unambiguous fall. So a fall is believed only past 45° held for 3 samples — the shape
cascade_certificate._topple_onsetalready uses — and the onset is walked back from there. Both traces are tests, including the two combined, where the artifact must not drag the true onset 15 frames early.Deliberately not a rate or jump gate. Those were measured upstream and rejected: during a real cascade the other dominoes translate 22–36 mm/frame, overlapping the 48–67 mm of the artifacts. Speed cannot separate them; visibility can, and the pipeline gates on it upstream.
Track ids are matched, not assumed
The ids are the order the initialization boxes were drawn — nothing makes that agree with the env's numbering. So every twin-point/track-point pairing is treated as a candidate calibration offset, and the one bringing the most dominoes within 40 mm wins.
Centroid cancellation was the first attempt and is wrong: one bad detection drags the centroid and then no pair matches. A test pins that case.
Automatic post-processing, and the wait
real_robot_process_takesruns the markerless pipeline over each take as it closes, detached, joined at run end. A manifest names each episode's take, its track, and whether the take was usable — rewritten in full as each take closes, so a killed run leaves a valid document.The fit then waits for the tracks the manifest promised (
code_sim_learning_track_wait_s, 15 min). This is what makes the flag mean what it says: the manifest is written synchronously, but its tracks are minutes behind, and the online loop fits as soon as an episode ends — so without the wait the fit would find nothing, warn, and fall back to per-step scoring, silently reinstating the defect all of this exists to remove.The distinction that makes it correct: open-loop exists to stop the robot waiting on perception. The learner has no such excuse — the track is its data. The arm still never waits.
§3.1 — the scored scope
code_sim_learning_rollout_scope_typesnarrows the scope, empty by default so the fidelity report is untouched. That report deliberately scores "everything that moves"; identifying one parameter is a different question, where the commanded arm reproduces at every candidate and can only dilute — and with it in scope nothing ever rests, so rest-point segmentation can never cut._moving_feature_scopeis now module-level (it captured nothing from its closure) so the narrowing is testable.Verification
run_markerless.sh's positional interface and env vars — a shell contract with no type checker behind it.No hardware validation. None of this has run against a real take.
Known limitations, stated rather than hidden