Skip to content

A segment with no cascade in it scores nothing, not penalties - #143

Open
amburger66 wants to merge 7 commits into
masterfrom
domino-skip-empty-segments
Open

A segment with no cascade in it scores nothing, not penalties#143
amburger66 wants to merge 7 commits into
masterfrom
domino-skip-empty-segments

Conversation

@amburger66

@amburger66 amburger66 commented Aug 19, 2026

Copy link
Copy Markdown

Rest-point segmentation splits an episode into several independently scored trajectories, while the track covers the whole episode. A segment that is pick-and-place therefore has no onsets to offer, and every observed interval reads as a cascade the sim failed to reproduce — one missing-cascade penalty each, worth the track's entire duration.

The evidence

run_20260819_104757 produced the best data this experiment has yet managed: a clean capture (all dominoes 1.1–4.5°), all five toppling in both the twin and the real row, and id matching complete at 5/5 with a correct non-identity permutation.

Its sweep still reported every physical parameter as flat, and the agent declined.

The reported SSEs were 1.791e+05, 1.877e+05, 1.962e+05, 2.047e+05. One penalty term is summary_weight × duration² = 5 × 41.30² = 8528.4, so those are:

21.00   22.01   23.01   24.00

Integers. The objective was made entirely of penalties and contained no measurement. Five of the six segments were pick-and-place contributing ~4 penalties each, and being near-constant in θ they flattened the sweep.

The change

A segment contributes nothing when neither side has a cascade in it.

Why both, and not just the rollout

Requiring both is the whole subtlety, and the naive version is actively harmful. A θ that stalls the cascade also produces no onsets — so its segment would be skipped and score zero, making a friction that breaks the chain look better than one that reproduces it. That is the exact inversion interval_residuals penalises a one-sided domino to prevent.

It isn't hypothetical: the rollout-only version broke a landed test on the first attempt — test_the_objective_prefers_the_cascade_that_matches_the_track, asserting 0 < 0, the property the whole step exists for.

Gating on the recorded states alone fails in the other direction, because that same test hands in two identical states as its recorded trajectory while the rollout carries the cascade being scored.

Verified on the real run

Not only on fixtures. Reproducing that run's own truncation and segmentation and scoring through the real objective:

lateral_friction SSE penalty-equivalents
0.05 25589.6 3.00
0.1 25589.6 3.00
0.3 25589.6 3.00
0.5 8530.3 1.00
0.8 8530.2 1.00
1.0 25589.4 3.00

21–24 penalties → 1–3, and the first discrimination this sweep has ever shown: 0.5–0.8 comes back 3× better than the rest.

Not sufficient on its own

The numbers say so plainly: 1.00 and 3.00 are still exact integers, so what remains is still pure penalty — just twenty fewer of them. A healthy objective would show fractional values, with real interval differences contributing.

Those remaining penalties come from the cascade segment itself, where sim and observed still measure from different origin dominoes: the gripper occludes the pushed domino exactly while it falls, so the track cannot report its onset, and propagation_intervals drops whichever falls first. Fixing that needs a separate interval-exclusion change, deliberately not in this PR.

Tests

Two, and the second pins the trap rather than the feature:

  • a no-cascade segment contributes nothing
  • a stalled θ in a real cascade segment is still penalised

Reverting to the rollout-only skip fails the second. tests/code_sim_learning 163 passed, mypy clean, pylint 10.00/10.


Second commit: a tie at the earliest onset must not delete both dominoes

bee9a3a. This supersedes the "Not sufficient on its own" section above, whose diagnosis was wrong.

That section said the remaining 1–3 penalties came from the pushed domino's reference mismatch. Checked: they did not. Both streams agree the reference is domino_3. The real cause was propagation_intervals dropping every entry at the earliest time rather than dropping the origin.

They tie routinely. The sim samples one state per action, 83.3 ms; the track runs at 60 fps. So the pushed domino and the one beside it land on the same sim step and are 5× resolvable on camera:

domino_3 domino_4
twin 0.0833 s 0.0833 s — tied
camera 23.902 s 24.252 s — 350 ms apart

Both were dropped from the twin's list; the camera kept domino_4; it had no counterpart and drew the full missing-cascade penalty — reporting that the twin's chain never reached a domino it had toppled cleanly to 90°.

