Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
33 changes: 30 additions & 3 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,33 @@ rule files the task needs.
* Every experiment carries one focused test. Run it locally; the fork CI gate is
a build, not the full upstream suite (faer's test dependencies are heavy).
The inherited upstream workflows still run their own heavier jobs.
* When an experiment uncovers a likely upstream bug, report it to the fork
maintainer and get explicit permission before preparing or opening an upstream
issue or PR.
* Upstream feedback is **deferred at this stage**: do not prepare or open an
upstream issue or PR and do not write to an upstream tracker, even for a defect
that is clearly upstream's. Measure it, record it in `docs/worklogs/` with the
upstream revision that was measured, and leave the decision to the maintainer.
Explicit permission is required before anything more than a record here.

## Work Logs And Review Intent

Follow the shared [Work Logs And Design
Records](https://github.com/tensor4all/tensor4all-agent-rules/blob/main/rules/common/repository.md#work-logs-and-design-records)
policy and the [local format](docs/worklogs/README.md). Keep a record for a
nontrivial multi-phase change, a non-obvious design choice, or a performance
experiment; small fixes and AI assistance alone do not require one. One file per
change theme under `docs/worklogs/`, linked from its PR, and read the linked
record when reviewing the decisions it explains.

Every performance experiment in this fork needs one, because the fork's whole
purpose is to be a measurable delta against upstream. `PATCHSET.md` says what the
fork carries today; the record says why — the isolation, the candidates that were
tried and dropped *with the measurement that dropped them*, and what the result
establishes. It is how the difference from upstream and the course of an
optimisation stay followable by someone who was not in the session, and how a
dead end that cost a day is not tried twice.

Keep complete performance results in the linked evidence (the campaigns'
published pages) rather than copying them into the record, and say plainly when a
number is a scan rather than a paired comparison, or when something is a
hypothesis rather than a measurement. A lead nobody has measured yet goes under
"what remains unverified" in the record of the experiment that uncovered it — the
ledger lists what the fork *carries*, not what it suspects.
2 changes: 1 addition & 1 deletion PATCHSET.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ upstream `faer`.
| EVD eigenvector normalization + overflow-safe back substitution | `evd_real`/`evd_cplx` return eigenvectors scaled to unit 2-norm (a conjugate pair is scaled as one complex vector, matching `xGEEV`), and the back substitution is rescaled when an entry would exceed `sqrt(max)` instead of overflowing to `NaN`. A finite input that still overflows, or a Schur iteration that does not converge, returns `NoConvergence`. | KEEP | tenferro-rs#2131 | Focused test `eig_2131_tests` (f32/f64 Jordan block, conjugate pair with left and right vectors, complex input, sequential + recursive solver). Normalization runs at the `evd_real`/`evd_cplx` boundary, so `operator/eigen` keeps its previous Ritz-vector scaling while still getting the overflow-safe finite Schur eigenvectors. |
| Unpivoted QR column skip | The skip lived only in `qr_in_place_unblocked`; `qr_in_place_blocked` has no threshold of its own and recurses into that core, so removing it there covers the tall and wide paths at once. The reflector is applied and the row advances for every nonzero column, matching LAPACK; the tall/wide `svd` path goes through it too. | KEEP | tlinalg-rs#28, tensor4all-rs#836 | Adopted. Tests: `test_near_dependent_columns_are_not_skipped` (tall `m x 3`, m=100/1000/8192) and `test_near_dependent_rows_are_not_skipped` (its transpose, wide `3 x n`, n=100/1000/8192), both at `<=1e-13`; measured 1e-16 to 5e-15. The wide fixture was already accurate before the change: at m=3 the removed threshold `16 (m-k) eps ||column||` is ~1e-14 and never tripped it (a wide skip's relative reconstruction error is ~`16 eps sqrt(m) sqrt(m/n)`), so the wide path needed no separate treatment — the same core change covers it. tlinalg's reported wide `3.33e-7` is a reconstruction-indexing bug in that test (entry `(i,j)` written to flat `j+3i` while the factor is column-major `i+3j`); faer's factors for that matrix are accurate to ~4e-16. Existing columns are unchanged; formerly skipped nonzero columns now pay reflector/update flops (the threshold norm bookkeeping is removed). No existing benchmark suite was run. Release smoke, 3-run mean, before→after: 100000×8 2.984→2.986 ms; 1000×1000 19.793→19.921 ms. |
| Row-split reduction in the compact-WY factor's deep half | `upgrade_householder_factor`'s flat branch (`prev_block_size < 8`) builds `T = diag(tau) + strict_upper(V^H V)`, and its second product reduces over every row below the basis' diagonal with an `n x n` output -- the same shape as the block apply's reduction, and serial for the same reason: 0.271 ms at eight threads at `m = 4096, n = 16`, against 0.040 with the rows split (6.7x; 2.96x at `m = 512`, 4.59x at `m = 1024`). The partials here are `n x n` each, tiny against the basis, and only the strict upper triangle is stored, so the split's overhead is negligible and no depth-against-width gate is needed -- the thread and work floors are enough. End to end, 8T pool: `householder_factor` 4096x256 b1 c64 15.72 -> 13.38 ms (f64 5.63 -> 4.84), `qr` c64 42.76 -> 37.00, and the 512x512 and 256x256 cells improve too (4.49 -> 4.14, 0.98 -> 0.92) with every 1T row unchanged. Test: `threaded_upgrade_matches_the_sequential_one` (4096x16, 2048x8, 4096x1, and 64x8 below the floors). | KEEP | tlinalg-rs#46 | Costs one `n * n * threads` buffer per call; the same scratch note as the row below. Own code: no OpenBLAS or MKL source. |
| No parallel region below the compact factor's own floor | `upgrade_householder_factor` threads its `V^H V` products, and its `deep` predicate already states when the reduction has the shape to pay for that (deeper than `2 * n`, at least one row per thread, over the `8 * 1024` work floor). Below that floor the products still opened a region each, and a blocked QR calls this function once per block -- at a block size the recursion halves every level -- so a `128 x 128` QR paid the fork-join cost of `n`-wide reductions about fifty times for work that same floor calls too small to split: the threaded call measured 4.2-4.6x *slower* than the sequential one at `n = 128` (1.05 vs 0.23 ms), 2.1x at 256, 1.3x at 384, and only broke even at 512 (0.96). Asking the floor once, before opening anything, gives 0.99 at 128, 0.79 at 256, 0.60 at 384 and 0.44 at 512 (f64), 0.72 / 0.50 / 0.30 (c64), and no region is opened below the floor at all. The tall shapes the ledger's other rows serve are untouched (`4096x256` f64 6.35 -> 6.33 ms, c64 16.67 -> 16.67), because their reductions are above the floor; `1024x64` goes from 1.4x slower than sequential to parity. No new constant: the predicate is the one the products below already use. | KEEP | tlinalg-rs#46 | Measured with an isolated driver on the call tlinalg's `qr` family makes (`qr_in_place` f64/c64, 128-768 square, 4096x256 / 1024x64 / 512x512), CPUs 4-11, best of 7. |
| No parallel region below the compact factor's own floor | `upgrade_householder_factor` threads its `V^H V` products, and its `deep` predicate already states when the reduction has the shape to pay for that (deeper than `2 * n`, at least one row per thread, over the `8 * 1024` work floor). Below that floor the products still opened a region each, and a blocked QR calls this function once per block -- at a block size the recursion halves every level -- so a `128 x 128` QR paid the fork-join cost of `n`-wide reductions about fifty times for work that same floor calls too small to split: the threaded call measured 4.2-4.6x *slower* than the sequential one at `n = 128` (1.05 vs 0.23 ms), 2.1x at 256, 1.3x at 384, and only broke even at 512 (0.96). Asking the floor once, before opening anything, gives 0.99 at 128, 0.79 at 256, 0.60 at 384 and 0.44 at 512 (f64), 0.72 / 0.50 / 0.30 (c64), and no region is opened below the floor at all. The tall shapes the ledger's other rows serve are untouched (`4096x256` f64 6.35 -> 6.33 ms, c64 16.67 -> 16.67), because their reductions are above the floor; `1024x64` goes from 1.4x slower than sequential to parity. No new constant: the predicate is the one the products below already use. | KEEP | tlinalg-rs#46 | Measured with an isolated driver on the call tlinalg's `qr` family makes (`qr_in_place` f64/c64, 128-768 square, 4096x256 / 1024x64 / 512x512), CPUs 4-11, best of 7. Why, and the candidates that lost: `docs/worklogs/2026-10-14-compact-factor-region-floor.md`. |
| Row-split reduction in the block Householder apply | `tmp = V^H C` has an `N x K` output and reduces over every row of the target. With `N` small and the target long, the products inside the generic path were one serial reduction inside one task, so the column split (capped at `min(par, K, 4)`) could not use the depth: 4096x16 basis against a 4096x240 target measured 2.03 ms at 8T where OpenBLAS's `?gemm` on the same shape is 0.53-0.71. Splitting the rows is a split along the depth -- it adds `threads` partials of `N x K` to write, read and add -- so it is taken only while the reduction is *deeper than it is wide*: a sweep over `M` against `K` at eight threads puts the crossover at `M ~ 2 K` (four times deeper gains 3-4x; as wide as it is deep, which is the 512x512 trailing update at `M = 512`, `K = 496`, loses for the partials it pays for). With that gate, 8T pool, `householder_factor` 4096x256 b1 c64 17.44 -> 15.72 ms (f64 5.83 -> 5.54), `qr` c64 46.75 -> 42.68, `svd_thin` c64 77.11 -> 68.16, and the 512x512 and 256x256 cells stay at baseline; every 1T row is unchanged. Tests: `threaded_panel_matches_the_sequential_one` (2048x4, 4096x16, and two shapes below the thresholds) plus the two existing cases. | KEEP | tlinalg-rs#46 | Costs one `N * K * threads` buffer per call on the new path; the public scratch function is unchanged, so upstreaming this should carry it there. Own code: no OpenBLAS or MKL source is used or adapted. |
| Threaded rank-one Householder apply | The `N == 1` arm of `apply_block_householder_on_the_left_in_place_generic` is a sequential rank-one update per target column and it never read `par`. A tall blocked QR's panel is made almost entirely of those updates, so the panel was the serial part of it: at 4096x256 c64 the panel is 5.96 ms at 1T and 6.31 ms at 8T, against OpenBLAS's `?gemv`/`?geru` panel at 5.97 -> 2.58 ms on the same eight cores. When the caller asks for threads and the update is at least 8192 elements, the arm now falls through to the generic path, which partitions the target's columns and threads its two products. Through tlinalg's compact factor (`householder_factor`), pool row, 4T / 8T: 4096x256 f64 6.73 -> 5.77 and 6.67 -> 5.83 ms, 4096x256 c64 21.42 -> 18.75 and 19.50 -> 17.44 ms (11-14%), with 512x512, 128x128, `qr`, `svd_thin` and every 1T row unchanged within noise. Tests: `threaded_rank_one_matches_the_sequential_one` (f64 and c64; 2048x8, 16384x1, 64x8) and `rank_one_negates_its_own_reflector_column`. | KEEP | tlinalg-rs#46 | Own code: no OpenBLAS or MKL source is used or adapted, their timings are only the target. |
| Tall compact-WY trailing update scaling | The blocked compact QR applies its panel through `apply_block_householder_transpose_on_the_left_in_place_with_conj`, whose generic path partitions the target's columns with `n_tasks = min(parallelism_degree(par), K, 4)` and `rayon(par / n_tasks)` inside each. On a tall DRAM-bound panel (4096x16 basis, 4096x240 target, eight physical Zen 5c cores sharing 8 MiB of L3, 16.8 MB target) the update is 8.72 ms at `Par::Seq` and 2.45 ms at `Par::rayon(8)` = 3.56x, where OpenBLAS reaches 5.5x on the same cores for the same factorization; splitting the columns by hand saturates at 3.87x, so the ceiling is the kernel, not the decomposition. Candidates: partition rows for the `C -= V tmp` half, which is row-parallel and has the long dimension (rows >> columns, while the column partition caps at 4), and the DRAM-bound path of the inner matmuls. | TODO | tlinalg-rs#46 | Reproducer and the phase split: tlinalg-benchmark `docs/worklog.md`, 2026-10-12. Drives no code change in tlinalg until the kernel moves. |
Expand Down
78 changes: 78 additions & 0 deletions docs/worklogs/2026-10-14-compact-factor-region-floor.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
# The compact factor's parallel region floor

PR: [tensor4all/faer#9](https://github.com/tensor4all/faer/pull/9) (merged as
`04542fae`). Ledger row: `No parallel region below the compact factor's own floor`.
Consumer: [tensor4all/tlinalg-rs#56](https://github.com/tensor4all/tlinalg-rs/pull/56).

## Decisions

- **Ask the floor once, before opening anything.** `upgrade_householder_factor`
threads its `V^H V` products, and its own `deep` predicate already states when that
reduction has the shape to pay for it: deeper than `2 * n`, at least one row per
thread, over the `8 * 1024` work floor. Below that floor the products still opened a
rayon region each — and a blocked factorisation calls this function **once per
block**, at a block size the recursion halves every level. The change is: below the
floor the call takes `Par::Seq`, above it the caller's `par` is forwarded exactly as
before. Thirteen lines, no new constant, nothing computed differently.
- **Why it was worth changing.** A threaded 128x128 QR measured **4.2-4.6x slower**
than the sequential call on the same input — the fork-join cost of `n`-wide
reductions paid about fifty times for work the same floor calls too small to split.
Isolation: `qr_in_place` (the call tlinalg's `qr` family makes), f64/c64,
CPUs 4-11, best of 7. `Par::rayon(8)` before → after, ms: 128x128 1.05 → **0.235**,
256x256 2.46 → **0.917**, 512x512 4.59 → **2.85**, 1024x64 0.65 → **0.466**,
4096x256 6.32 → 6.31 (unchanged, its reduction is above the floor).
At tlinalg level the pair of campaign pages `bf88c0a6e568-mkl` → `7fdba8a53ba3-mkl`
gives `faer-pool` medians of **0.576** (f64) / **0.742** (c64) for `qr`, with the
sequential rows and the unchanged MKL binary at 1.00 as the floor; `qr` 128x128 b128
c64 goes 121.77 → **40.79** ms against MKL's 39.29 (3.10x behind → **1.04x**).
- **Rejected: gate the per-block apply on the trailing area**
(`(m - row) * (n - col) < par_threshold`, the rule the column-pivoted sibling uses
for itself). It fixes the small and medium shapes and **regresses the tall ones**:
4096x256 f64 6.36 → 6.82 ms, c64 16.53 → 18.23. A tall block update is long in depth
and small in area, so an area threshold sends real wins to `Par::Seq` — the same
reason the fork's own block-apply gate is a depth-against-width inequality and not
an area.
- **Rejected: force the `n x n x n` head product of the `prev_block_size < 8` branch to
`Par::Seq`.** 512x512 f64 2.49 → 3.43 ms (38% worse), neutral elsewhere.
- **Interacts with** the fork's rows `Row-split reduction in the compact-WY factor's
deep half` and `Row-split reduction in the block Householder apply`: those splits are
*inside* this function and stay exactly as measured; this change is the gate in front
of them, so a shape that used to open one region now opens none, and a shape above the
floor behaves as before.
- **Upstream.** Upstream's `upgrade_householder_factor` forwards `par` to the same
products and the same `join_raw` recursion with no floor at all, so the pattern is
upstream's, not the fork's; upstream's `qr/no_pivoting/factor.rs` likewise declares a
`par_threshold` that nothing reads. The fork cannot port the fix back verbatim, because
the floor it reuses is itself a fork addition from the row-split work.
*What was measured* is the published upstream release, `faer-0.24.4` from crates.io
(linked by path from the registry source), with the same driver, host and pins as the
fork's numbers on this page. *What was read* is the GitHub mirror clone
`sarah-ek/faer-rs` at `ddbd2ae`; the source of truth is Codeberg, so treat the line
numbers there as the mirror's, not upstream's. Upstream feedback is deferred for now
(see `AGENTS.md`).

## Verification conclusions and constraints

- Establishes: a threaded no-pivot QR is at worst at parity with the sequential call and
wins from `n = 224` upward (0.88x at 224, 0.37x at 768), and the tall shapes the
ledger's other rows serve are unchanged within 1.2%.
- Limits: the tlinalg-level pair is a **scan**, not a paired comparison (`--aa 1`), so no
pooled cell is claimed; one pooled cell moved the wrong way and is not attributable
(`householder_apply` 32x32 b128 f64 reads 0.246 → 0.472 ms, a shape that fans the batch
out over lanes with a sequential inner call and therefore does not contain this change
at all). One pre-existing failure in the fork is unrelated:
`sparse::linalg::qr::tests::test_numeric_simplicial_qr_1_transpose` panics in
`triangular.rs:990` on the parent commit too.
- Remains unverified, and these are hypotheses only — each was found by reading the
source while measuring this change, none has an isolated measurement yet:
- `svd/bidiag.rs`: `bidiag_in_place_scratch` discards its hint (`_ = par; _ =
params;`) and `BidiagParams::par_threshold` is never read, so the bidiagonalisation
is sequential however the caller calls. Consistent with `svd_values` being flat from
1T to 8T (128x128 b1 f64 1.412 → 1.433 ms against MKL's 0.704 → 0.678).
- `evd/tridiag.rs` uses no parallel degree at all, and `eigh` is flat for the same
shape (1.26 → 1.24 ms against MKL's 1.08 → 0.586).
- `lu/partial_pivoting/factor.rs` is *not* missing its hint (panel solve, trailing
matmul and recursion all take `par`, and its `par_threshold` is read), but
`lu_factor` 512x512 b1 f64 scales only 1.85x from 1T to 8T where MKL reaches 4.9x.
This one needs a phase profile before any threshold is touched; the `M ~ 2K` episode
in this ledger is the lesson about choosing a constant from the wrong cell.
31 changes: 31 additions & 0 deletions docs/worklogs/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
# Work Logs

Keep lightweight records for nontrivial multi-phase changes, non-obvious design
choices, or performance experiments, following the shared
[Work Logs And Design Records policy](https://github.com/tensor4all/tensor4all-agent-rules/blob/main/rules/common/repository.md#work-logs-and-design-records).
Small fixes and AI assistance alone do not require one.

One file per change theme: `YYYY-MM-DD-<slug>.md`, linked from the PR that carries
the change. Start with a few lines:

```markdown
# <Change theme>

## Decisions
- Chosen approach and why; important rejected alternatives, if any.

## Verification conclusions and constraints
- What the final state establishes, and what remains unverified or limited.
- Links to detailed evidence or special reproduction conditions, when needed.
```

A performance experiment in this fork has one extra duty: an upstream revision was
measured, or it was not, and the record says which. `PATCHSET.md` is the ledger of
what the fork carries; a record here explains why it carries it and what lost along
the way.

Do not include commands run (Cargo or otherwise), files read, agent activity, or
edit/review chronology. Mention failed attempts only when they explain a choice or
limitation. Keep complete performance results in the linked evidence rather than
copying them here. Update the same record when its conclusions change, not after
every correction; do not rewrite historical records for this format.
Loading