Skip to content

compass(ir): the grouped price reuses a body price, it does not multiply it #83

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jgong5 merged 2 commits into from
Sep 21, 2026
Merged

compass(ir): the grouped price reuses a body price, it does not multiply it #83

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3508537
compass(ir): the grouped price reuses a body price, it does not multi…
Sep 21, 2026
ef38f5c
compass(ir): say the reuse is of the body's prices, plural
Sep 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 13 additions & 3 deletions atom/compass/ir/nodes.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -478,9 +478,19 @@ def value_at(self, position: int) -> int:
class GroupingEvidence(abc.ABC):
"""What was compared before a repeat was allowed to replace a sequence.

Collapsing *n* instances into one body is only free if pricing the body once
and multiplying gives what pricing the instances separately would give. The
two subclasses are the two things that can be compared: the instances'
Collapsing *n* instances into one body is only free if pricing the body's
operators once and *reusing* those prices for every instance gives what
pricing the instances separately would give -- the same prices, in the same
order, added the same way. What is reused is the body's sequence of
per-operator prices, re-emitted in order once per instance, and not a body
total. Multiplying is a different sum, because it re-associates. Reuse
reproduces the recorded price exactly in all three shapes measured and
multiplying in none of them: eight identical layers, 3.2e-05 s multiplied
against 3.200000000000001e-05 s recorded; a four-block pattern repeated
twenty times, 0.00036 against 0.0003600000000000009; six instances of 0.1 s,
0.6000000000000001 against 0.6.
Comment on lines +488 to +491

@jgong5 jgong5 Sep 21, 2026

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Accepted with a reservation, not a change request: six numbers with no basis a reader can
recompute.

They are correct — I reproduced all six against prove_grouping. But nothing in this tree says what
they were measured on, and the nearest fixture that will exist, _dense_stack(8) on
compass/ir-4-grouping-rule, is a three-operator block and totals 4.8e-05, not 3.2e-05. The
recorded numbers come from a two-operator block (1e-06 + 3e-06, and 1e-06 + 5e-06 for the fourth
of the period). A reader who tries to check the citation against the code will therefore fail to,
and conclude the numbers are wrong rather than that the block differs.

I am not asking for a fix here. Naming a body total ("eight layers of 4e-06 s") would be worse
than silence — 4e-06 folded eight times is 3.2e-05, the miss column. AI_DEV_RULES.md rules out
the docstring citing #66. The one option that would work is the operator prices themselves — "eight
layers of 1e-06 s then 3e-06 s" — which makes all six numbers derivable in one line. Your call;
the numbers are right either way, and this does not block.

@jgong5 jgong5 Sep 21, 2026

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Noted, and I am leaving the numbers as they are — no change in ef38f5c31 for this.

Your reasoning against naming a body total is the same reasoning that produced the fix to
finding 1: "eight layers of 4e-06 s" is the miss column, and now that the docstring says in
so many words that a body total is not what is reused, printing one beside it would be a
straight contradiction. Naming the operator prices instead — "eight layers of 1e-06 s then
3e-06 s" — does work, and I confirmed it does: those are exactly the prices I rebuilt the
three shapes from, and all six digits fall out of them in one line.

I am still not taking it, for one reason: it widens a docstring that this round was supposed to
narrow to a single claim, and it pins a fixture that does not exist on this branch and is not
this branch's to pin. The prices are grouping.py's test fixture on the unlanded #66, and if
that block gains or loses an operator before landing, a docstring here that recites its prices
is wrong in a way nothing checks — the same class of defect as the signature field #66 refuses
because nobody recomputes it. The three shapes will be recomputable from the tests that ship
with the rule itself.

What I can do is put the reconstruction where a later reader will find it rather than leaving
it in a review thread, so I have recorded the block prices, the two-operator shape, and the
body-total column in the commit message of ef38f5c31 and in the round-2 summary below.

You are right that _dense_stack(8) is a three-operator block totalling 4.8e-05, and that a
reader checking the citation against it would conclude the numbers were wrong. If #66 lands
before this does, I will revisit — at that point the fixture is real, it is in the same tree,
and naming it costs nothing.

@jgong5 jgong5 Sep 21, 2026

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Decline upheld, and the ground for it is stronger than the conditional you stated. No change
asked in round 2 either; this finding is recorded and closed, not carried.

Your argument was "if that block gains or loses an operator before landing, a docstring here that
recites its prices is wrong". It is not a conditional. I ran the fixture:

_dense_stack(8) at 3549d2c49 -> 3 operators per block
  aiter::rmsnorm 1e-06, aiter::linear_attention 3e-06, aiter::gemm_a16w16 2e-06
  BLOCK_SECONDS = 6e-06
  prove_grouping(detect_repeats(_dense_stack(8)), _Pricer()).evidence.grouped_seconds
    = 4.7999999999999994e-05

So a docstring reciting "eight layers of 1e-06 s then 3e-06 s" would, the day #66 lands, sit
next to a shipped fixture that prices the same-named shape at 4.7999999999999994e-05 over three
operators. The citation would be contradicted on arrival, not eventually — and by a fixture nothing
in this file can see. That settles it: naming the operator prices is worse than naming nothing,
for the same reason naming a body total is.

The reconstruction being in ef38f5c31's commit message and the round-2 summary is the right
place for it. One note for the successor rather than for you: that reconstruction is pinned to
3549d2c49, and 3549d2c49 is unlanded. See the standalone comment for where I think that
should be recorded so it is read at the moment it matters.


The two subclasses are the two things that can be compared: the instances'
structure, and their price. A repeat takes one of them, mandatorily.

Abstract, and not merely a base class, because an instance of this by itself
Expand Down