Nothing is dropped now. The origin contributes a zero, which carries no information while both streams agree on which fell first, and everything when they do not. Dropping exactly one would require the streams to agree on which, and neither can see the other from inside that function.

Effect, at the sweep's best candidate

before after
penalties 1 0
SSE 8530.1 1.0524
spread across the six candidates 2.99990 16211

1.0524 is the first fractional SSE this experiment has produced — a measurement in seconds rather than a count of dominoes that failed to pair up.

What it still does not do

It does not get friction declared. The verdict moves from flat across the range - this data cannot constrain it to weak evidence, because the rule that recommends declaring compares the best candidate against the baseline, and the baseline (1.191) is already close to the best (1.052).

So sharp identifiability with an already-correct baseline reads identically to data too thin to say anything. That's a gap in the verdict rule rather than in this PR, and worth raising on its own.

Rest-point segmentation splits an episode into several independently scored
trajectories, while the track covers the whole episode. A segment that is
pick-and-place therefore has no onsets to offer, and every observed interval
reads as a cascade the sim failed to reproduce -- one missing-cascade penalty
each, the track's entire duration.

Measured on run_20260819_104757, whose data was otherwise the best this
experiment has produced: clean capture, all five dominoes toppling in both the
twin and the real row, id matching complete at 5/5 with a correct non-identity
permutation. Its sweep still reported every physical parameter as flat and the
agent declined. The reported SSEs were 1.791e+05, 1.877e+05, 1.962e+05 and
2.047e+05 -- which are 21.00, 22.01, 23.01 and 24.00 times one penalty term
(summary_weight * duration^2 = 5 * 41.30^2). Integers, so the objective was
made ENTIRELY of penalties and contained no measurement. Five of its six
segments were pick-and-place contributing about four penalties each, and being
near-constant in theta they flattened the sweep.

So a segment contributes nothing when NEITHER side has a cascade in it.

Requiring BOTH is the whole subtlety, and I got it wrong first. Skipping on the
rollout alone is what the request literally asked for and is actively harmful: a
theta that STALLS the cascade also produces no onsets, so its segment would be
skipped and score zero, making a friction that breaks the chain look BETTER than
one that reproduces it. That is the exact inversion interval_residuals penalises
a one-sided domino to prevent. It also broke a landed test on the first attempt
-- test_the_objective_prefers_the_cascade_that_matches_the_track, asserting
0 < 0, the property the whole step exists for. Gating on the recorded states
alone fails in the other direction, because that same test hands in two
identical states as its recorded trajectory while the rollout carries the
cascade being scored.

VERIFIED ON THE REAL RUN, not only on fixtures. Reproducing that run's own
truncation and segmentation and scoring through the real objective takes it from
21-24 penalties to 1-3, and produces the first discrimination this sweep has
ever shown: 0.5 and 0.8 come back 3x better than 0.05, 0.1, 0.3 and 1.0.

NOT SUFFICIENT ON ITS OWN, and the numbers say so plainly: 1.00 and 3.00 are
still exact integers, so what remains is still pure penalty. Those come from the
cascade segment itself, where the sim and the observed sides still measure from
different origin dominoes -- the gripper occludes the pushed domino exactly
while it falls, so the track cannot report its onset. Fixing that needs the
separate interval-exclusion change, which is deliberately not in this commit.

Two tests, and the second pins the trap rather than the feature: a no-cascade
segment contributes nothing, and a stalled theta in a real cascade segment is
still penalised. Reverting to the rollout-only skip fails the second.
@amburger66 amburger66 self-assigned this Aug 19, 2026
propagation_intervals dropped every entry at the earliest time, which is a
different thing from dropping the origin. They tie routinely: the sim samples
one state per action, 83.3 ms, while the track runs at 60 fps, so the pushed
domino and the one beside it land on the SAME sim step and are 5x resolvable on
camera.

Measured on run_20260819_104757, where the twin toppled all five dominoes
cleanly to 90 deg. domino_3 and domino_4 both came back at 0.0833 s and both
were dropped. The camera had them 350 ms apart -- 23.902 s and 24.252 s -- and
kept domino_4, which then had no counterpart and drew the full missing-cascade
penalty. The objective was reporting that the twin's chain never reached a
domino it had in fact laid flat.

