Skip to content

feat(LTX25-PHASE-INSTRUMENT): the phase table charges its own cost and names the gap its residue is in (#1569, #1571) - #1706

Closed
localai-bot wants to merge 3 commits into
mainfrom
row/LTX25-PHASE-INSTRUMENT
Closed

feat(LTX25-PHASE-INSTRUMENT): the phase table charges its own cost and names the gap its residue is in (#1569, #1571)#1706
localai-bot wants to merge 3 commits into
mainfrom
row/LTX25-PHASE-INSTRUMENT

Conversation

@localai-bot

Copy link
Copy Markdown
Collaborator

unaccounted_seconds shipped as one number. Four issues — #1439, #1470, #1494 and #1536 — argued about whether the floor over it was the right tolerance, and none of them split it into the gaps between consecutive leaves. Splitting it is one pass over the table the render already writes, and it settles the argument by pointing at a region instead of at a constant.

On the first ABI render after this change, at wall 21.89 s on a box at load 130: a residue of 2.013 ms over 21 gaps, of which 0.944 ms — 47% — is charged to the instrument itself, and the largest remaining gap is load.dit -> load.video_vae at 0.627 ms. That is precisely the load.dit_config anchor #1668 still owes. The decomposition named the next un-named region on its first run, out of the emitted file, with no scratch script — which is the whole of what #1571 asked for.

What lands

Record::instrument_seconds and PhaseLog::Instrument(). Every interval the instrument spends inside its own entry points is charged to the innermost live non-span record, and to the table when none is live. Open stamps its start after taking the process-wide mutex and Close stamps its end before it flushes its progress line, so both intervals used to land outside every record and could not be told from a phase nobody named. Ported behaviour-identically from refs/pull/1556/head = b45ea3bbb, which measured, gate-ran and three-times-reviewed the mechanism. Item 4 of #1668.

WriteJson reads its clock before it serialises. Sum(records, Elapsed()) after ByStart(Records()) charged the writer's own copy and sort to the render's wall, and therefore to unaccounted_seconds. This table measures the render.

gaps in phase-log.json. One interval before each named leaf and one after the last, each carrying the two names it lies between, plus a gap_rule string saying what the array is.

The gap gate is arithmetic, not a measurement

Open marks a leaf nested whenever another leaf is already live, so the leaves Sum adds are non-overlapping, and ByStart orders them. Their complement inside [0, wall] is therefore exactly wall - sum_leaf_seconds. The gaps reconcile to unaccounted_seconds by construction rather than by tolerance, and no box load can move that verdict. Five of the ten mutations below are caught by a comparison with no clock in it.

That matters here more than it usually would. Every assertion this table has ever carried was a ratio of two wall-clock quantities, and two of them spent days being argued about across four issues.

#1569: an instrument whose own mutation could not fail

WriteJson's clock ordering was ungated, and a fresh review of #1556 restored the wrong order and watched the case that claimed to pin it stay green 10 of 10. The copy and the sort of a three-record table are nanoseconds, so the mutation's effect sat far below the slack in any bound that case carried.

It reds now. Over a 4000-record table the two orderings differ by exactly one whole copy and one whole stable_sort, and the case measures that copy and that sort in the same run, through the same public Records(), rather than quoting a constant from another box.

quantity min median p90 max
head / serialize, 50 consecutive runs at load 103-131 0.000716 0.002308 0.003115 0.004228
the same case with the ordering mutated 1.00383

0 red in 50. The bound is 0.5, so the worst honest run had 118x of margin and the defective value is 237x above the honest maximum. Every run's test cases: 1 | 1 passed | 0 failed line was recorded, because a -tc filter that matches nothing prints 0 cases ran and Status: SUCCESS!.

The constant is not a tolerance. Under the correct order the head holds zero copies and zero sorts; under the mutated order it holds exactly one of each and is therefore at least 1.0 * serialize by the definition of the two quantities, so any constant inside (0, 1) separates them. And the estimator is a minimum over probes because contention is one-sided: it strips the sporadic term from the honest side and cannot strip the deterministic term from the defective side. serialize > 1e-5 guards the other end — a table too cheap to serialise cannot separate the orderings at all, which is exactly why the three-record case was a mute switch.

What this does NOT do

instrument_seconds appears in no denominator anywhere. residue <= 2 * instrument was measured red 4 times in 45 runs at load 88 (max 4.115) and 28 times in 160 at load 125 (max 5.55) and withdrawn before this row started, because the un-instrumented remainder of a boundary dilates faster than the instrumented part when the box slows. It is emitted and reported so a reader can subtract it, and asserted against nowhere. .agents/specs/ltx25-phase-residue.md ## Design 3 is the evidence; it was not re-derived.

Neither existing floor is touched, in form or in constant. leaves >= 0.95 * wall and covered >= min_coverage * leaf_seconds are byte-identical.

#1668 stays open. It owns four items and this lands one. Items 1 to 3 — load.dit_config, artifacts.mux, denoise.update and Ltx2ConditioningTrace::sampler_updates — are one unit with the Carrying table in test_ltx2_video.cpp, and #1568 and #1570 both need denoise.update to exist before they can be closed. Marking #1668 done on the strength of item 4 alone would lose the other three, which is the failure it was filed to prevent. #1439 stays open too: a quantity a reader can see is not yet a budget a gate holds.

Mutations

Each prints its own compile_status and a sha256 pair, because a mutation that fails to build and a mutation that never applied both read as a passing test. Each restored the tree byte-for-byte — and the first restore mechanism FAILED that check, because a reverse text edit's anchor was no longer unique once applied (RenderText carries the same two lines WriteJson does), so the harness restores from a pristine byte copy instead.

mutation verdict
WriteJson reads its clock after the copy and sort, i.e. main's own code RED, 1.004 against 0.5
the decomposition drops the FIRST gap — the prologue, 92% of a real residue RED on the gap count
the decomposition counts NESTED records as leaves RED on all three: a negative gap, the identity, the count
the tail gap reported as zero, count and names untouched RED on the identity alone
each gap measured to the leaf's END rather than its START RED on the identity alone
every instrument interval charged to the TABLE, never to a leaf RED on 4 assertions across 2 cases
a SPAN absorbs the charge, so the residue's explanation vanishes into a number Sum skips RED on the span assertion
the per-record charge is not emitted RED on 2 assertions across 2 cases
the production emitter stops writing gaps, run against the RENDER case RED at REQUIRE(table.contains("gaps"))
ChargeLocked charges nothing anywhere, run against the RENDER case RED at REQUIRE(instrument > 0.0)

The last two are the reachability half, and both were run against ltx2 video: a render through the ABI emits a phase table that SUMS to wall, which enters through vllm_video_engine_load and vllm_video_generate. They prove a render reaches this code rather than that the class works.

One of those verdicts is itself a repair. The nested-records mutation first aborted the case at a fatal count REQUIRE placed above the loop, so the negative gap that same mutation produces was never observed and two of the case's three assertions were unproven while the case reddened. The count moved below the loop and became a CHECK.

Where the tests live

The instrument's own cases go in a new test_render_phase_log executable rather than into test_ltx2_video. Two of them need a table of thousands of records that no render produces; the model suite costs a fixture build; and #1470, #1536, #1572 and #1576 are being edited in that file right now by another agent. The reachability block stays in test_ltx2_video because only that case can prove a render reaches any of this, and it is the only thing this change adds to that file.

Baseline

main is RED on its own scheduled baseline at 019f66c1abuild-test-cpu, both sanitize-cpu arms and both windows-msvc-* — so inheritance is read by failure text rather than by job name. scripts/agent-preflight.sh on the base tree already failed test_cpu_x86_llamacpp_floor with waiting for quiet: 15s busy=122% ... load=67.25, which is the load-sensitivity failure that harness reports and not this change.

Closes #1569
Closes #1571

FOLLOWING_AGENTS_PROTOCOL

Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: AGENT:claude-opus-5 [claude-code]

mudler added 3 commits August 22, 2026 15:23
…nd the residue says which gap it is in (#1668, #1569, #1571)

`ltx25-phase-residue.md` records a body of work that was measured, gate-run and
reviewed three times and then closed unmerged. This row lands the part of it
that is about the INSTRUMENT: `Record::instrument_seconds`, `WriteJson` reading
its clock before it serialises, and the residue decomposed into the gaps between
adjacent leaves inside the emitted file.

The three driver anchors stay out. They are one unit with `denoise.update`,
which #1568 and #1570 both need before they can be closed, and bundling them
here would make one reviewer read two unrelated changes.

Two things this spec says NO to, and each is written down because each is the
obvious move. `instrument_seconds` never appears in a denominator: three fresh
reviews measured `residue <= 2 * instrument` red 4 times in 45 runs at load 88
and 28 times in 160 at load 125, because the un-instrumented remainder of a
boundary dilates faster than the instrumented part when the box slows. And
neither existing floor is widened, in form or in constant.

The one new bound is derived rather than tuned. Under the correct clock ordering
`WriteJson`'s wall is one function call behind the caller's; under the mutated
ordering it is a whole copy plus a whole `stable_sort` behind it, and the case
measures that copy and that sort in the same run rather than quoting a constant.
The estimator is a minimum over probes because contention is one-sided, which is
what separates this from the bound that was withdrawn.

FOLLOWING_AGENTS_PROTOCOL

Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: AGENT:claude-opus-5 [claude-code]
…d names the gap its residue is in (#1569, #1571)

`unaccounted_seconds` shipped as one number, and four issues argued about the
tolerance over it without anyone splitting it into the gaps between consecutive
leaves. Splitting it is one pass over the table the render already writes, and it
settles the argument: on the first ABI render after this change, 47% of a 2.013 ms
residue is charged to the instrument itself and the largest remaining gap is
`load.dit` -> `load.video_vae` at 0.627 ms -- which is the `load.dit_config`
anchor #1668 still owes, found from the emitted file with no script.

Three things land. `Record::instrument_seconds` and `PhaseLog::Instrument()`
charge every interval the instrument spends inside its own entry points to the
innermost live non-span record, and to the table when none is live, ported
behaviour-identically from `refs/pull/1556/head` = `b45ea3bbb`. `WriteJson` reads
its clock before it copies and sorts, so the writer stops charging its own
serialization to the render. And `gaps` decomposes the residue in the file, one
interval before each leaf and one after the last, each naming the two leaves it
lies between.

THE GAP GATE IS ARITHMETIC, NOT A MEASUREMENT, and that is the point. The leaves
`Sum` adds are non-overlapping, so their complement inside `[0, wall]` is exactly
`unaccounted_seconds`. The gaps reconcile to it by construction, which no box
load can move. Five of the ten mutations below are caught by a comparison with no
clock in it.

#1569's own mutation used to stay green 10 of 10, because the copy and sort of a
three-record table are nanoseconds. It now reds: over a 4000-record table the two
clock orderings differ by one whole copy and one whole sort, and the case
measures that copy and that sort in the same run rather than quoting a constant.
50 consecutive runs at load 103-131 read a max of 0.004228 against a bound of
0.5, and the mutation reads 1.004. Contention is one-sided, so a minimum over
probes strips the sporadic term from the honest side and cannot strip the
deterministic term from the defective side.

`instrument_seconds` appears in no denominator anywhere. `residue <= 2 *
instrument` was measured red 4 times in 45 runs and withdrawn before this row
started, and re-deriving a settled negative result is what the record exists to
prevent. Neither existing floor is touched, in form or constant.

The instrument's own cases go in a new `test_render_phase_log` executable: two of
them need thousands of records that no render produces, and three other issues
are editing `test_ltx2_video` right now. Reachability stays where only it can be
proved -- deleting the emitted `gaps`, and disabling charging entirely, each red
the ABI render case.

FOLLOWING_AGENTS_PROTOCOL

Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: AGENT:claude-opus-5 [claude-code]
Three commits on main touch docs and the Tenstorrent backend, and none of them
touches this row's files, so the running gate on this tree stays valid.

FOLLOWING_AGENTS_PROTOCOL

Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: AGENT:claude-opus-5 [claude-code]
@localai-bot

Copy link
Copy Markdown
Collaborator Author

Superseded by #1711, and the reason is a per-commit gate rather than a review finding.

This branch's implementation commit 87c46f097 changed include/vllm/multimodal/render_phase_log.h without touching docs/USAGE.md. check-doc-checkpoint.py walks the range COMMIT BY COMMIT, so a follow-up commit adding the document cannot clear it — the offending commit still lacks it — and clearing it in place would mean rewriting a pushed branch.

#1711 is rebuilt from origin/main with the same content in one implementation commit that carries the document with it. The code is byte-identical; the only addition is the docs/USAGE.md entry describing gaps and instrument_seconds.

Nothing here was wrong on the merits. plan, build and pr-size passed on this head, and both windows-msvc-* reds are the inherited #1649 /W4 /WX refusal — same failing step and same error text as main's own scheduled baseline at 019f66c1a.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants