Skip to content

feat(roofline): CSV interface to decouple analytical roofline producers/consumers - #1729

Open
abdubey-amd wants to merge 15 commits into
mainfrom
roofline-csv-interface
Open

abdubey-amd wants to merge 15 commits into
mainfrom
roofline-csv-interface

Conversation

@abdubey-amd

Copy link
Copy Markdown
Collaborator

Summary

Introduces a CSV interface that decouples Hyperloom's analytical-roofline producers from its consumers, so an external author (e.g. MAIDAS) can supply the roofline data instead of Hyperloom computing it natively. Backward-compatible: absent/blank CSVs fall back to native computation, so a run with no external CSV is unchanged.

5 commits on top of main:

  • CSV interface to decouple analytical roofline producers/consumers (schema + writer + reader + resolver).
  • Content-address roofline_ceiling.csv by config key — rows keyed by the config that determines them (fw/prec/act/kv/tp/ep/tep/pp/dcp/pcp/conc/isl/osl) rather than symbolic baseline/current_best.
  • Column-keyed ceiling CSV + ep/tep determinant + split untrusted/wrong-store logging.
  • Ceiling UPSERT new-vs-overwrite key-count logging.
  • Peak-swap diffusion ceilings from gpu_arch_peaks.csv via the arch-peak resolver.

Motivation

Lets MAIDAS (or any external roofline authority) feed Hyperloom's roofline CSVs (arch peaks, per-kernel, ceiling) at a stable interface, rather than Hyperloom recomputing them — the external ceiling is consumed in both the --no-kernel and kernel paths via the existing saturation/reloop steering.

Test plan

  • Native run with no external CSV -> byte-identical roofline output vs pre-change (backward-compat safety property).
  • External mode: point --roofline-csv-dir at a MAIDAS-authored dir -> assert the external ceiling/peaks are what the ceiling + snapshot actually use (CONSUME R / CONSUME M).
  • Ceiling CSV round-trips through the real reader/writer for every config key.

🤖 Generated with Claude Code

abdubey and others added 5 commits September 23, 2026 03:35
…rs/consumers

Introduce a CSV-based interface so an external program (e.g. MAIDAS) can author the
analytical roofline data Hyperloom consumes, with zero MAIDAS code in Hyperloom.
Modes via new CLI flags:
  --roofline-csv-dir DIR  read externally-authored CSVs (MAIDAS mode; HL writes none)
  (unset)                 native mode: HL producers write the CSVs and read them
  --no-roofline-csv       stock JSON/state path (byte-identical rollback)
  --roofline-csv-strict   fail instead of fail-soft when an expected CSV is missing

- NEW orchestrator/kernel/roofline_csv.py: stdlib-only leaf module — single source
  of the three CSV schemas (kernel_roofline / roofline_ceiling / gpu_arch_peaks),
  atomic writers (ceiling upserts by arm/op), fail-soft readers with numeric
  coercion + bound normalization, canonical_key (roofline_match_key parity), and a
  RooflineResolver facade.
- CLI/state gating threads the flags through bootstrap into SharedState;
  roofline_csv_read_dir / roofline_csv_write_dir resolver methods.
- Ceiling seam (in-process): compute_roofline_breakdown_from_state becomes a thin
  wrapper -- MAIDAS-read via _external_ceiling_breakdown (external CSV in place of
  native compute) else _compute_roofline_breakdown_native; native writes the
  composed arms via write_ceiling_arm at both assembly sites (baseline + per-cycle).
  Ceiling consumers are unchanged (they read the assembled snapshot).
- Tests: test_roofline_csv.py (18) + test_roofline_ceiling_csv.py (6).

Implements the ceiling path of CSV_INTERFACE_REFACTOR_PLAN.md; per-kernel (M2) and
arch peak-swap (M3) cross-process wiring still to come.