So nothing is dropped now. The origin contributes a zero, which carries no
information while both streams agree on which domino fell first, and everything
when they do not.

Dropping exactly ONE would need the two streams to agree on WHICH, and neither
can see the other from inside this function. A local tie-break on id picks the
wrong one as easily as the right one, and picking wrong reproduces the identical
false penalty on the other domino. Keeping every entry needs no agreement, and
where both streams do agree it costs one residual that is identically zero.

Effect on that run, at the sweep's best candidate (lateral_friction 0.6931):
penalties 1 -> 0, SSE 8530.1 -> 1.0524. That is the first fractional SSE this
experiment has produced -- a measurement in seconds rather than a count of
dominoes that failed to pair up. Across the agent's own six sweep candidates the
spread went from 2.99990 to 16211, against a 3.0 consistency bar.

It does NOT by itself get friction declared: the verdict moves from "flat across
the range - this data cannot constrain it" to "weak evidence", because the rule
that recommends declaring compares the best candidate against the BASELINE, and
the baseline (1.191) is already close to the best (1.052). Sharp identifiability
and an already-correct baseline read the same as thin data, which is worth
raising separately.

Three tests, all failing under a revert: the origin is kept at zero, a tie keeps
both dominoes, and two streams that disagree about which fell first still
produce real residuals rather than two penalties.
_TRACK_CACHE was keyed on config.track_path while storing what
_track_in_world_frame returned. The key is the file; the contents depended on
the frame transform. So whichever caller loaded first fixed the frame for every
later caller in the process, and one load with the transform unset left every
subsequent evaluation matching base-frame track positions against world-frame
twin states.

That does not raise. It silently degrades the id matching, which is the one
failure mode this whole path is least able to notice. I introduced it when the
transform was added: putting it inside the cached path was the mistake.

On run_20260819_133802 the effect was total. The sweep reported the first
"strong evidence FOR declaring" this experiment has produced -- lateral_friction
93672x better at 0.6931 -- the agent declared it, and sim.fit() then ran for the
first time and reported SSE exactly 0 at every candidate, concluding
"rollouts do not respond to ['lateral_friction'] anywhere in their boxes".
Friction stayed at the registry anchor.

The chain: 99 evaluations matched 2 of 5 dominoes; with three unmatched, a
segment cannot reach the two onsets an interval needs; the skip added in
fbc9659 then returned nothing, silently; and a fit handed nothing but zeros
called the parameter insensitive. Reconstructing the same segmentation from the
persisted trajectory matches 5 of 5 and gives SSEs from 8399 to 0.179, which is
what said the run's track object, not its data, was wrong.

Verified against that run's own data: with a poisoning load first, matching is
5/5 on both segments where the run got 2/5.

Second fix, in the skip itself. A skip means "no cascade here", and that reading
depends on having named every domino. With a partial mapping the same emptiness
can equally mean "the dominoes that fell are the ones I could not identify", so
an incomplete mapping now says so instead of returning a silent zero. Silence
stays only where the mapping is whole. This does not change any score; it stops
a measurement that never happened from being read as evidence that the
parameters do not matter.

Two tests. The cache one loads with no transform, then with the quarter turn,
and asserts the second caller is not handed the first caller's frame -- it fails
with the transform moved back inside the cache. The other pins that a WHOLE
mapping with no cascade stays silent, so the new warning cannot creep onto the
legitimate skip.

The shared _cascade_track fixture deliberately still carries no centres:
_cascade_states puts every domino at the origin, so centres there would break
the positional matching the surrounding tests depend on. The cache test builds
its own positioned track.
@amburger66
amburger66 marked this pull request as ready for review August 19, 2026 19:25

@yichao-liang yichao-liang left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review (Claude Code): 10 inline findings, ranked most-severe first in each comment's header tag. 7 correctness, 3 cleanup. The common thread in the top findings: the new skip gate consults the simulated and recorded onsets but never the camera track itself, which reintroduces the stall-scores-better inversion in a few forms.

