Conversation
| class. Whoever holds a scheduled batch reads the integers off it and | ||
| builds these rows. | ||
|
|
||
| There is no field and no constructor argument for a batch-level sum. The |
There was a problem hiding this comment.
The collapsed form is reachable three ways this sentence does not cover (module docstring, line 48: "cannot be handed in even by accident"). Non-blocking. Fix: __init_subclass__ guards on BatchView and RequestShape, as StepCost has (~10 lines), and soften this docstring and the PR body to what was measured.
The three routes, run on this branch
True, and worth keeping: no constructor path takes tokens/history in place of rows, and no supplied sum can disagree with the rows it came from.
- A one-row
BatchViewistokens x history.BatchView((RequestShape(512, 2048, False),))is legal and givesSum N_Q.N_KV = 512 x 2048 = 1048576, against524288for the two-row batch it collapses (RequestShape(256,1024)twice). They price differently, 284.389 us vs 280.194 us, astest_two_batches_that_collapse_alike_are_priced_apartshows, but that proves they are distinguishable, not that the wrong one is unreachable. Nothing tieslen(requests)to the number of requests the scheduler scheduled. - A subclass passes
estimate'sisinstancecheck and can override theprefillproperty to synthesise a collapsed row from fields it added. I built one; its price is bit-equal to (1).StepCostin this package refuses exactly this with__init_subclass__("an answer that does not come from the terms is the thing this class exists to prevent"); the projection does not inherit it. RequestShapethe same way: a subclass overridingcached_tokensfabricates the cross term directly.
What actually prevents collapsing is the projection step, one row per scheduled request, and that lives as _project in tests/compass/test_backend_shape_stub.py (lines 396-413), with no home in the package. #70 asked for the type to sit under backends/ so the scan reaches it; the builder that makes the type honest is still unowned.
There was a problem hiding this comment.
Round 2 pushed 28592b983: both halves applied. _refuse_shadowing refuses routes 2 and 3 at class creation, and also a subclass dropping __post_init__, where row integers and rung width are checked. The protected set is read off the owner class, as StepCost does, so a later reader is covered. The docstrings and PR body are softened; test_one_row_is_a_legal_batch_and_is_the_collapsed_form asserts route 1. Owning the projection stays the successor's task.
Refusals and what the docstrings say
route 2: BatchView subclass overriding prefill
REFUSED: TypeError: CollapsedView redefines prefill of BatchView; the shape sums are
functions of the rows, and a reader that answers from anything else is what summing
per request exists to prevent
route 3: RequestShape subclass overriding cached_tokens
REFUSED: TypeError: Fabricated redefines cached_tokens of RequestShape; ...
__post_init__ is the only non-public member whose replacement changes what the object reports: a subclass that drops it admits shapes the sums are not defined over. A subclass that adds a field and redefines nothing is still accepted and its sums still come from the rows, so the guard is not a ban on subclassing.
The module docstring, the class docstring, the test-file docstring and the PR body now say: every shape sum is a function of the rows, no constructor takes a sum in place of them, and no supplied sum can disagree with the rows it came from. Then what that does not close: one row is a legal batch, and a one-row batch is tokens x history (1 048 576 against 524 288, 284.389 µs against 280.194 µs, reproduced). The module says the projection, one row per scheduled request, is what prevents collapsing, and that its builder does not live in this package yet.
| """ | ||
|
|
||
| requests: tuple[RequestShape, ...] | ||
| capture_rung: int | None = None |
There was a problem hiding this comment.
capture_rung is a batch-level scalar field, so "no field and no constructor argument for a batch-level sum" (line 176) and the PR body's "no field ... accepts any batch-level scalar" are false as written. It is the right scalar to carry, but it is unbounded above and enters the price linearly. Non-blocking. Fix: say "the shape sums are functions of the rows", here and in the PR body.
Measured
The rung is a property of the replayed graph, not of the rows, and __post_init__ guards the two directions that matter (a rung with no decode rows, a rung narrower than the decode count).
BatchView((RequestShape(1, 100, True),), capture_rung=10**9)
-> graph_padding = 99999999900 -> decode.graph_padding = 99.9999999 s
One decode row, one hundred seconds. No caller would pass that; the point is the wording.
There was a problem hiding this comment.
Round 2 pushed 28592b983: fixed. The module docstring and PR body now say capture_rung is the right scalar (a property of the replayed graph), checked by __post_init__ in both directions against the decode rows, and unbounded above, entering the price linearly. Your capture_rung=10**9 figure reproduces on the new head and is in both. The structural claim everywhere is now "the shape sums are functions of the rows".
| name, | ||
| count * coefficient, | ||
| Provenance( | ||
| Species.FITTED, |
There was a problem hiding this comment.
FITTED is the right local call and still leaves a false aggregate: detail is dropped by both aggregating views, so 100% of this backend's seconds report as regressed over measured steps. Non-blocking, since nothing here consumes ProvenanceMix. Fix: register it in 12_open_items.md or a follow-up issue; adding DECLARED = "declared" to a closed vocabulary is the owner's call.
Measured
Accepted locally: ANALYTICAL is reserved by provenance.py, the landed package's own example is Provenance(Species.FITTED, "declared coefficients") in test_backend_interface.py, and test_the_species_is_not_analytical pins it. The disclosure in detail reaches rows() and describe(); checked against the real output.
step.seconds_by_species() -> {'fitted': 0.00014931072}
ProvenanceMix().record(step); .seconds_by_species() -> {'fitted': 0.00014931072}
FITTED means "a form chosen, coefficients regressed over measured steps", and nothing was regressed, with no decomposition carrying the correction. That breaks two design principles: never report an aggregate without its decomposition, and every claim carries its measurement (here a number with a false source). The backend emits only StepCost, verified. The fix is one enum member plus a sentence in provenance.py, but the design calls the vocabulary "closed and small". Left in a PR body, the next backend copies FITTED from here for the same reason this one did.
There was a problem hiding this comment.
Round 2 pushed 28592b983: registered as #87, "Species has no word for declared coefficients, so a stub that measured nothing reports as fitted", referenced from the PR body. No enum member here and no 12_open_items.md row: another PR is guarding that file's counts. FITTED stays, pinned by test_the_species_is_not_analytical. The aggregate is unchanged, and the PR body now states it in those terms.
Re-measured on 28592b9
step.seconds_by_species() -> {<Species.FITTED: 'fitted'>: 0.00014931072}
| whatever `StepCost` folds from them, so there is no second summation | ||
| to disagree with the first. | ||
| """ | ||
| if not isinstance(batch_view, BatchView): |
There was a problem hiding this comment.
isinstance rather than type(...) is lets route 2 of the line-176 comment through: a BatchView subclass overriding prefill is accepted here. The refusal message is good: it names the type and who builds the projection. Not a change request: an __init_subclass__ guard on BatchView closes it with no change on this line. Accepted with reservation: CostBackend.estimate names CostRefused for refusals, and this backend raises TypeError here and ValueError in BatchView.__post_init__; state it, because a Resolver catches only CostRefused.
There was a problem hiding this comment.
Round 2 pushed 28592b983: both remarks taken. The isinstance needs no change: BatchView now refuses the subclass at class creation (refusal text in the line-176 reply). The asymmetry is recorded in estimate's docstring, where a caller looks, and in the PR body's "left undone" list.
The docstring sentence
The two refusals below and in
BatchView.__post_init__areTypeErrorand
ValueErrorrather than theCostRefusedthe seam names, because a batch that is not
a batch is a caller defect and not a step this backend declines to price; the asymmetry
is worth stating because a resolver that wraps this backend catchesCostRefusedand
falls through to the next rung, and will not catch these.
|
APPROVE at Checked: the diff is the four files claimed; the named result reproduces, 0.35% included; the doubling ratios, Findings:
Accepted with reservation: the 0.35% is a constructed margin; the rung path has no integration coverage; Findings 3-5 in full3. The RPC path. 4. The doubling criterion. #70's exit criterion and D12's Option B bullet say "double the chunk size and the step time doubles". The form the same documents mandate is 5. The conflict. A history artifact, not a content disagreement: the parent's hunks arrive twice by two routes. Taking this branch's Checked, with measurementsDiff: Closed loop, with my own driver against ATOM's
Cross-term contribution over the five steps before the arrival: 749.1750 - 746.5536 = 2.6214 µs, 0.3499% of 749.1750 µs. The arrival sits inside [746.554, 749.175], so with the term the second request makes batch 6 and without it batch 7. Under constant pricing the break is step 4 at both chunk sizes. Doubling:
Ungating: The disclaimer reaches the output: Gates on node 18 (
+40 decomposes: 37 tests in No design-document identifiers, task labels or doc file names in either new file. Reservations and watch-next in fullAccepted with reservation:
Watch next:
Effort: my counts and my readingSame AST method (parse, strip module/class/function docstrings, unparse, count non-blank), plus physical:
Every physical figure in the PR body reproduces exactly; the AST figures to within a line. Production 0.67x by AST, 1.81x physical; the pair 1.89x by AST, 4.20x physical. My reading for the owner: the 200 is production physical non-blank, 362 = 1.81x, inside the halt line. (a) D12 sizes the options physically and in production only ("about 40 lines", "about twenty lines more than A"). (b) An AST count is invariant to the docstrings that are this house style's main cost, so it will never fire. (c) Counting tests against a production estimate makes the 2x rule punish test coverage. For the owner, since four tasks have now reported the same split: the physical-to-AST ratio here, 2.70x production and 1.95x test, is in line with the parent's 2.3x and the landed package's 2.03x, so the gap is the project's documentation style. #70's 200 is already 3.3x D12's sizing of the same option (~60 lines), so a 2x rule applied to it measures the estimator. Whichever unit is chosen, #70 should state it ("200 production LOC, physical, excluding tests"). Not checked
|
…m, and state what the type does not close Review round 2 on #81. The module claimed the collapsed form "cannot be handed in even by accident". Four routes to it were measured. Two are closed here and two are stated rather than claimed away. Closed at the type. `_refuse_shadowing` gives `BatchView` and `RequestShape` the `__init_subclass__` guard `StepCost` in this package already has: a subclass that redefines a reader can answer from a field it added rather than from the rows, and it passes the backend's `isinstance` check. Overriding `prefill` synthesises one row holding a whole batch's tokens against a whole batch's history, which is the collapsed form exactly; overriding `cached_tokens` fabricates the cross term directly. `__post_init__` is protected by name as the only non-public member whose replacement also changes what the object reports. Stated, not closed. A one-row `BatchView` is `tokens x history` by construction, and nothing in the type ties the row count to the number of requests a scheduler scheduled -- the projection does that, and the projection is not in this package yet. `capture_rung` is a batch-level scalar field, correctly so because a rung is a property of the replayed graph rather than of the rows, but it is unbounded above and enters the price linearly. The docstrings now say the property that is true -- every shape sum is a function of the rows -- rather than the stronger one that is not. The refusals here are `TypeError` and `ValueError` where the seam names `CostRefused`; `estimate` now records that a resolver wrapping this backend will not catch them. Design text, where a flag in a PR body would have been lost. D12's Option B bullet said "double the chunk size, the step time doubles", which the form the same section mandates forbids: measured, the intercept is 1.00x, the token term 2.00x, the quadratic 4.00x and the total 1.8836x, and the total doubles only for a = c = d = 0. The bullet now states the term-by-term property with those figures. The stale `scheduler.py:790-792` citation for the three detailed aggregate fields is corrected to `801-803` in all four design files that carry it, and D12's gate and method spans to `:2819-2820` and `:2788-2841`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
af97a2b to
f8ad2a0
Compare
…m, and state what the type does not close Review round 2 on #81. The module claimed the collapsed form "cannot be handed in even by accident". Four routes to it were measured. Two are closed here and two are stated rather than claimed away. Closed at the type. `_refuse_shadowing` gives `BatchView` and `RequestShape` the `__init_subclass__` guard `StepCost` in this package already has: a subclass that redefines a reader can answer from a field it added rather than from the rows, and it passes the backend's `isinstance` check. Overriding `prefill` synthesises one row holding a whole batch's tokens against a whole batch's history, which is the collapsed form exactly; overriding `cached_tokens` fabricates the cross term directly. `__post_init__` is protected by name as the only non-public member whose replacement also changes what the object reports. Stated, not closed. A one-row `BatchView` is `tokens x history` by construction, and nothing in the type ties the row count to the number of requests a scheduler scheduled -- the projection does that, and the projection is not in this package yet. `capture_rung` is a batch-level scalar field, correctly so because a rung is a property of the replayed graph rather than of the rows, but it is unbounded above and enters the price linearly. The docstrings now say the property that is true -- every shape sum is a function of the rows -- rather than the stronger one that is not. The refusals here are `TypeError` and `ValueError` where the seam names `CostRefused`; `estimate` now records that a resolver wrapping this backend will not catch them. Design text, where a flag in a PR body would have been lost. D12's Option B bullet said "double the chunk size, the step time doubles", which the form the same section mandates forbids: measured, the intercept is 1.00x, the token term 2.00x, the quadratic 4.00x and the total 1.8836x, and the total doubles only for a = c = d = 0. The bullet now states the term-by-term property with those figures. The stale `scheduler.py:790-792` citation for the three detailed aggregate fields is corrected to `801-803` in all four design files that carry it, and D12's gate and method spans to `:2819-2820` and `:2788-2841`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
f8ad2a0 to
28592b9
Compare
|
Round 2 pushed RestackThe head had moved past Both replayed clean; a plain Finding 1: the routes, re-run on 28592b9
The docstrings and the PR body claim only that the shape sums are functions of the rows, and say that a one-row batch is the collapsed form, that the row count is the projection's guarantee, and that Findings 2 and 4, and the stale citationsFinding 2: Finding 4: D12's Option B bullet now states the term-by-term property with the figures: intercept 1.00x, token term 2.00x, quadratic 4.00x, total 1.8836x, and the total doubles only for Stale citations: Gate and effort on 28592b9Node 18,
+45: 42 tests in
Under the round-1 reading (production physical non-blank) this is 2.12x, up from 1.81x. Round 2 added +63 physical non-blank to Not done: no enum member or |
| loop — step cost to queueing to a different batch — is what the whole design rests on, | ||
| and constants cannot exercise it. | ||
| - *Not* "the step time doubles". Measured against the form this section mandates, at | ||
| `b`=5e-7, `c`=2e-8 and a 256→512 single-request chunk: the intercept is 1.00x, the |
There was a problem hiding this comment.
Finding 1: this bullet states c=2e-8; the backend's c is 2e-11, and with 2e-8 the bullet's own 1.8836x becomes 3.7834x. a (2e-5) is omitted, and the total ratio depends on it. This amendment retires an unreproducible claim, and the milestone-2 analytical backend will read it. Fix: 2e-8 -> 2e-11, plus one clause naming a=2e-5.
Computed on 28592b9
shape.py Coefficients.prefill_query_square = 2.0e-11 <- the backend's c
c=2e-11 : short=149.3107 us long=281.2429 us ratio=1.8836x <- the figure this bullet quotes
c=2e-8 : short=1458.7200 us long=5518.8800 us ratio=3.7834x <- the figure this bullet's own c gives
The per-term ratios (1.00x / 2.00x / 4.00x) are independent of the coefficients and correct: prefill.step 1.000000, prefill.tokens 2.000000, prefill.query_square 4.000000, total 1.883608. Only the total is unreproducible from the stated inputs.
There was a problem hiding this comment.
Round 3 pushed 0fc02f458: fixed. This bullet now reads a=2e-5, b=5e-7, c=2e-11, names the chunk as a first chunk so d's absence is visible, and carries both absolute step times so the ratio reproduces from what is written. Your 3.7834x for c=2e-8 reproduces too (1458.72 µs -> 5518.88 µs). The per-term ratios are unchanged. #70's exit criterion states no coefficients, so it needed no change; checked.
The arithmetic now in the bullet
a=2e-5, b=5e-7, c=2e-11, first chunk 256 -> 512, single request
short = 2e-5 + 5e-7*256 + 2e-11*65536 = 149.31072 us
long = 2e-5 + 5e-7*512 + 2e-11*262144 = 281.24288 us
ratio = 1.883608
| correctly so, because a rung is a property of the replayed graph rather than | ||
| of the rows; it is checked against the decode rows in both directions but is | ||
| unbounded above and enters the price linearly, so a rung nobody would pass | ||
| prices a single decode row at any duration one likes. Subclassing is refused |
There was a problem hiding this comment.
Finding 2: "Subclassing is refused outright" is not what the guard does. Shadowing is refused; a BatchView subclass adding a field and redefining nothing is accepted, as your round-1 reply and _refuse_shadowing's docstring both say. The type moved and the prose overshot it again. Fix: "Shadowing a reader is refused outright", no net lines.
Measured on 28592b9
BatchView subclass overriding prefill -> REFUSED at class creation
RequestShape subclass overriding cached_tokens -> REFUSED
RequestShape subclass replacing __post_init__ -> REFUSED
BatchView subclass adding a field, redefining nothing -> ACCEPTED, sums still from the rows
There was a problem hiding this comment.
Round 3 pushed 0fc02f458: fixed. The sentence now reads: "Shadowing a reader is refused outright (see _refuse_shadowing); a subclass that adds a field and redefines nothing is accepted, and its sums still come from its rows." Pinned: test_a_subclass_that_redefines_nothing_is_accepted adds a label field to a frozen BatchView subclass, asserts Sum N_Q.N_KV still comes from its rows, and asserts it prices like the plain batch. The protected sets are unchanged.
| a reading from a field a subclass added; the row count is not closed here and | ||
| cannot be. What closes it is the projection -- one row per scheduled request, | ||
| built by whoever holds the scheduled batch -- and that builder does not live | ||
| in this package yet. |
There was a problem hiding this comment.
Finding 3: both unclosed routes (the row count and the rung) are named here and registered nowhere, and the successor named as their owner has landed: #80, #90 and #97 reference neither type. Round 1's standard for the species defect was "a registered open item or a follow-up issue", and #87 met it. Fix, no diff lines: open one issue naming the projection builder and its cross-check against Scheduler.compute_detailed_aggregates, and reference it as #87 is.
Measured on d175b03
git grep -n "BatchView\|RequestShape\|_project\|CostBackend" d175b03c6 -- atom/compass/runner/ -> no matches
No open issue names the projection builder. The next agent to build a projection will read the seam's signature, not this docstring. My answer to the review question is in the review record: for the row count, stating it is a close; what is missing is the schedule.
There was a problem hiding this comment.
Round 3 pushed 0fc02f458: registered as #124, carrying both unclosed routes, since whoever builds the projection reads the rows off the scheduled batch and the rung off the capture ladder. The gap reproduces at d175b03c6. One deviation to rule on: #124 is cited in the PR body, not in shape.py, as #87 is. Say so if the finding requires it in the source.
#124's exit criteria, and why the number stays out of the source
Exit criteria: one row per scheduled request over a batch built by ATOM's real Scheduler, plus a test that a two-request batch does not project to one row; the sums cross-checked against Scheduler.compute_detailed_aggregates (scheduler.py:2788-2841), ATOM's own method rather than a re-derivation; the rung from the ladder, None where no graph replayed. It also carries round-1 finding 3 as a criterion, so "assign the attribute, do not issue the RPC" outlives this PR body.
Why not in shape.py: AI_DEV_RULES bans design-doc identifiers in code and says the durable task record is the issue and its PR; a number in a docstring is the same habit in another vocabulary, and it would add lines to a diff already over the halt line.
| """ | ||
|
|
||
| requests: tuple[RequestShape, ...] | ||
| capture_rung: int | None = None |
There was a problem hiding this comment.
Finding 4: the third rung check, a rung far wider than the rows, is absent, the asymmetry is not argued, and nothing pins the statement, unlike route 1. The provenance's declared, not measured covers the coefficient, not the count; CANDIDATE exists here for a count less certain than its reading. Not blocking: the closed loop never sets capture_rung. Fix: close it, or argue it and pin it with a test; register it at minimum.
Measured on 28592b9
BatchView((RequestShape(1, 100, True),), capture_rung=10**9)
graph_padding = 99999999900
decode.graph_padding = 99.9999999 s
total = 100.0000223 s (one decode row)
__post_init__ refuses a rung with no decode rows and a rung narrower than the decode count; both check a rung against the rows. The open direction is the one that yields a fictional number rather than a negative one. grep capture_rung tests/compass/test_backend_shape_stub.py returns the two refusal tests, the field-list test and two pricing tests; nothing pins the unbounded-above behaviour, so bounding the rung later would leave this paragraph silently stale.
There was a problem hiding this comment.
Round 3 pushed 0fc02f458: argued and pinned, not closed. The module docstring now argues the asymmetry; two tests pin both directions; the padding term's provenance carries UNCHECKED_RUNG whenever a rung was supplied. The close, a rung read off the real capture ladder, is the second half of #124. Cost: +18 physical non-blank lines.
The argument, the tests, the qualifier
The argument: the two refused directions are refusable from the rows. A rung with no decode rows names a graph that replayed nothing; a rung narrower than the decode row count makes the padding negative. A rung far wider than the rows is not that kind of error: rows cannot contradict a width, only a ladder of captured widths can, and this backend has no ladder (the "no ladder" decision in the dev record). Refusing on a guessed bound would invent the ladder, the kind of decision this project treats as a defect. So it is priced at face value.
The tests:
test_a_rung_no_ladder_would_hold_is_priced_and_labelledassertsgraph_padding == 10**9 * 100 - 100,decode.graph_padding == 99.9999999 s, and the qualifier in the provenance.test_with_no_rung_the_padding_term_carries_no_rung_qualifierasserts the qualifier is absent with no rung, so it cannot become decoration on every batch.
Bounding the rung later now breaks a test.
The qualifier:
UNCHECKED_RUNG = "padding from a supplied rung width; nothing here bounds it above"
It is attached only when a rung was supplied: on a zero term it would claim a width was supplied when none was. CANDIDATE was not reused: its text says a collective may not run, a different uncertainty from a count nothing bounds. The +18: +9 in the docstring paragraph, +9 for the constant and the conditional term; nothing was trimmed to offset it.
| - `ScheduledBatch` fields — `scheduler.py:579-820`; notably `detailed_sqsq` / | ||
| `detailed_sqsk` / `detailed_sk` at `:790-792`, which are sum(N_Q^2), sum(N_Q * N_KV), | ||
| `detailed_sqsk` / `detailed_sk` at `:801-803`, which are sum(N_Q^2), sum(N_Q * N_KV), | ||
| sum(N_KV) per batch, computed by `compute_detailed_aggregates` (`:2788-2842`) and |
There was a problem hiding this comment.
Finding 5, low: after this PR three design documents cite compute_detailed_aggregates with two end lines. This line, edited here on its adjacent half, still reads :2788-2842; 02_model_runner_and_cost_backend.md now says :2788-2841, which is right. 09_fitting_and_law_selection.md is outside the file set. Fix: one character here.
Measured
def compute_detailed_aggregates is at scheduler.py:2788 and its last statement (scheduled_batch.detailed_sk = sk) at :2841; :801-803 and the gate at :2819-2820 also check out.
01_execution_and_time_model.md:1399 compute_detailed_aggregates (`:2788-2842`)
02_model_runner_and_cost_backend.md:468 compute_detailed_aggregates (`:2788-2841`)
09_fitting_and_law_selection.md:96 scheduler.py:2788-2842
There was a problem hiding this comment.
Round 3 pushed 0fc02f458: fixed here and in 09_fitting_and_law_selection.md, both now :2788-2841. def compute_detailed_aggregates is at 2788, its last statement at 2841, blank at 2842, def _connector_flag at 2843; the gate at :2819-2820 and the fields at :801-803 check out. 09 is outside the original file set: one character, and one of three documents left disagreeing is worse than the conflict risk. My call to take.
|
REQUEST CHANGES at Checked: the eight-file diff; the four routes, the control and the guard's full protected sets; the species aggregate; #87; the named result by closed arithmetic; the four inline replies against the tree; ruff; the gate, +45 at Findings, inline:
Round-1 finding 3 (the profile RPC) is not yet a two-cycle halt, because its handoff remedy was not due; if still open after round 3, the loop halts on it. Round-1 finding 3, still openThe RPC path Not treated as the two-cycle halt trigger, stated so it can be overruled: the remedy round 1 asked for was a handoff clause, and the handoff is a closing comment on #70, which is open. So the finding has not survived a cycle in which its remedy was due. It is due now: one sentence in the PR body or in #70's handoff. The review question: is stating an open hole in a docstring a close, or a guess wearing a disclaimer?For route 1 it is a close. Refusing rather than guessing forbids answering without a basis; it does not forbid accepting a legal input. A one-row What is missing is the schedule, not the honesty. A named gap becomes a close when it becomes work somebody can claim. Round 1 made that argument about the species word, it was accepted, and #87 is the result. It was not applied here: finding 3. For route 4 it is weaker, closer to a disclaimer. The type already refuses two of the three ways a rung can disagree with its rows, which concedes that a rung is checked against the rows; the third turns one decode row into any duration and alone is handled by a paragraph. No test measures it, and the provenance carries no qualifier although this file has Checked, with measurementsFile list from
The guard is complete, not a listed subset: #87 is open, unlabelled, and carries the measurement and three options. The bound on the harm: Doubling recomputed: Named result by closed arithmetic, independent of this PR's helpers: with The four inline replies land where they claim: the guards exist and refuse; the softening is in the module docstring, the class docstring and the body; #87 exists; the Accepted, and not to revisit in round 3
Drift and gate
Each tree gated with its own
+45 passed, 0 skipped, 0 xfail; skips and xfails identical, so the ±1 flake did not fire. 42 tests in EffortAST = parse, strip docstrings, unparse, count non-blank;
Both reproduce the PR body. Against #70's 300-450 envelope: production AST 145 below, production physical 425 inside, the pair by AST 426 inside, the pair physical 958 outside. Against #70's stated unit ("200 LOC, production, physical non-blank lines, excluding tests") production is 2.13x: a halt-and-discuss event. The developer flagged it rather than trimming prose. Not decided here: the instrument question is escalated to the owner on #89, which carries The round-2 growth was +63 physical non-blank on Watch next, and not checked
Not checked: the GPU tier (not required; scope, not a gap); the round-1 gate rows (the tip moved twice past them); accuracy (nothing here to be accurate about, and every row says so). |
… schedule that moves with it (M1-2) A stand-in cost backend whose price is a declared linear form over the shapes a scheduled batch already carries, replacing the alternative of two constants. Constants price every step the same, so nothing downstream can depend on the price and no run can show whether it did; the prior effort measured what that hides, at +47.9% time-to-first-token once shapes varied. The form is a launch intercept, a token term, a quadratic query term and a query-by-history cross term for prefill; an intercept, a request count, a context sum and the graph rung's padding rectangle for decode; plus one term per collective the deployment widths admit, charged as a candidate and saying so in its own provenance because two of the conditions that gate one are not widths. Constant pricing is retained as a coefficient set with every shape coefficient zero, which needs no branch in the estimator and leaves the zeros visible in the breakdown. Three properties are structural rather than asked for in prose. The attention sums are computed from one row per request and the projection has no field that could carry a collapsed scalar instead. The projection type lives in this package, so the scan that forbids engine imports covers it, and a test fails if it moves out. And every term's provenance, plus the backend's own description, says the coefficient was declared rather than measured and is not an accuracy claim -- in the record an artifact writes, not only in a docstring. The tests drive ATOM's own scheduler, chunked prefill and admission included, with a virtual clock advanced by nothing but these predictions, and check the batch it builds against what the form predicts. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…m, and state what the type does not close Review round 2 on #81. The module claimed the collapsed form "cannot be handed in even by accident". Four routes to it were measured. Two are closed here and two are stated rather than claimed away. Closed at the type. `_refuse_shadowing` gives `BatchView` and `RequestShape` the `__init_subclass__` guard `StepCost` in this package already has: a subclass that redefines a reader can answer from a field it added rather than from the rows, and it passes the backend's `isinstance` check. Overriding `prefill` synthesises one row holding a whole batch's tokens against a whole batch's history, which is the collapsed form exactly; overriding `cached_tokens` fabricates the cross term directly. `__post_init__` is protected by name as the only non-public member whose replacement also changes what the object reports. Stated, not closed. A one-row `BatchView` is `tokens x history` by construction, and nothing in the type ties the row count to the number of requests a scheduler scheduled -- the projection does that, and the projection is not in this package yet. `capture_rung` is a batch-level scalar field, correctly so because a rung is a property of the replayed graph rather than of the rows, but it is unbounded above and enters the price linearly. The docstrings now say the property that is true -- every shape sum is a function of the rows -- rather than the stronger one that is not. The refusals here are `TypeError` and `ValueError` where the seam names `CostRefused`; `estimate` now records that a resolver wrapping this backend will not catch them. Design text, where a flag in a PR body would have been lost. D12's Option B bullet said "double the chunk size, the step time doubles", which the form the same section mandates forbids: measured, the intercept is 1.00x, the token term 2.00x, the quadratic 4.00x and the total 1.8836x, and the total doubles only for a = c = d = 0. The bullet now states the term-by-term property with those figures. The stale `scheduler.py:790-792` citation for the three detailed aggregate fields is corrected to `801-803` in all four design files that carry it, and D12's gate and method spans to `:2819-2820` and `:2788-2841`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… not, and fix the coefficient a document quotes Round 3 of the review loop, against the five inline findings on the round-2 head plus the round-1 finding left open. The module docstring claimed subclassing was refused outright. It is not: shadowing a reader is, and a subclass that adds a field and redefines nothing is accepted with its sums still running over its rows. The sentence now says that, and a test asserts the accepted case so the corrected claim is pinned rather than restated. The rung asymmetry is argued rather than noted. Two ways a capture rung can disagree with the decode rows are refused because the rows settle them: a rung with no rows to pad, and a rung narrower than the rows it pads. A rung far wider than the rows is not refusable from the rows -- only a ladder of captured widths could refuse it, and this backend has none -- so it is priced at face value and the padding term's own provenance now says the width was supplied and nothing here bounds it. Two tests pin both directions, so bounding the rung later breaks a test instead of leaving the paragraph stale. The design text carried a coefficient three orders of magnitude from the backend's. D12's doubling bullet said c=2e-8, which gives 3.7834x, not the 1.8836x printed beside it; the backend's c is 2e-11, a=2e-5 was omitted although the total depends on it, and neither absolute step time was stated. All three fixed. The span of compute_detailed_aggregates was re-measured -- def at scheduler.py:2788, last statement at :2841 -- and the two documents still citing :2788-2842 now agree with the one that was corrected. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
28592b9 to
0fc02f4
Compare
|
Round 3 pushed A plain What moved
First a restack onto the integration head: the old base Then one new commit, |
|
Round 3 pushed Findings, and where each reply is
Round-1 finding 3 (the profile RPC): its remedy was a handoff clause, and #70's handoff is written on closure, which is why it survived two rounds without falling due. Closed now rather than deferred again, in three places: the PR body's dev record (the two ungating paths are not interchangeable), the comment above #123 not taken: the tensor-parallel collective is charged per paged layer ( Not done: not merged or undrafted; no RestackAs of Gate on 0fc02f4Node 18,
+48, by collection on each tree: Effort on 0fc02f4
Under #70's unit (production, physical non-blank, excluding tests): 362 (1.81x) -> 425 (2.12x) -> 443 (2.22x). More than ~2x is a halt-and-discuss event; reported, not trimmed. This round's +18 production physical non-blank, all from findings 2 and 4:
Finding 4's remedy was "argue it explicitly and pin it with a test"; an argument is prose. No comment or docstring was trimmed to offset it. By AST it is 0.73x, so the instrument decides the verdict: the open owner decision. |
| f"capture rung {self.capture_rung} is smaller than the " | ||
| f"{len(decode)} decode requests it pads, which would make the " | ||
| "padding negative" | ||
| ) |
There was a problem hiding this comment.
Note, not a finding: this refusal is right for a stronger reason than its message gives. A rung narrower than the decode rows need not make the padding negative; the real reason is that a graph captured at width 2 cannot replay three rows, the rows-settle-it argument the module docstring already uses. The check is unchanged across all three commits and accepted by two cycles, so it is recorded for #124, where the rung gets a real source.
Measured on 0fc02f4
rows contexts (1000, 1, 1), capture_rung=2
REFUSED: capture rung 2 is smaller than the 3 decode requests it pads,
which would make the padding negative
but rung*max - sum = 2*1000 - 1002 = 998, which is NOT negative
|
APPROVE at Checked: each finding re-measured; the restack replayed byte-identical; the gate, 4647 passed at The six findings, re-measuredRange pinned to head
Rulings on the two deviationsA. #124 not written into
B. Restack, gate, effortThe two original commits replayed without loss; Node 18,
+48, Effort: production AST 146, physical non-blank 443 (425 + 18); test AST 300, physical non-blank 572 (556 + 16). All four reproduce the PR body. 443 = 2.215x the 200; the series 362 -> 425 -> 443 reproduces; the +18 decomposes as the PR states. Production AST went 145 -> 146 across round 3: one statement line for eighteen physical ones (a four-tuple, one Notes, reservations, watch nextNote A: the round-3 comment prints Note B: #70's body, in the "ATOM already computes the attention terms" paragraph, still reads Note C, inline: the rung refusal is right for a stronger reason than its message gives. Unchanged by round 3 and accepted by two prior cycles, so not a finding; recorded for #124. Accepted with reservation:
Watch next:
Not checked: the GPU tier ( |
Restacked onto #81's round-3 head and amended for the six inline findings of round 1. The three assertions that pinned the collective's layer count as a literal no longer do. The count is recovered from the priced step -- the collective term is tokens x layers x a coefficient -- and the step seconds and the ratio are looked up in tables keyed on it, carrying both answers: 1.030496x charged per paged layer as it is today, 1.121986x charged on the depth an all-reduce actually runs on. The second row is priced rather than worked out in a comment: a uniform 64-layer stand-in charges what the published hybrid would if the charge moved, and the same read-back recovers 64 from it. A count that is neither is refused by name. So when the charge moves the assertions move with it instead of going red with nothing in them to say why. The seam test's docstring claimed a route it does not close. Measured: a config type defined beside the engine reaches FakeModel through its duck-typed config argument and neither the import scan nor this test fires. The docstring now claims only what the test does -- these types are defined in files the scan reads, and the case only this test catches is a move to a package that is not atom.* at all, which the scan skips by its own filter -- and names the duck-typed route as open, in the module docstring as well. head_dim now defaults to unset, which reaches the reader's one fallback and makes hidden_size decide something: unset, the block is hidden_size over the query head count, and 4096 and 8192 no longer produce an equal geometry. A dial that is not a whole number is refused by name rather than by a bare comparison error, and an unset head_dim over too few hidden units is refused by name too. stages() checks that the spans partition the stack, not only that there are as many as there are stages. Four identical spans, four spans that tile a quarter of the model, and a gap between two spans were all accepted; each of them sizes pools that hold a fraction of the KV with nothing saying so. Sorted, the spans must start at 0, meet end to start, and end at the declared depth. The layer-kind spellings are one declaration again: the dial reads them from the module that decides what they mean rather than repeating two string literals across a module boundary. geometry.py is a landed PR's file and is not touched. Decode-per-rung is filed as #128 rather than left in a PR body a squash erases, and #24 now carries a named result. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
No blocking issues: APPROVE at Checked: the published counts reproduce (the two files 58 passed; full gate 4647 passed, 149 skipped, 3 xfailed, rc 0). Both fixes made under review have pins that fail when the real pre-fix code is put back. Of 37 mutants that introduce a defect, 31 bite. Five leave the whole gate green, and a pre-existing sibling test catches the sixth. State and staleness
Integration is now MethodBranch tree staged into Harness conditions, and what each did:
Full gate on the unmutated tree: 4647 passed, 149 skipped, 3 xfailed, Denominators: 58 collected = 45 in A. The two fixes made under review, pre-fix state recovered from this PR's commitsRecovered with
Published count 58 passed in every row. Two whole-blob reverts, B. Every other pinned behaviour, one mutant at a time42 mutants; published 58 passed / rc 0 for all.
Of the 42 mutants, 37 introduce a defect (the other five are the three tripwires below and the two null controls); 31 bite and 6 leave both files green. Over 43 mutants and 4 recovered pre-fix states (2 discarded as total failures), every pin this PR presents as guarding a specific behaviour bites, and none of the six blind rows is a pin this PR claims. C. What no test in the tree catches, and the one a sibling doesEach run through the full CPU gate on its own copy; control on the same tree and procedure: 4647 passed,
Notes on reach, not findings against the approved change:
D. Assertions that read the same expression on both sides; E. tripwires; F. proseBy AST over both files, with local single-assignments inlined three levels, checked against what 43 mutants did:
Tripwires (
No refusal branch in this PR's file is unreached. Prose: nothing in the tree reads a docstring ( |
|
Blocked on #89: Under The label also holds every PR stacked above this one. No agent will land, amend or review this chain until the owner rules on #89 and removes the label. |
Base update under the need-human exception in AI_DEV_RULES.md: the branch conflicted with the integration tip in one file. tests/compass/test_backend_interface.py: both sides kept. The branch's reformatted import assertion and its projection-location test stay; the tip's test_every_module_the_walk_returns_is_a_case is appended after them. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
Blocked on #89; base update pushed Resolved file
The four design docs merged cleanly. The remerge diff ( Dropped-change check
Gate
Node-id delta, from junit XML:
GPU tier not required, per the |
|
No blocking issues: the conflict resolutions of merge Checked: the merge keeps every change from both sides and changes nothing else; the remerge diff is one hunk; both dropped-change directions are clean; the merged file behaves as both sides intended; the gate logs agree. 1. Remerge diff
The tip's three 2. Dropped-change check, both directions
PR direction: the two context differences the agent reported. In 3. The merged file, run and mutated
Unmutated: 17 collected, 17 passed, rc 0, on both of two runs.
4. Gate, read from the logsThe node-18 staging dir is gone; read from
Both show Evidence: Agent-authored delta review under the base-update exception. No push, merge, label or draft change. |
Part of #70; closes nothing yet. Blocked on #89 (effort overrun):
need humanapplied (comment). Draft; do not merge or undraft.Head
58cb9dcce. APPROVE covers0fc02f458(review); the base-update merge's resolutions were reviewed sound (review). No blocking issues in the code.A cost backend,
atom/compass/backends/shape.py, whose price is a declared linear form over shapes the scheduled batch already carries, so the schedule moves with the batch shape. Constant pricing stays as a coefficient set, not the default.What the backend is
shape.py, four names on the package's exports, and one new assertion in the seam's own test file. The form above, plus one term per collective the widths admit. A mixed step is priced as both groups and pays both intercepts. Constant pricing isCoefficients.constant(...), every shape coefficient zero: no branch inestimate, and the zeros stay visible in the breakdown rather than implied by a flag.Nothing here is an accuracy claim, and the output says so. Every term's provenance reads
5e-07 s x 128; declared, not measured -- a plumbing figure, not an accuracy claim, which is whatStepCost.rows()hands an artifact;describe()repeats it. A collective's term addsa collective these widths admit, not one observed to run.Dev record
scheduler.profile_active = True, and no ATOM edit. The profile RPC is not equivalent: it starts a real profiler.FITTEDwith the origin indetail(Species has no word for declared coefficients, so a stub that measured nothing reports as fitted #87 asks for a "declared" word); tierCOARSE; a rung on a prefill-only step is refused; no ladder; the refusals areTypeError/ValueError, notCostRefused.Dev record, in full
Ungating.
compute_detailed_aggregatesis already called on every step that has sequences (EngineCore._process_engine_step_inner); the gate is inside the method and reads two things.ATOM_ENABLE_DETAILED_ANNOTATIONis a real env flag (atom/utils/envs.py), cached intoScheduler._detailed_annotation_enabled.profile_activeis a public attribute,FalsefromScheduler.__init__, flipped only by_handle_start_profile/_handle_stop_profileinatom/model_engine/engine_utility.py; it has no env flag, so ungating means setting it from outside._handle_start_profilecallsrunner_mgr.call_func("start_profiler")before it flips the flag, andModelRunner.start_profilerstarts a realtorch.profilerand installs a trace-export callback wheneverprofiler_diris set. So assign the attribute; the RPC drags a profiler and its trace files into a run with no device. Recorded in the comment abovePUBLISHINGintest_backend_shape_stub.py, in exit criterion 2 of #124, and in #70's closing handoff when #70 closes.The cross term. ATOM's
sqskisSum N_Q.N_KV; the form wantsSum N_Q.N_KV_cached. On both branches the method usesN_KV = cached + N_Q(decode viaseq.num_tokens, the same quantity), soSum N_Q.N_KV_cached == sqsk - sqsqexactly: a difference of two sums ATOM already publishes, no fourth reading. Asserted.Every shape sum is a function of the rows.
BatchViewholdsrequestsandcapture_rungand nothing else. No constructor takes a sum in place of the rows, and no supplied sum can disagree with the rows it came from._refuse_shadowinggivesBatchViewandRequestShapethe__init_subclass__guardStepCostalready has: a subclass that redefines a reader, or__post_init__(where a row's integers and a rung's width are checked), raisesTypeErrorat class creation. Four tests assert the refusals, including that the message names the member. Not closed, and said in the module:BatchViewistokens x history:RequestShape(512, 2048)alone sums to 1 048 576 where the two 256-token rows it collapses sum to 524 288. The row count is the projection's guarantee (one row per scheduled request), and that builder lives only as_projectin the test file.atom/compass/runner/references neither type and compass(runner): a model runner that constructs without device memory (RUNNER-1) #80, compass(runner): the RPC surface, every reply shape taken from its caller (RUNNER-2) #90, compass(runner): the three step semantics, driven through ATOM's scheduler (RUNNER-3) #97 landed without it: compass(runner): the projection that builds a BatchView, and the rung it supplies #124.capture_rungis a batch-level scalar, correctly, because a rung is a property of the replayed graph. A rung with no decode rows and a rung narrower than the rows are refused. A rung wider than needed is not: rows can show a rung is too narrow, never too wide, and refusing a wide one needs a ladder of captured widths this backend does not have. It is priced at face value, and the padding term's provenance gainspadding from a supplied rung width; nothing here bounds it abovewhenever a rung was supplied;capture_rung=10**9on one decode row pricesdecode.graph_paddingat 99.9999999 s. Two tests pin both directions. The close, a rung read off the real capture ladder, is the second half of compass(runner): the projection that builds a BatchView, and the rung it supplies #124.A test prices two batches that collapse identically (same token total, same context total) and asserts the answers differ: that shows they are distinguishable, not that the wrong one is unreachable, and the test file says so. Another checks the sums equal what
Scheduler.compute_detailed_aggregatespublishes, against ATOM's own method.BatchViewis defined inbackends/, so the no-engine-imports scan reaches it;test_the_projection_type_is_where_the_scan_can_reach_itresolvesinspect.getfileforBatchViewandRequestShapeand asserts each is inside the scanned package.Decided, not covered by #70:
ANALYTICALis reserved for something computed without measuring the subject.FITTEDwith the origin indetailis the smallest misstatement and matches the landed package's example intest_backend_interface.py; a test assertsanalyticalnever appears.detailis dropped by both aggregating views, sostep.seconds_by_species()andProvenanceMix().seconds_by_species()each report{'fitted': 0.00014931072}: 100% of this backend's seconds reported as regressed over measured steps. Nothing in this PR consumesProvenanceMix. The enum is the owner's call: Species has no word for declared coefficients, so a stub that measured nothing reports as fitted #87.COARSE, which says step granularity.ANALYTICis tier 0's roofline and would read as the later backend arriving early.Resolverwould wrap a single rung, so the zero-priced-term collision is never reached. A second source will meet it.Parallelism.collectives()is necessary and not sufficient, so its members are charged as candidates and each term says so.TypeErrorandValueError, notCostRefused, because they are caller defects. AResolverwrapping this backend catchesCostRefusedand falls through; it will not catch these. Recorded inestimate's docstring.Design text this PR changes. D12's Option B bullet and #70's exit criterion now state the doubling term by term (below). The stale
scheduler.py:790-792citation fordetailed_sqsq/detailed_sqsk/detailed_sk(now801-803) is corrected in01_execution_and_time_model.md,02_model_runner_and_cost_backend.md,03_memory_and_kv_model.mdand14_speculative_decoding.md, with D12's gate span (:2819-2820) and method span (:2788-2841).01and09_fitting_and_law_selection.mdalso read:2788-2841now;09is outside the original file set.04_model_capture_and_cost_ir.mdis not touched; another task holds it.Surprised. ATOM's
Schedulerconstructs againstconftest.MockConfig, andschedule()/postprocess()drive chunked prefill for real without a driver, so the integration property is a 30 ms test and not a GPU job.Left undone.
bytes_per_blockandbytes_per_token_per_layerare deliberately unused: that is the roofline, and it belongs to the analytical milestone.coefficient x tokens x layers, one all-reduce per layer, using the geometry's layer count and nothing else. The stub charges the collective on paged layers, and the stack is four times deeper #123: that count is per paged layer (geometry.layers) where the all-reduce runs on the whole stack, 16 against 64 on the hybrid; it has no referent until compass(backends): the stand-in model, its widths declared once (M1-3, #24) #120 landsstage_layers.ProvenanceMix's unenforced counters are not depended on; this backend producesStepCostonly.+on a number is the integer accumulator in_per_request; each term is one product and the total is whatStepCostfolds. The bit-for-bit re-fold is asserted.Named result
A 512-token request arriving at virtual 748.00 µs joins batch 6 under the full form and batch 7 with the cross term
dset to 0: one term, 0.35% of elapsed time, changes which batch ATOM's realSchedulerbuilds. Predicted equals observed in all four cases.Break point, predicted against observed
The batch sequence is built by ATOM's real
Scheduler(chunked prefill, real admission, the real block manager). The only channel from the cost model to the scheduler is a virtual clock advanced by nothing but this backend's predictions; a request is handed toscheduler.addwhen that clock reaches its arrival time. A 2048-token prompt at a 256-token per-request chunk cap and a 512-token batch budget, so a second request that has become admittable can share the step.dset to 0The prediction is a closed sum over the declared coefficients and consults no scheduler; the observation is the first batch whose
req_idshas two entries. Over the five steps before the arrival the cross term is worth 2.62 µs against 749.18 µs elapsed, 0.35%, and that is the whole difference between making the sixth batch and missing it.Row four is why constants are not the default: under a constant price the break is at step 4 whether each step computes 256 tokens or 512. The work in a step does not enter, and a run cannot show that anything downstream reacted to the model.
Doubling the chunk, term by term. The tests assert each part:
prefill.stepprefill.tokensprefill.query_square#70's original "doubling the chunk doubles the step" is exactly true of the token term and cannot be true of the total, because the mandated form carries a quadratic query term. It is asserted literally where it can hold (quadratic and cross coefficients zero, no intercept:
long == 2 * shortexactly) and term by term everywhere else. D12's bullet and #70's exit criterion now say so: the total doubles only fora = c = d = 0.Gates
58cb9dcce: 5329 passed, 155 skipped, 3 xfailed, rc 0; tipc92e4c1c15281; +48 node ids, all this PR's (base update). GPU tier not required.Effort
Estimate 200 lines. Production physical non-blank 443 (2.22x); by AST 146 (0.73x). Over 2x is an escalation; the unit is #89's ruling.
Effort, three instruments
Measured at
0fc02f458againstd175b03c6. AST = parse, strip module/class/function docstrings, unparse, count non-blank.shape.py+ the__init__.pydelta)test_backend_shape_stub.py+ thetest_backend_interface.pydelta)Against the 200: production 0.73x by AST, 2.22x physical non-blank, 2.59x physical-all; the pair 2.23x by AST and 5.08x physical non-blank. Under the unit #70 now states (production, physical non-blank, excluding tests) the reading went 362 (1.81x) -> 425 (2.12x) -> 443 (2.22x); the last +18 are itemised in the review-reply comment. Reported, not trimmed; no comment or docstring was cut to offset it. A review that asks for explanation moves this number up.
The physical-to-AST ratio is 3.03x production and 1.91x test, against 2.3x for the parent and 2.03x for the landed
backends/package: the gap is the project's documentation style. The 200 is already 3.3x D12's sizing of the same option (~60 lines), so a 2x rule applied to it partly measures the estimator. Whichever unit the owner picks, #70 should state it ("200 production LOC, physical, excluding tests"); the ambiguity, not the code, produced the split.Generated with Claude Code