adr: add ADR-0043/0044 (eval harnesses); reconcile ADR-0024/0032 for SIP w/h
Document the allreduce + GEMM evaluation harnesses and bring the affected allreduce ADRs in line with the refactored code. New (Accepted, EN + KO): - ADR-0043 — allreduce evaluation harness (tests/sccl/): distributed-driven correctness, latency/buffer-kind sweeps, sessionfinish plot aggregators, topology + FSIM-comparison figures. Verified against the implementation. - ADR-0044 — GEMM evaluation harness (scripts/gemm_sweep.py + tests/gemm/): heavy-script data gen vs. fast test-rendered figures, slow regenerator, the 3-figure set. Records two limitations as open questions: the theoretical-model constants are inherited (not yet traced to ADR-0033/ 0014), and the *_measured figure is a naming misnomer. Updated (EN + KO): - ADR-0024 — add D5: SIP grid w/h resolution (explicit sips.w/h, square fallback, fail-loud), documenting the AhbmCCLBackend fix. - ADR-0032 — D4/D5/Non-goals reconciled: rectangular SIP grids (e.g. 6 SIPs as 3x2) are supported via explicit w/h; the square requirement now applies only to the fallback. Affected-files repointed to tests/sccl/. Verification: ADR-0023 and ADR-0042 confirmed still matching the code (no change). verify_adr_lang_pairs.py passes (EN/KO Status blocks byte-equal). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -173,6 +173,37 @@ placement = resolve_dp_policy(
|
||||
No post-hoc `pe_index` shifting — ShardSpec carries the `(sip, cube, pe)`
|
||||
structural coordinates directly. ShardSpec details in ADR-0026.
|
||||
|
||||
### D5. SIP grid dimensions — explicit `sips.w/h` resolution
|
||||
|
||||
For 2D inter-SIP topologies (`torus_2d`, `mesh_2d_no_wrap`) the SIP grid
|
||||
shape (width × height) is resolved from `system.sips.w` / `system.sips.h`,
|
||||
mirroring how D1 resolves `world_size` from `sips.count`. Precedence:
|
||||
explicit `w/h` (validated `w*h == count`) > square fallback
|
||||
(`round(sqrt(count))²`, used only when no `w/h` is given) > error.
|
||||
|
||||
```python
|
||||
sips = spec.get("system", {}).get("sips", {})
|
||||
if sip_topo == "ring_1d":
|
||||
w, h = 0, 0 # 1D sentinel (no grid)
|
||||
elif sips.get("w") is not None and sips.get("h") is not None:
|
||||
w, h = int(sips["w"]), int(sips["h"])
|
||||
if w * h != n_sips:
|
||||
raise ValueError(f"sip layout {w}x{h} != sips.count ({n_sips})")
|
||||
else:
|
||||
side = int(round(math.sqrt(n_sips)))
|
||||
if side * side != n_sips:
|
||||
raise ValueError("non-square sips.count requires explicit sips.w/h")
|
||||
w, h = side, side
|
||||
```
|
||||
|
||||
This lifts the earlier assumption that 2D SIP grids must be perfect
|
||||
squares: a 6-SIP `torus_2d` / `mesh_2d_no_wrap` is now expressible as
|
||||
`w: 3, h: 2` (or `2x3`). The derived `(w, h)` feed the algorithm's
|
||||
inter-SIP exchange (consumed in ADR-0032 D5). The prior code path silently
|
||||
took `round(sqrt(count))²` for any non-ring topology, which produced a
|
||||
wrong grid (e.g. 2×2 for 6 SIPs); the explicit-`w/h` path with a
|
||||
fail-loud fallback replaces that.
|
||||
|
||||
---
|
||||
|
||||
## Dependencies
|
||||
|
||||
@@ -138,20 +138,24 @@ system:
|
||||
```
|
||||
|
||||
- `ring_1d`: n_sips-1 rounds of `send global_E / recv global_W`.
|
||||
- `torus_2d`: sqrt(n_sips)×sqrt(n_sips) wrapping mesh. Row ring on
|
||||
`global_E/W` then col ring on `global_S/N`.
|
||||
- `mesh_2d_no_wrap`: square mesh without wrap-around. Chain reduce +
|
||||
- `torus_2d`: `w × h` wrapping mesh. Row ring on `global_E/W` then col
|
||||
ring on `global_S/N`.
|
||||
- `mesh_2d_no_wrap`: `w × h` mesh without wrap-around. Chain reduce +
|
||||
broadcast per dimension.
|
||||
|
||||
2D variants require `n_sips` to be a perfect square.
|
||||
2D grid dims `(w, h)` come from `system.sips.w/h` (ADR-0024 D5). A square
|
||||
fallback (`round(sqrt(n_sips))²`) applies **only** when `w/h` are omitted,
|
||||
so rectangular grids (e.g. 6 SIPs as `3×2`) are supported by giving
|
||||
explicit `w/h`.
|
||||
|
||||
### D5. Process-group integration — `AhbmCCLBackend`
|
||||
|
||||
At `init_process_group` time the backend:
|
||||
|
||||
1. Loads `ccl.yaml` + `topology.yaml`.
|
||||
2. Derives `sip_topo_kind, sip_topo_w, sip_topo_h` from
|
||||
`system.sips.topology` using the algorithm module's `TOPO_NAME_TO_KIND`.
|
||||
2. Derives `sip_topo_kind` from `system.sips.topology` via the algorithm
|
||||
module's `TOPO_NAME_TO_KIND`, and `sip_topo_w, sip_topo_h` from
|
||||
`system.sips.w/h` with a square-only fallback (ADR-0024 D5).
|
||||
3. Calls `configure_sfr_intercube_multisip(engine, spec, cfg)` — one-time
|
||||
SFR wiring, mirrors NCCL communicator creation.
|
||||
|
||||
@@ -222,8 +226,10 @@ Modules loaded via `cfg["module"]` must export:
|
||||
|
||||
- **Per-PE allreduce** (intra-cube PE-to-PE reduce). Out of scope — the
|
||||
workload for this algorithm is per-cube DP.
|
||||
- **Asymmetric SIP topologies** (non-square mesh/torus). `torus_2d` and
|
||||
`mesh_2d_no_wrap` require `n_sips = k²`.
|
||||
- **Square-grid fallback requires `n_sips = k²`**: rectangular SIP grids
|
||||
(non-square mesh/torus) are supported, but only when `system.sips.w/h`
|
||||
are given explicitly (ADR-0024 D5). With `w/h` omitted, 2D topologies
|
||||
fall back to a square grid and still require `n_sips = k²`.
|
||||
- **Pipelined chunks**: single-tile per cube, no pipelining yet.
|
||||
- **Root cube runtime election**: the kernel currently uses
|
||||
`root_cube = (mesh_h // 2) * mesh_w + (mesh_w // 2)` — the geometric
|
||||
@@ -270,7 +276,6 @@ Modules loaded via `cfg["module"]` must export:
|
||||
| `ccl.yaml` | Single `lrab_hierarchical_allreduce` entry |
|
||||
| `topology.yaml` | Added `system.sips.topology` |
|
||||
| `benches/ccl_allreduce.py` | Row-wise cube-mesh tensor layout |
|
||||
| `tests/test_allreduce_multidevice.py` (new) | Config-driven ring/torus/mesh |
|
||||
| `tests/test_distributed_lrab_hierarchical_allreduce.py` (new) | Full `dist.all_reduce` path |
|
||||
| `tests/test_intercube_sfr_config.py` (new) | SFR wiring verification |
|
||||
| `tests/sccl/` (test package) | Config-driven ring/torus/mesh correctness + full `dist.all_reduce` path + latency/buffer-kind sweeps (evaluation harness — ADR-0043) |
|
||||
| `tests/test_intercube_sfr_config.py` | SFR wiring verification |
|
||||
| Removed | `ring_allreduce.py`, `mesh_allreduce.py`, `tree_allreduce.py`, `hierarchical_allreduce.py`, `hello_send.py`, `testing.py` and their tests |
|
||||
|
||||
@@ -0,0 +1,130 @@
|
||||
# ADR-0043: Allreduce Evaluation Harness — `tests/sccl/`
|
||||
|
||||
## Status
|
||||
|
||||
Accepted
|
||||
|
||||
Documents the `tests/sccl/` evaluation harness; verified against the
|
||||
implementation (constants, file set, and sweep dimensions cross-checked).
|
||||
|
||||
## Context
|
||||
|
||||
ADR-0032 defines the intercube all-reduce *algorithm*; ADR-0023/0024/0027
|
||||
define the IPCQ backend, the rank=SIP launcher, and `mp.spawn`. None of
|
||||
them describe **how the allreduce is exercised and characterized** — the
|
||||
correctness tests, the latency/buffer-kind sweeps, and the derived plots.
|
||||
ADR-0013 (verification strategy) is the general policy; this ADR pins the
|
||||
concrete allreduce harness so the "evaluation" half of the work is
|
||||
documented, not just the implementation.
|
||||
|
||||
The harness lives under `tests/sccl/` (the package created when the
|
||||
allreduce tests were consolidated). It supersedes the earlier flat
|
||||
`tests/test_allreduce_multidevice.py` + `tests/test_distributed_*` layout.
|
||||
|
||||
## Decision
|
||||
|
||||
### D1. Drive evaluation through the public `torch.distributed` path
|
||||
|
||||
Correctness and the sweeps run the collective through the real DDP-shaped
|
||||
path — `init_process_group(backend="ahbm") → mp.spawn → dist.all_reduce`
|
||||
(ADR-0024/0027) — not the lower-level `ctx.launch`. A shared helper
|
||||
`_run_distributed(tmp_path, monkeypatch, topo_path, corr_id, n_elem)` in
|
||||
`tests/sccl/_allreduce_helpers.py` builds the engine, runs the workers, and
|
||||
returns `(engine, n_cubes)`. `monkeypatch.chdir` points the backend's
|
||||
`load_ccl_config()` (cwd lookup) at a per-case temp `ccl.yaml`.
|
||||
|
||||
A direct-launch reference (`run_allreduce`) is retained in the same helper
|
||||
module — not for the distributed tests, but because the IPCQ buffer-kind /
|
||||
root-center micro-tests under `tests/` import it.
|
||||
|
||||
### D2. One file per evaluation concern
|
||||
|
||||
| File | Concern | `torch.distributed`? |
|
||||
|---|---|---|
|
||||
| `test_allreduce_ring_torus_mesh.py` | correctness across ring_1d / torus_2d (2×3) / mesh_2d_no_wrap (2×3) | yes |
|
||||
| `test_distributed_default_topology.py` | full path on `topology.yaml` as-is | yes |
|
||||
| `test_plot_latency_sweep.py` | latency sweep rows (n_elem × topology) | yes |
|
||||
| `test_plot_buffer_kind_sweep.py` | TCM/SRAM/HBM sweep rows | yes |
|
||||
| `test_plot_topology_diagram.py` | topology.png (pure matplotlib) | no |
|
||||
| `test_plot_comparison_fsim.py` | broken-axis model-vs-FSIM comparison | no |
|
||||
| `test_intercube_root_center.py` | ADR-0032 center-root latency guard (direct path) | no |
|
||||
|
||||
`_allreduce_helpers.py` holds the shared plumbing (driver, config writers,
|
||||
sweep/buffer-kind constants, plot aggregators, topology-diagram + FSIM
|
||||
comparison emitters). It is not collected (no `test_` prefix).
|
||||
|
||||
### D3. Latency metric — critical-path `pe_exec_ns`
|
||||
|
||||
The reported latency per config is `crit_ns = max(pe_exec_ns)` over
|
||||
`engine._results` — the slowest rank's PE execution time. This is the
|
||||
number plotted on every latency chart and recorded in `summary.csv`.
|
||||
|
||||
### D4. Sweep dimensions
|
||||
|
||||
- **Latency sweep**: `n_elem ∈ {8, 32, 64, 128, 512, 1024, 2048, 4096,
|
||||
8192, 16384, 32768, 49152}` (16 excluded — collides with `n_cubes`) ×
|
||||
topology ∈ {ring_1d (6), torus_2d 2×3 (6), mesh_2d_no_wrap 2×3 (6)}.
|
||||
- **Buffer-kind sweep**: `buffer_kind ∈ {tcm, sram, hbm}` × a smaller
|
||||
`n_elem` grid, on torus_2d 6-SIP (3×2). buffer_kind is set in the temp
|
||||
`ccl.yaml` (read by the backend at `init_process_group`, ADR-0023 D6).
|
||||
|
||||
The 2×3 / 3×2 grids exercise the explicit-`w/h` SIP resolution
|
||||
(ADR-0024 D5).
|
||||
|
||||
### D5. Derived plots via `pytest_sessionfinish` aggregators
|
||||
|
||||
Sweep tests are xdist-friendly: each parametrized case writes one JSON row
|
||||
to a staging dir. The conftest `pytest_sessionfinish` hook (controller node
|
||||
only) calls the aggregators in `_allreduce_helpers.py`:
|
||||
|
||||
- `_aggregate_sweep_plots()` → per-topology PNGs + `summary.csv`
|
||||
- `aggregate_buffer_kind_plot()` → the TCM/SRAM/HBM comparison PNG + csv
|
||||
|
||||
The topology-diagram and FSIM-comparison figures are emitted directly by
|
||||
their own `test_plot_*` tests (no row staging — they are pure functions of
|
||||
`topology.yaml` and `summary.csv` respectively). All outputs land in
|
||||
`docs/diagrams/allreduce_latency_plots/` and are **derived artifacts** per
|
||||
CLAUDE.md (consistent-with-ADRs, no Phase-2 gate).
|
||||
|
||||
### D6. The FSIM comparison reference is a hardcoded constant
|
||||
|
||||
`emit_comparison_fsim_plot()` overlays the model curves against a single
|
||||
external FSIM single-device reference (`366 µs`), held as a literal — there
|
||||
is no external data file. The "measured" series comes from the simulator
|
||||
(`op_log` GEMM count, `composite_window_ns`); the "theoretical" series is a
|
||||
hand-derived analytical model (the same one ADR-0044 D5 flags as
|
||||
ADR-unverified).
|
||||
|
||||
## Consequences
|
||||
|
||||
### Positive
|
||||
|
||||
- The allreduce is evaluated through the same API a real DDP script uses,
|
||||
so the harness doubles as an integration test of ADR-0024/0027.
|
||||
- Figures regenerate on every `pytest` run from committed data; no manual
|
||||
plot step.
|
||||
- Rectangular-grid sweeps gave the regression coverage that surfaced the
|
||||
ADR-0024 D5 `w/h` fix.
|
||||
|
||||
### Negative / limitations
|
||||
|
||||
- The full latency sweep runs in the default `pytest` (~minutes); it is not
|
||||
marked `slow`. (Contrast ADR-0044, where the GEMM sweep is `slow`.)
|
||||
- `test_intercube_root_center.py` carries a latency *threshold* assertion
|
||||
(ADR-0032 center-root guard) — the only absolute-latency assertion in the
|
||||
suite; it is sensitive to latency-model changes (ADR-0033).
|
||||
|
||||
## Dependencies
|
||||
|
||||
- **ADR-0013**: verification strategy (general policy this specializes).
|
||||
- **ADR-0023 / ADR-0024 / ADR-0027**: IPCQ backend, rank=SIP launcher,
|
||||
`mp.spawn` — the path D1 drives.
|
||||
- **ADR-0032**: the algorithm under evaluation; D4 grids exercise its
|
||||
topology branches.
|
||||
- **ADR-0044**: the sibling GEMM evaluation harness.
|
||||
|
||||
## Open questions
|
||||
|
||||
- Should the latency sweep be marked `slow` for parity with the GEMM sweep?
|
||||
- Should the FSIM reference move from a hardcoded constant to a versioned
|
||||
data file?
|
||||
@@ -0,0 +1,130 @@
|
||||
# ADR-0044: GEMM Evaluation Harness — `scripts/gemm_sweep.py` + `tests/gemm/`
|
||||
|
||||
## Status
|
||||
|
||||
Accepted
|
||||
|
||||
Documents the GEMM evaluation/characterization harness; verified against the
|
||||
implementation (constants, tile sizes, figure set, and the script↔test
|
||||
split cross-checked). The D5/D6 caveats are recorded limitations, not
|
||||
inaccuracies.
|
||||
|
||||
## Context
|
||||
|
||||
ADR-0014 (PE pipeline) and ADR-0042 (tile-plan generators) define the GEMM
|
||||
*implementation*; ADR-0033 defines the latency model. None of them describe
|
||||
**how GEMM performance is swept and characterized** — the shape/variant
|
||||
sweep that produces the timing data, and the figures that interpret it.
|
||||
This ADR pins that harness.
|
||||
|
||||
Unlike the allreduce harness (ADR-0043), the GEMM sweep is **heavy** (24
|
||||
sim runs: 8 shapes × 3 operand-staging variants; the `512` shape alone is
|
||||
2048 tiles). That weight drives the split below.
|
||||
|
||||
## Decision
|
||||
|
||||
### D1. Two-layer split — heavy data generation (script) vs. fast figures (tests)
|
||||
|
||||
- **Data generation stays a manual script**: `scripts/gemm_sweep.py` runs
|
||||
`matmul-composite` (ADR-0042 plans) across shapes × variants via the same
|
||||
`run_bench` path the CLI uses, harvests `result.engine.op_log`, and
|
||||
writes `docs/diagrams/gemm_sweep.json` (per-stage / per-engine wall-clock
|
||||
+ occupancy + record counts + pe/composite windows).
|
||||
- **Figure rendering is test-generated**: `tests/gemm/` reads the committed
|
||||
`gemm_sweep.json` and renders matplotlib PNGs into
|
||||
`docs/diagrams/gemm_plots/`. These tests are fast and run by default.
|
||||
|
||||
Rationale: a slide-deck-scale sim sweep does not belong in every `pytest`
|
||||
run, but the figures (cheap, deterministic) should regenerate freely and be
|
||||
guarded by CI. This mirrors CLAUDE.md's script-vs-test split (scripts for
|
||||
heavy/manual generation; tests for fast assertions).
|
||||
|
||||
### D2. Slow regenerator test wraps the script
|
||||
|
||||
`tests/gemm/test_gemm_sweep.py` is marked `@pytest.mark.slow` (excluded by
|
||||
the default `addopts: -m "not slow"`). It invokes `scripts/gemm_sweep.py`
|
||||
via subprocess to regenerate `gemm_sweep.json` on demand
|
||||
(`pytest -m slow tests/gemm/test_gemm_sweep.py`). The sweep logic has a
|
||||
single home (the script); the test only wraps it, so there is no duplicated
|
||||
sim-driving code.
|
||||
|
||||
### D3. Figure set (3 charts, `load_ref` variant)
|
||||
|
||||
| Test | PNG | Content |
|
||||
|---|---|---|
|
||||
| `test_plot_gemm_stage_breakdown.py` | `gemm_stage_breakdown.png` | per-stage engine wall-clock (DMA in / Fetch / GEMM / DMA out) |
|
||||
| `test_plot_gemm_mac_utilization.py` | `gemm_mac_utilization_measured.png` | GEMM util % + useful eff % |
|
||||
| `test_plot_gemm_mac_utilization.py` | `gemm_mac_utilization_theoretical_vs_measured.png` | theoretical vs simulator-measured util/eff |
|
||||
|
||||
`tests/gemm/_gemm_plot_helpers.py` holds the shared renderers (series logic
|
||||
mirrors the GEMM `_render_*` functions in `scripts/build_overview_slides.py`,
|
||||
which still draws these natively in the PPTX). Not collected (no `test_`
|
||||
prefix). Each `test_plot_*` skips if `gemm_sweep.json` is absent.
|
||||
|
||||
### D4. Tile sizes are data-driven; under-tile shapes are flagged
|
||||
|
||||
Tile sizes are read from `gemm_sweep.json` (`tile_sizes`), which the sweep
|
||||
records from `PeSchedulerComponent.TILE_M/K/N = 32/64/32` — the authoritative
|
||||
source. Shapes with `M<TILE_M ∨ K<TILE_K ∨ N<TILE_N` are flagged
|
||||
("under-tile") on the charts. The `512³` shape is excluded from the figures
|
||||
(`EXCLUDED_SHAPES`).
|
||||
|
||||
### D5. Theoretical model — inherited constants, NOT yet ADR-verified
|
||||
|
||||
The "theoretical" curves use an analytical ideal-pipeline model with
|
||||
constants copied verbatim from `scripts/build_overview_slides.py`:
|
||||
|
||||
```
|
||||
HBM_GBS = 256.0 # GB/s T_STAGE = 16.0 ns
|
||||
D_STAGES = 3 BPE = 2
|
||||
```
|
||||
|
||||
**These are not yet sourced against the ADRs.** Notably ADR-0033's `256`
|
||||
is `burst_bytes` (256 B), a *different* quantity than this `256 GB/s`, and
|
||||
ADR-0033 derives bandwidth as `pc_bw_gbs = hbm_to_router_bw_gbs / num_pcs`.
|
||||
`T_STAGE`/stage-count are not traced to ADR-0014 here. The model is
|
||||
therefore **consistent with the existing deck script, not verified against
|
||||
the ADRs**, and the constants are duplicated (deck + helper). Reconciling
|
||||
them (source from topology/ADR-0033/0014, de-duplicate) is deferred — see
|
||||
Open questions.
|
||||
|
||||
### D6. Known naming caveat — `_measured` chart
|
||||
|
||||
`gemm_mac_utilization_measured.png` currently plots the *theoretical*
|
||||
ideal-pipeline numbers (its footnote says so), only the filename says
|
||||
"measured". This is a known misnomer pending a decision to either repoint
|
||||
its content to the simulator-measured series or retitle it.
|
||||
|
||||
## Consequences
|
||||
|
||||
### Positive
|
||||
|
||||
- GEMM figures are test-generated and CI-guarded, like allreduce.
|
||||
- The heavy sweep stays opt-in, keeping the default test run fast.
|
||||
- Single source for the sweep logic (the script), reused by the slow test.
|
||||
|
||||
### Negative / limitations
|
||||
|
||||
- The theoretical-model constants (D5) are unverified and duplicated.
|
||||
- The `_measured` figure is a misnomer (D6).
|
||||
- `build_overview_slides.py` still renders the GEMM bars natively from
|
||||
`gemm_sweep.json` rather than embedding these PNGs — the deck rewiring to
|
||||
consume the test artifacts is not done.
|
||||
|
||||
## Dependencies
|
||||
|
||||
- **ADR-0013**: verification strategy.
|
||||
- **ADR-0014 / ADR-0042**: PE pipeline + tile-plan generators — the GEMM
|
||||
implementation the sweep measures; D4's stage record counts come from
|
||||
ADR-0042 D2/D3.
|
||||
- **ADR-0033**: latency model — the source the D5 constants should (but do
|
||||
not yet) trace to.
|
||||
- **ADR-0043**: the sibling allreduce evaluation harness.
|
||||
|
||||
## Open questions
|
||||
|
||||
- Reconcile D5 constants against `topology.yaml` / ADR-0033 / ADR-0014 and
|
||||
de-duplicate (one source for the model parameters)?
|
||||
- Resolve the D6 `_measured` naming (repoint content vs. retitle)?
|
||||
- Rewire `build_overview_slides.py` to embed the `gemm_plots/` PNGs instead
|
||||
of native bar-drawing?
|
||||
Reference in New Issue
Block a user