# recorded trajectory (test_the_objective_prefers_the_cascade_that_matches
# _the_track passes two identical states) while the rollout carries the
# cascade being scored.
if (len(_onsets(sim_series)) < 2 and

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[correctness / severity 1] The no-cascade skip gates on sim and recorded onsets but never on the track's own onsets. When the recorded baseline itself stalls the cascade, a stalling candidate theta scores 0 while a theta that reproduces the camera's real cascade scores > 0 - the exact objective inversion this gate's comment says it prevents.

The recorded states are the twin's own open-loop simulation under the baseline theta. If that baseline friction stalls the chain after the push (< 2 onsets in the cascade segment), any candidate theta that also stalls hits both gate legs and the segment yields zero terms, while a candidate matching the track's 5-domino cascade proceeds to interval_residuals and accrues nonzero SSE. compute_rollout_sse then prefers the stalling theta over the correct one, silently - even though track.angles_deg, the one theta-independent witness that a cascade happened, is available two lines below the gate and is never consulted.

entry needs no agreement. Where both streams do agree on the origin
it costs one residual that is identically zero.
"""
if len(onsets) < 2:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[correctness / severity 2] Keeping the origin at 0.0 while retaining the len(onsets) < 2 -> return {} guard creates a new false missing-cascade penalty: when one stream detects exactly 1 onset and the other >= 2, the origin domino that BOTH streams agree fell now draws the full penalty (pre-PR it was absent from both sides).

Example: a candidate theta topples only the pushed domino. Sim onsets = {origin: t}, the < 2 guard collapses sim_intervals to {}, while obs_intervals now keeps all 5 entries including the origin at 0.0. interval_residuals over the union emits 5 penalties instead of the pre-PR 4 - one charged for the pushed domino both streams saw fall, the exact "chain never reached a domino it laid flat" false report this PR's docstring says it eliminates. It also makes "pushed domino fell" score identically to "nothing fell", and creates a 2-penalty jump between 1-onset and 2-onset thetas.

Consistent semantics: return {id: 0.0} for a single onset (guard on not onsets instead of len(onsets) < 2).

"is NOT evidence that the parameters do not matter -- the "
"objective could not measure them here.", len(name_to_id),
len(track.angles_deg))
return

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[correctness / severity 3] When every segment skips (e.g. partial matching), the objective returns exactly 0.0, flat in theta, with only a repeated log warning as a trace - and the identifiability pipeline confidently reports INSENSITIVE. That violates this module's own fail-loud policy, which the load-failure path enforces by falling back to per-step scoring.

The all-skip state is demonstrated by this PR's own comment (run_20260819_133802: 2-of-5 matching, 99 zero evaluations read as "insensitive to friction"). Nothing numeric detects it: compute_rollout_sse is a plain sum, identifiability's sensitivity screen sees d_sse = 0 and declares insensitivity, and solve_lm's empty-residual check returns the prior centre - precisely the outcome _load_scored_track's docstring ("scoring nothing would make every theta equally good... Failing loud is the whole point of the flag") forbids. The warning here only annotates the failure; a zero-interval-terms-across-all-trajectories condition should fall back to per-step scoring or raise, mirroring the load-failure path.

return

sim_intervals = propagation_intervals(_onsets(sim_series))
obs_intervals = propagation_intervals(_onsets(track.angles_deg))

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[correctness / severity 4] The skip only rescues all-or-nothing segments: any segment with >= 2 recorded onsets that is not THE cascade segment is still scored against the WHOLE episode's track intervals, drawing the same theta-flat integer penalty mass this PR diagnoses - and two gate-passing segments double-count the track's observed intervals.

Example: a cascade that pauses past segment_min_rest_steps is split by split_at_rest_points into two segments that each pass the gate and each get compared against all 5 track onsets - each drawing whole missing-cascade penalties for the dominoes in the other half. Likewise a gripper knock-over of 2 dominoes during pick-and-place. A bad candidate theta toppling 2 dominoes in a pick-and-place segment even makes the penalty mass appear/disappear discontinuously across thetas.

The deeper mechanism already half-exists: _episode_id_maps builds the concatenated whole episode. Computing onsets once per episode over it and yielding interval residuals once per episode removes both the special-case skip and the double count (preserving original step indices across the concatenation).

"scoring.", config.track_path)
return None
tracks = [_track_in_world_frame(t, config) for t in tracks]
_TRACK_CACHE[config.track_path] = tracks

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[correctness / severity 5] The rewritten cache contract ("What is cached is the FILE's contents, never anything derived from a config") is false: the cached value is load_tracks(path, config.track_fallback_fps, config.track_wait_s) keyed by path alone, so the first caller's fps fixes every later caller's timestamps.

For a track JSON without per-frame timestamps, load_track derives every sample time as index / fallback_fps and bakes it into the cached angles_deg; a later caller with a different track_fallback_fps gets the cache hit and silently inherits scaled intervals and a wrong duration_s penalty magnitude - the same first-loader-poisons-the-process class this PR just fixed for the frame, one field over. Independently of config divergence, load_tracks' track_wait_s/completeness filtering means a fit that runs while only k of N episode tracks are post-processed caches the truncated list forever (reset_track_cache is only called from tests).

Fix: cache the parsed frames and derive timing on the way out, or include fps/wait in the cache key.

# two onsets an interval needs, and the fit read the resulting zeros
# as "insensitive to friction" -- a decision made on data the
# objective had quietly declined to score.
if len(name_to_id) < len(track.angles_deg):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[correctness / severity 6] The partial-mapping guard len(name_to_id) < len(track.angles_deg) is a count comparison, wrong in both directions, and its test asserts the absence of a warning string no code emits, so the branch is effectively untested.

  • Suppressed when it should fire: when match_ids_by_xy fails, _map_for falls back to track_name_to_id, which maps every <prefix><n> object to id n regardless of the track's ids - with >= as many env dominoes as track detections the count check passes although nothing was positively identified (and mapped ids may not exist in track.angles_deg).
  • Falsely fires: a track with one spurious extra detection makes every legitimate no-cascade skip log the scary "objective could not measure" warning on every evaluation.
  • test_a_partial_mapping_does_not_score_zero_in_silence builds a full 4-vs-4 mapping and asserts "could not be matched" not in caplog.text - the actual warning says "could be matched", so the assertion is vacuous and a regression silencing the real 2-of-5 case would pass CI.

# _the_track passes two identical states) while the rollout carries the
# cascade being scored.
if (len(_onsets(sim_series)) < 2 and
len(_onsets(sim_topple_series(recorded, step_s, name_to_id))) < 2):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[correctness / severity 7 - plausible, not fully confirmed] The skip makes the residual-vector length flip between 0 and N as a function of theta at the 2-onset threshold. On the fit_map_lm_rollout -> solve_lm -> scipy.optimize.least_squares path, a finite-difference probe that crosses the threshold returns a different-length vector, and solve_lm's blanket except Exception silently degrades the whole MAP fit to its initial theta.

Under score_observed_only, interval terms are the ONLY residuals fed to least_squares (method='trf', FD Jacobian). A no-cascade recorded segment whose rollout sits near the 2-onset boundary in theta yields N terms at f0 and 0 terms at the perturbed point (or vice versa); the shape mismatch raises inside approx_derivative, is swallowed by the except ("LM fit raised ...; skipping"), and the fit returns the init point - a silent quality regression rather than a crash. Residual length was already weakly theta-dependent pre-PR via the union, but the guaranteed 0-vs-N cliff is new.

len(track.angles_deg))
return

sim_intervals = propagation_intervals(_onsets(sim_series))

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[efficiency] _onsets(sim_series) is computed twice per scored segment - once in the skip gate above and again in propagation_intervals(_onsets(sim_series)) - and the recorded-side series+onsets are rebuilt on every objective evaluation although recorded and name_to_id are theta-invariant across a sweep.

topple_onsets walks every domino's full angle series (O(states x dominoes) with a backdating inner loop), so every non-skipped segment pays onset detection twice per evaluation, ~100 times per sweep, and the recorded-side gate leg re-derives a per-trajectory constant precisely in the stall region the sweep explores most. Hoist sim_onsets = _onsets(sim_series) once and reuse it in both the gate and propagation_intervals; compute the recorded-side onset count once per trajectory alongside the similarly per-episode id_maps. The doubled call also invites divergence if one call site's detection arguments are later edited and the other is not.

if cached is not None:
return cached
# Transform on the way OUT, never on the way in: see _TRACK_CACHE.
return [_track_in_world_frame(t, config) for t in cached]

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[simplification] return [_track_in_world_frame(t, config) for t in ...] is duplicated verbatim on the cache-hit path (here) and the cache-miss path below. A single exit point (store into the cache on miss, then one shared transform-and-return) leaves exactly one place where the on-the-way-out invariant is enforced.

The invariant this PR introduces ("transform on the way OUT, never in") currently lives at two call sites that must stay identical. The next change to what happens on the way out - a filter, an extra config-derived derivation, per-config caching - applied to one branch only makes cache-hit and cache-miss return different objects: the same invisible first-loader-wins divergence this PR exists to fix, reintroduced on whichever path the editor did not test.

"a WHOLE mapping with no cascade is a legitimate silent skip"


def _cascade_track(tmp_path, onsets):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[test-coverage] The new _cascade_track helper is a verbatim re-implementation of the inline track builder already living in test_the_objective_prefers_the_cascade_that_matches_the_track (~lines 1016-1028): same 200-frame loop, same min(max((t - onset) / 8.0, 0.0), 1.0) * 90.0 ramp, same timestamp_ns: int(t * (1e9 / 60.0)). The older test should call the helper instead.

Two byte-identical fixture builders now coexist in one file (plus a fifth hand-rolled frame-schema literal in the new cache test, which _cascade_track could absorb via an optional centers parameter). A future change to the onset ramp, frame count, or fps edited in _cascade_track alone silently leaves the ordering test exercising a different track shape than the skip tests - the two properties this PR argues must be measured against the same track.

settled_xy_before_cascade fell back to states[0] when nothing toppled. I wrote
that fallback and called it reasonable. It is not: for a take that starts at the
push, states[0] is the PRE-PROLOGUE layout while the track shows the
POST-PROLOGUE one, so the dominoes the arm PLACES get compared against positions
they have not occupied since before the episode began.

The failure names itself. On run_20260819_152448 the log carried "could not
match 3 domino(s) ['domino_1', 'domino_2', 'domino_4']" 63 times -- exactly the
three placed dominoes -- while the two the arm never touches still matched. I
reproduced that 2-of-5, character for character, by truncating the cascade off
that run's own trajectory; with this change the same input yields no mapping at
all instead of a mangled one.

So the anchor returns {} when nothing topples. There is then no moment at which
the two streams are known to describe the same arrangement, and refusing is the
honest answer: the trajectory has no cascade to score anyway.

Second, track_name_to_id is no longer a consolation prize. It documents itself
as the fallback for a track carrying NO POSITIONS, and that is now the only case
it serves. Reaching for it when the twin merely could not be anchored is worse
than scoring nothing: the ids are box-drawing order, and on that run the true
mapping was a permutation (domino_3 -> id 4, domino_4 -> id 3), so a name match
would have attributed each domino's topple to a different one -- silently, and
with a full set of confident-looking residuals.

Verified end to end on that run's data, not on fixtures. The objective went from
SSE 0 at every candidate -- which the fit read as "rollouts do not respond to
lateral_friction anywhere in their boxes" -- to 3075.8 / 3075.8 / 3075.8 /
2.005 / 1.359 / 3075.6 across lateral_friction 0.05 to 1.0. The
penalty-equivalents are no longer whole numbers either (0.36), so even the
stalled region now carries real residuals rather than a pure count.

Two of my own earlier tests had been written around the fallback, using
trajectories with no cascade at all. They now carry one, which is what the
design actually requires. Two new tests, both failing under a revert: no cascade
yields no anchor, and a positioned track is not matched by name as a
consolation.
An all-digit serial written as "30264679" in a launcher config does not arrive
as a string. utils.string_to_python_object parses it as a NUMBER on the way in
from the command line, while the recorder reports its serials as strings -- so
the membership test rejected a camera that was in the list, and took a run down
at startup before anything had moved:

  ValueError: real_robot_snapshot_camera 30264679 is not one of the recorder's
  cameras ['32294776', '30264679']

The wanted serial is unquoted and the list is quoted. That is the whole clue,
and it is easy to read straight past.

Both sides are normalised to strings now. MarkerlessSnapshotPerception already
did str(serial) internally; the check simply ran before it.

Worth knowing that the trap is latent wherever a numeric-looking config value is
COMPARED rather than converted. real_robot_track_camera has the same shape and
works only because it is str()'d at every use.

One test, feeding the int form and asserting the camera resolves; it fails under
a revert to the raw comparison.
…nsistencies

Four findings from the #143 review, all of them mine.

THE TEST THAT PROVED NOTHING. test_a_partial_mapping_does_not_score_zero_in_
silence asserted `"could not be matched" not in caplog.text`. The warning it
guards says "could be matched". The assertion was therefore VACUOUSLY TRUE and
would have passed with the guard deleted -- a regression silencing the real
2-of-5 case, which is the exact failure that made a fit read "insensitive to
friction", would have gone through CI green. It now matches the string the code
actually logs.

That is the second time today a test of mine passed for the wrong reason. The
first was caught by mutating the source; this one only by someone reading it.

THE COMMENT THAT WAS FALSE. My cache comment claimed "what is cached is the
FILE's contents, never anything derived from a config". load_tracks takes
fallback_fps and wait_s, and for a track without per-frame timestamps every
sample time is index/fallback_fps -- baked straight into angles_deg. So the
first loader fixed the TIMEBASE for the whole process, the same
first-loader-poisons-everyone bug I had just fixed for the frame transform, one
field over. The key is now (path, fallback_fps, wait_s) and the comment says
what is true.

ONE ONSET IS AN ORIGIN, NOT NOTHING. propagation_intervals kept the origin at
0.0 for a cascade of two or more while collapsing a single-onset stream to {}.
The inconsistency costs a residual: one stream returns nothing while the other
keeps every entry including its origin, so a domino BOTH streams watched fall
has no counterpart and draws the missing-cascade penalty -- 5 where the
pre-origin-keeping code had 4. Guards on `not onsets` now.

AND ONE WASTED PASS. Onset detection ran twice over the same rollout series,
once for the skip gate and once for the intervals. Once now.

Two tests updated to the corrected contracts, one added: two callers asking for
different fallback_fps must not be handed each other's timebase, which fails
with the cache keyed on the path alone.

Still outstanding from that review, and deliberately not in this commit: the
per-episode restructure that the skip-gate, double-count and all-skip findings
converge on. It replaces scoring per segment with scoring once per episode, and
is a different shape of change from these.
Rest-point segmentation is a ROLLOUT device -- multiple shooting, to stop early
divergence compounding across a whole manipulation. It is not a statement about
how the evidence divides. The track covers the whole episode, so comparing
against it is inherently an episode-level operation, and doing it per segment
created three problems that each then needed a guard:

* the same observed intervals were compared once per segment, so a cascade
  watched by one camera counted as many times as the episode happened to be
  cut;
* segments with no cascade in them drew a missing-cascade penalty for every
  observed interval, which is what the skip gate in fbc9659 exists to stop --
  and that gate cannot consult the track, the only theta-independent witness,
  because the track spans the episode while a segment is a sub-range of it;
* when every segment skipped, the objective was exactly 0 and flat in theta,
  which the fit reads as "this parameter does not matter".
  run_20260819_163114 refused all five parameters that way, on data whose
  twin cascaded all five dominoes to 90 degrees.

Scoring once removes all three by construction rather than by guard. The skip
gate is gone, not patched.

Segments still roll out separately -- that is the point of them -- and are
concatenated with a per-segment time offset for detection. They are NOT one
continuous simulation, since each is re-anchored at rest with velocities
zeroed. Onset detection only needs each domino's fall to lie within one
segment, which holds comfortably (a cascade runs under a second; segments are
cut at quiescence), but a cascade straddling a boundary would be misreported
and the docstring says so.

Where one track pairs with one trajectory, that trajectory already IS an
episode and the old path is kept unchanged.

A no-cascade-on-either-side episode now warns and yields nothing, rather than
silently returning a flat zero. Nothing was measured, and the log says that
rather than letting a fit read it as evidence.

Verified on run_20260819_163114's own data. The objective produces real
residuals that vary with friction (1.04 to 1.34) where it previously produced
none. That run still cannot discriminate, but for a data reason rather than a
structural one: perception lost domino_0 mid-fall -- it was tracked to 45.8 deg
and then vanished for the last 20 s of a 55 s take -- so the track reports four
onsets against the twin's five, and the missing key draws one constant 15273.9
penalty at every theta that swamps the signal. On run_20260819_152448, where
all five were tracked, the same code gives 3075.8 / 2.005 / 1.359.

The first version of this had NO test that failed under a revert: the mutation
back to per-segment scoring passed all 170. The test added here counts residual
terms for one episode cut into one piece and into three, and asserts the count
does not change; it fails under that revert.
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.

2 participants