Co-Authored-By: Claude <noreply@anthropic.com>
Key each ceiling row by the config that determines its analytical values
(v1|fw|prec|act|kv|tp|ep|tep|pp|dcp|pcp|conc|isl|osl) instead of the symbolic
baseline/current_best, so nothing overwrites across optimizer rounds and a
consumer reads exactly its own config's row. One shared build_ceiling_key
builder (HL + MAIDAS bridge), default-deny server-args guard, external-store
model/gpu validation, op rows arm-scoped. 287 kernel tests green.

Local WIP checkpoint: also snapshots the in-progress roofline CSV-interface
producers/consumers on this branch so a stray reset --hard cannot lose them.
Not pushed.

Co-Authored-By: Claude <noreply@anthropic.com>
Emit an INFO line on every roofline_ceiling.csv write reporting how many
rows are new config keys (history preserved) vs overwrote a same-config
row (idempotent), so the content-key migration is observable in a live
optimizer run and a genuine cross-config overwrite is visible.

Co-Authored-By: Claude <noreply@anthropic.com>
…ntrusted/wrong-store logging

- decompose the ceiling content-key into per-determinant CSV columns and drop the
  packed arm string on both producer and consumer sides
- fix the ep/tep determinant in _ceiling_config: framework-aware EP degree
  (vllm tp*dp vs sglang state.ep), tep derived from ep, with a default-deny
  untrust backstop when ep is requested but resolves <= 1
- log UNTRUSTED vs WRONG STORE separately, with detail, at every consumer site
- add test_tracelens_analysis_roofline_csv.py for tool producer/consumer coverage

Co-Authored-By: Claude <noreply@anthropic.com>
…via resolver

Thread an optional RooflineResolver into the diffusion memory/compute ceiling
helpers so external-CSV mode sources HBM bandwidth and achievable TFLOPS from
gpu_arch_peaks.csv; native/disabled falls back to the vendor HW_SPECS tables,
byte-identical to prior behavior.

Co-Authored-By: Claude <noreply@anthropic.com>
import sys
from pathlib import Path

import pytest
Comment thread src/hyperloom/inference_optimizer/roofline_csv.py Fixed
abdubey-amd and others added 10 commits September 30, 2026 03:57
… + factory (additive)

Co-Authored-By: Claude <noreply@anthropic.com>
Brings the branch up to date with main (517 commits) so it is mergeable. Conflict
resolution preserves the CSV-interface feature and adopts main's refactors:

- roofline_ceiling.py / roofline_snapshot.py relocated to inference_optimizer/ (PR#1650);
  re-homed roofline_csv.py + roofline_provider.py there too (removes the
  inference_optimizer->orchestrator back-edge) and updated all importers.
- collectors/roofline.py deleted on main (PR#1455); ported the 9 None-preserving
  analytical-magnitude passthrough fields into recorder/roofline_event.py:_kernel_roofline_row.
- _build_trace_analyze_cmd relocated to actions/executors/trace_analyze.py; re-applied the
  --roofline-csv-dir/--roofline-csv-read-dir arg-passing there.
- roofline_ceiling.py content conflicts: kept my resolver= peak-swap AND main's #1602
  `_resolve_achievable_tflops(...) or _resolve_peak_tflops(...)` vendor fallback.
- load_roofline_results: kept the CSV round-trip (publish + overlay) over main's early-return.
- shared_state ceiling round-trip kept.
- Updated one test for main's #1602 (MI355X achievable table removed -> vendor fallback).

Validated: 170 roofline tests pass; full inference_optimizer/tests collects (14421) with
parity to main; conc_sweep passes in full-dir run (the isolated-collection circular import
is a pre-existing main quirk).

Co-Authored-By: Claude <noreply@anthropic.com>
Add CsvRooflineProvider (read-through over an external MAIDAS-authored CSV
dir with per-value native fallback, or strict-raise mirroring
--roofline-csv-strict) and a transparent CsvWritingProvider decorator.
Wire make_roofline_provider to return the external provider when
--roofline-csv-dir is set, else pure native. Provider is not yet consumed
by any runtime choke point, so a default/native run is unchanged.

Co-Authored-By: Claude <noreply@anthropic.com>
Route compute_roofline_breakdown_from_state through
make_roofline_provider(state).ceiling(...) instead of the inline
external-vs-native branch. The provider's ceiling() encapsulates the
identical dispatch (external arm read, native fallback, strict-raise), so
the change is byte-identical while making the provider the single seam for
every ceiling read. No duplicate mechanism remains: the dispatch moves into
the provider rather than being copied.

Co-Authored-By: Claude <noreply@anthropic.com>
The provider is a read-only seam (external CSV vs native compute); CSV
persistence is owned by the existing publish_*_csv producers, so the
write-decorator had no consumer and no job. Remove it and align the module
docstring with the read-only design.

Co-Authored-By: Claude <noreply@anthropic.com>
…eaders

--roofline-csv-strict is contractually scoped to the CONSUME-R ceiling and is
enforced inside _external_ceiling_breakdown. Drop the provider's own strict
branch (and the now-unused strict param) so CsvRooflineProvider.arch_peak /
mem_bw / model_meta fall back leniently, matching their live seams; a strict
guard is added deliberately, with a test, if a site needs it. Also correct the
module docstring to say only the ceiling read is wired today.

Co-Authored-By: Claude <noreply@anthropic.com>
The "ceiling wired today, other methods are the read surface" note appeared
twice; state it once. Docstring only, no behavior change.

Co-Authored-By: Claude <noreply@anthropic.com>
Routing the ceiling choke point through provider.ceiling() had loosened its
static type to Optional (the protocol default), while the returned value is
never None: native compute always yields a breakdown and the external reader
falls back to it. Type ceiling as -> RooflineBreakdown on the protocol and both
concrete providers so compute_roofline_breakdown_from_state's non-optional
annotation holds again. Type-only; no runtime change.

Co-Authored-By: Claude <noreply@anthropic.com>
…ak_convention)

gpu_arch_peaks.csv matrix_*_tflops was consumed as the achievable peak
regardless of what it held; an external author (MAIDAS) supplying theoretical
MFMA peaks was silently used as achievable, inflating the compute ceiling. Add
a matrix_peak_convention column ("achievable"|"theoretical"); the native
producer tags achievable, and RooflineResolver.arch_peak returns the value only
for an achievable/untagged row -> a theoretical row returns None so the caller
falls back to its native microbench-achievable table. mem_bw is unaffected
(convention-independent). Back-compat: a legacy row with no convention column
reads as achievable.

Co-Authored-By: Claude <noreply@anthropic.com>
…-interface

# Conflicts:
#	src/hyperloom/agents/kernel/tools/tracelens_analysis.py

def arch_peak(self, device: str, dtype: str) -> float | None:
"""Max-achievable matrix TFLOP/s for ``device``/``dtype`` (None on miss)."""
...

def mem_bw(self, device: str) -> float | None:
"""HBM bandwidth GB/s for ``device`` (None on miss)."""
...

def ceiling(self, state: Any, *, arm: str | None = None) -> "RooflineBreakdown":
"""Composed tok/s ceiling (mem/cmp/peak + bound) for the run's ``arm`` (native floor guaranteed)."""
...
self, state: Any, model_path: "str | Path", *, precision_hint: str = ""
) -> "ModelMeta | None":
"""Model memory sizes + geometry."""
...

def kernel(self, name: str) -> dict | None:
"""Per-kernel analytical roofline row by kernel name (None when computed inline / absent)."""
...
# Fail-soft: a write failure must not raise into the optimizer loop.
try:
os.unlink(tmp_name)
except OSError:
import sys
from pathlib import Path

