Skip to content

fix: apply the five measured findings (#24-#28) - #32

Merged
onlyxItachi merged 2 commits into
mainfrom
fix/measured-findings
Jul 19, 2026
Merged

fix: apply the five measured findings (#24-#28)#32
onlyxItachi merged 2 commits into
mainfrom
fix/measured-findings

Conversation

@onlyxItachi

Copy link
Copy Markdown
Owner

The four demanding-workload studies and the interpreter matrix each found something the tool got away with because nothing contradicted it. This applies all five.

Three are code ([dispatch] tagging for torch.compile envelopes, math.fsum for interpreter-stable digests, distinct-name counts in summarize_report); two are usage-prompt caveats, because in those cases the numbers were correct and only the premise was unmarked.

Verified beyond the suite: the tagging rule was written from a fresh real capture (all eight envelope families arrive as cpu_op, so a cat rule was never going to work), and the fsum fix was confirmed by hashing the same digest under 3.11/3.12/3.14 — now identical.

213 tests, no version bump, no release.

🤖 Generated with Claude Code

onlyxItachi and others added 2 commits July 19, 2026 17:27
Closes #24, #26, #28; addresses #25 and #27 as usage-prompt caveats.

#24 chrome_trace now tags torch.compile's own envelope spans [dispatch].
Verified against a fresh compiled-training capture: every one of them
(Torch-Compiled Region, CompiledFunction/Backward, TorchDynamo Cache
Lookup, AOTDispatcher wrappers, Inductor's ## Call CompiledFxGraph ##)
arrives with cat 'cpu_op' — indistinguishable by category from a real
aten op — so the existing cat rule could never see them and the match
has to be by name. The autograd engine also wraps the compiled backward
in an envelope of its own, so the compile-family token is matched
anywhere in the name; a prefix-only rule left that 7.4ms span looking
like work. Eager autograd envelopes stay untagged on purpose and are
documented instead: tagging those is a separate decision about eager
traces, not this finding.

#26 coverage now accumulates with math.fsum. CPython 3.12 changed sum()
to compensated summation, which made this the only value in the surface
that differed between supported interpreters. fsum is exactly rounded,
so a digest is now the same bytes everywhere — meaning a diff between
two digests always says the report changed, never the runtime. Verified
by hashing the same digest under 3.11/3.12/3.14: identical.

#28 summarize_report reports distinct_names_total, plus
distinct_names_omitted when top-N hides a name entirely. Top-N ranks
units, and one name can own many units, so a variant that lost a 0.15%
near-tie vanished from a summary that otherwise looked complete.

#25 and #27 are usage-prompt caveats because the arithmetic was never
wrong — the surface just let a false premise pass unmarked. Comparing
across a fusion boundary pairs one kernel against part of its multi-
kernel replacement (measured: +101% where the honest group comparison
was -12.3%), and ninja's start_ms/end_ms are measured from each build's
own origin, so cross-run deltas subtract unrelated clocks.

New fixture is a real compiled-training trace, shrunk by dropping whole
events rather than editing any. 213 tests.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The new test guarded against vacuity with `fsum(values) != sum(values)`
— an assertion that is itself interpreter-dependent, which is the exact
class of assumption issue #26 exists to remove. It held on 3.10/3.11 and
failed on every 3.12+ leg of the matrix, because 3.12's compensated
sum() returns the exact answer for these values.

The guard is now an explicit left-to-right accumulation, which loses the
small terms on every version. Measured while fixing: 3.12+ compensation
is exact for every sequence tried, so no unit test can distinguish sum()
from fsum() there — the docstring now says so rather than implying a
coverage this test does not have. The cross-interpreter property itself
is verified by hashing one digest under 3.11/3.12/3.14.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.

1 participant