import pytest
the native compute, so this call is byte-identical to the prior inline branch while routing
every ceiling read through the single provider choke point.
"""
from .roofline_provider import make_roofline_provider
# Fail-soft: a write failure must not raise into the optimizer loop.
try:
os.unlink(tmp_name)
except OSError:
if TYPE_CHECKING: # avoid import cycles at module load; these are only type hints
from pathlib import Path

from .roofline_ceiling import ModelMeta, RooflineBreakdown

def arch_peak(self, device: str, dtype: str) -> float | None:
"""Max-achievable matrix TFLOP/s for ``device``/``dtype`` (None on miss)."""
...
return float(vendor) if vendor and vendor > 0 else None

def mem_bw(self, device: str) -> float | None:
from .roofline_ceiling import HW_SPECS
return float(bw) if isinstance(bw, (int, float)) and bw > 0 else None

def ceiling(self, state: Any, *, arm: str | None = None) -> "RooflineBreakdown":
from .roofline_ceiling import _compute_roofline_breakdown_native
self, state: Any, model_path: "str | Path", *, precision_hint: str = ""
) -> "ModelMeta | None":
# The pure-native model meta comes from the HF dir; the CSV-aware path is CsvRooflineProvider.
from .roofline_ceiling import load_model_meta
def ceiling(self, state: Any, *, arm: str | None = None) -> "RooflineBreakdown":
# _external_ceiling_breakdown reads the external ceiling arm and owns the strict-raise /
# trust guards; None means "not in the external CSV" -> native fallback (always yields).
from .roofline_ceiling import _external_ceiling_breakdown
) -> "ModelMeta | None":
row = self._resolver.model_meta()
if row:
from .roofline_ceiling import _model_meta_from_row
@chaojhou

Copy link
Copy Markdown
Collaborator

@abdubey-amd this PR conflicts with current main. Could you rebase it?

Files that conflict: src/hyperloom/inference_optimizer/roofline_ceiling.py, src/hyperloom/inference_optimizer/roofline_snapshot.py, src/hyperloom/inference_optimizer/cli/parser.py, src/hyperloom/orchestrator/state/shared_state.py, src/hyperloom/agents/kernel/tools/tracelens_analysis.py, the bypass trace-analysis modules under src/hyperloom/orchestrator/trace_analysis/, and their tests.

#1787 moved the kernel tools out of agents/kernel/tools/ (those files no longer exist there), and later refactors (#1973, #2020, #2057, #2077) reworked roofline_ceiling.py and the trace-analysis paths, so this is closer to a re-port than a rebase. CI's ruff job also fails at the current head. At about 2,900 changed lines, splitting it would help: the CSV schema and reader/writer first, then the provider and its consumers. This is also the open PR behind #1646.

There has been no activity since 2026-09-30; if it is no longer planned, say so and we'll close it.

Once it merges cleanly I'll review it.

@chaojhou chaojhou 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.

PR #1729 -- feat(roofline): CSV interface to decouple analytical roofline producers/consumers

What it does: lets an external roofline author (MAIDAS) supply Hyperloom's analytical roofline. --roofline-csv-dir DIR makes the ceiling, arch peaks, model meta and per-kernel rows come from CSVs in DIR. A RooflineProvider sits at the ceiling choke point, falling back to native compute when DIR has nothing usable. Native runs now write the same CSVs under <session_dir>/roofline_csv/ and read them back.

The branch cannot merge as it stands. It is 746 commits behind main and conflicts in 8 files:

  • agents/kernel/tools/tracelens_analysis.py was deleted on main.
  • The _bypass_* files moved to orchestrator/trace_analysis/ (#1787).
  • roofline_ceiling.py, roofline_snapshot.py, shared_state.py and cli/parser.py were rewritten (#2077, #2042).

The rebase is a re-implementation. The issues below are in the mechanism and carry over into it.

Blocking issues: 5

  1. [free:wrong-store-meta-leak] The "native fallback" for a store authored for another model still uses that store's model geometry [verified]
    Problem: when _external_store_mismatch rejects the external store (model_id differs), _external_ceiling_breakdown logs "WRONG STORE ... -> native fallback" and returns None (src/hyperloom/inference_optimizer/roofline_ceiling.py:1406-1411). The fallback reaches _compute_roofline_breakdown_native, which calls resolve_model_meta (:1671). Because roofline_csv_dir is set, resolve_model_meta reads the same rejected model_roofline_meta.csv with no model_id check (:1001-1008). ExternalRooflineProvider.model_meta does the same (roofline_provider.py:144-148). On an extract of the head, a native ceiling of 18104.50 tok/s becomes 2958.55 tok/s once the store holds another model's meta.
    Impact: the run's ceiling, and with it saturation and reloop steering, is computed from the wrong model's layer count, KV geometry and weight bytes, while the log says it fell back to native.
    Action: decide the store's validity once and have every external read obey it. When the store is rejected, nothing from it (ceiling, model meta, arch peaks) may be used.

  2. [free:partial-meta-zero-fill] A blank cell in an external model_roofline_meta.csv becomes 0 instead of falling back [verified]
    Problem: the reader returns None for blank cells, as its contract says ("blank is None, not 0", roofline_csv.py:13-16). _model_meta_from_row then maps every non-number to 0 or 0.0 (src/hyperloom/inference_optimizer/roofline_ceiling.py:989-991). resolve_model_meta returns that meta whenever any row exists (:1005-1008), so it never falls back to load_model_meta. With num_layers, num_kv_heads and head_dim blank, the KV bytes are 0 and the reproduced ceiling is 3.2e13 tok/s.
    Impact: a partially filled external meta produces an absurd ceiling rather than the native one the description promises for blank values.
    Action: treat a row missing any field the ceiling needs as absent and fall back to load_model_meta, or refuse it, but never coerce to 0. Pin it with a test.

  3. [free:snapshot-two-peak-sources] One roofline snapshot records two ceilings from different peak sources [verified]
    Problem: in external mode with gpu_arch_peaks.csv and no ceiling row, the headline ceiling uses the external peaks through _arch_peak_resolver (roofline_ceiling.py:1700, :1709, :1726). attach_perfmodel_breakdown recomputes the PerfModel without resolver= (src/hyperloom/inference_optimizer/roofline_snapshot.py:211). It then records the vendor-table peak in roofline_provenance (:220). Reproduced: the headline is 9052.25 tok/s and perfmodel_breakdown is 18104.50 tok/s, with a provenance naming a source the headline did not use.
    Impact: consumers of the snapshot and the breakdown export see two ceilings, and a provenance that misattributes the external number.
    Action: pass the same resolver to the PerfModel in attach_perfmodel_breakdown and derive the provenance from it, so a snapshot has one peak source.

  4. [free:strict-swallowed] --roofline-csv-strict never fails the run, because the PR wraps the ceiling in new except Exception: pass blocks [verified]
    Problem: strict mode raises FileNotFoundError when the run's ceiling row is missing (roofline_ceiling.py:1415-1421). Both production callers now catch it in a new except Exception: pass (src/hyperloom/orchestrator/state/shared_state.py:2014, :2357). record_baseline_roofline_ceiling then returns {} and the run continues with no ceiling. The flag neither fails nor falls back to native, contrary to its help text (parser.py:673-677). The PR adds three more broad catches (roofline_ceiling.py:1018, :1592, :1735); the one at :1735 turns a PerfModel failure into the legacy-formula ceiling. Neither the base nor current main has any of these handlers. This breaks AGENTS.md Don't grow the debt (no new broad except Exception).
    Impact: an operator who asks for strict gets a silently ceiling-less run, and native ceiling failures that used to surface are now hidden.
    Action: remove the five broad catches and let strict raise to the run boundary. Decide whether strict also covers the untrusted and wrong-store skips (:1399-1411), and make the help text match.

  5. [X2] The description covers 5 of 15 commits and leaves out operator-observable changes [verified]
    Problem: the body does not mention:

    • --roofline-csv-strict or --no-roofline-csv (src/hyperloom/inference_optimizer/cli/parser.py:673-686);
    • the env defaults HYPERLOOM_ROOFLINE_CSV_DIR and HYPERLOOM_ROOFLINE_CSV_READ_DIR (tracelens_analysis.py:7383, :7393);
    • the RooflineProvider layer or matrix_peak_convention;
    • that every default native run now writes <session_dir>/roofline_csv/ and logs a ceiling UPSERT line on each snapshot.

    It says a run without an external CSV is "unchanged". The flag help says native mode writes "under reports/" (parser.py:669), but roofline_csv_write_dir writes <session_dir>/roofline_csv (shared_state.py:2188). kernel_roofline.csv is advertised as an external input (parser.py:665), but no production path reads it: trace_analyze never passes --roofline-json. Three comments cite CSV_INTERFACE_REFACTOR_PLAN.md, which is not in the tree. The PR template's Observable effect, Size/complexity and Breaking changes sections are missing, and no test-plan item is checked.
    Impact: operators and the release notes get an incomplete account of new flags, env vars and session artifacts. An external author supplying kernel_roofline.csv has it silently ignored.
    Action: for the re-implementation, write the description from the template and state every flag, env var, artifact and log line. Either wire kernel_roofline.csv into a production read or drop it from the interface. Remove the references to the missing plan document.

deferred: free:rollback-valve -- --no-roofline-csv ("Rollback valve ... for A/B comparison and safety", parser.py:680-686) exists only to switch off a native write-then-read of the same value, and the two env defaults are a second route to --roofline-csv-dir. Both are against AGENTS.md Don't grow the debt. Drop the native round-trip, the flag and the env twins in the re-implementation.

CI: ruff fails at f8784e2 (gated, so it is not reviewed here). Every other check passed at that head, but the head is 746 commits behind main, so those results do not cover the code this would merge into.

Checked: roofline_csv.py, roofline_provider.py, roofline_ceiling.py (_external_store_mismatch, _external_ceiling_breakdown, resolve_model_meta, _model_meta_from_row, publish_and_read_ceiling_arm, _compute_roofline_breakdown_native, _arch_peak_resolver), roofline_snapshot.py attach_perfmodel_breakdown, shared_state.py record_baseline_roofline_ceiling and the snapshot path, cli/parser.py and bootstrap.py, tracelens/bypass producers and trace_analyze; main's versions of the touched files and #1787/#2077/#2042 | Ran: trial merge onto main (8 conflicts); on an extract of the head, native vs wrong-store vs partial-meta ceilings, strict with a missing row through record_baseline_roofline_ceiling, external arch peaks through the ceiling and the snapshot; broad-except counts at base, main and head; the PR's roofline test files at head (168 passed); rule checklist and two independent refutation passes per finding | Base: bd5a335 | Head: f8784e2

@tsrikris

Copy link
Copy Markdown
Collaborator

@chaojhou I would wait on this PR till we internally align across teams on the direction to be followed. TraceLens has also build a custom perf model interface that should be considered to bring up a unified interface.

@chaojhou

Copy link
Copy Markdown
Collaborator

Closing this for now, as noted in the 2026-10-22 rebase request. There has been no reply or push since then, and the branch still conflicts with main: a trial merge against ba689ef conflicts in 32 files. Main has moved a long way under it. The kernel tools left agents/kernel/tools/ (#1787), and roofline_ceiling.py and the trace-analysis paths were reworked since, so bringing it up to date is closer to a re-port than a rebase.

Thank you for the work on the roofline CSV interface. If you'd like to continue, please re-port it onto current main and split it into smaller PRs, for example the CSV schema and reader/writer first, then the provider and its consumers. Reopen this or link the new PRs here. The review comments above say where each piece now belongs.

@chaojhou chaojhou closed this Oct 25, 2026
@chaojhou chaojhou reopened this Oct 25, 2026
@chaojhou

Copy link
Copy Markdown
Collaborator

Reopening: closing three days after the 2026-10-22 rebase request was too soon. This stays open for a month from that request, until 2026-11-22. If there is still no rebase or reply by then, it will be closed. The notes in the comment above still apply.

This branch has not been deployed

No deployments
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.

4 participants