[alxmrs]

🪭 Claude (einfold)

Third pull request of the restacked M1 (design v17, #42), stacked on #44. It ports #38's dense algorithm, its review fix included, into einfold-kernels, onto the new IR.

When it runs. A two-operand fold whose every dimension is an output held only by the first operand (x), summed and held by both (z), or an output held only by the second (y). That is a matrix product out[x, y] = Σ_z a[x, z]·b[z, y], with each role's dimensions flattened into one position. Keys must be non-negative integers that fit in BIGINT, used directly as positions. Otherwise the hash path from #44 runs. fold reports which algorithm ran.

Where SQL and dense arrays differ (design §8.3):

• Duplicate key tuples, and NULL keys: the hash path runs.
• Missing or NULL rows of b are held as 0, but only where no factor of a is NaN or infinite, since 0·∞ is NaN where SQL forms no product.
• Group existence: new with the domain. A domain whose default is +0.0, COALESCE(SUM(…), 0.0) as ddx's gradients write it, makes an unreached group and an all-NULL one look the same, so existence can be ignored. With any other default, or none, b must be complete. A group (x, y) then exists exactly when a has a row at x, and its sum is NULL exactly when every factor of a there is NULL.
• Domain keys that are NULL, or outside the operands' range, match no group under either key equality, since the operands have no NULL keys.

Determinism. As in #38 (spike S24): fixed blocks chosen from the shape alone, 256 output rows per block or 4,096 positions along z, added in block order, with rows visited in position order. Options::threads caps the threads, and the bits don't depend on it.

About 320 non-test lines.

Invariants tested (tests/kernels_vs_sql.rs, extended). Half the cases come from the dense family, chosen by the seed. Each case now runs twice, each run on 1–8 threads and in its own arrival order, and must give the same bits. The dense path must run in at least a quarter of the dense family; it runs in about 35%. The dense family leans to the 0.0 default ddx uses.

🤖 Generated with Claude Code

https://claude.ai/code/session_01M96xSez6PZwTmfaGh2eJgS

Summary by CodeRabbit

• Performance
  • Added a faster processing path for eligible folds, with the existing processing path retained for cases that don’t meet its requirements.
• Reliability
  • Fold results remain consistent across different input orderings and thread counts.

[coderabbitai]

No actionable comments were generated in the recent review.

Recent review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: 7889bb6f-a9b7-4ec5-abba-ff70b20d41a0

Commits

Reviewing files that changed from the base of the PR and between f730efa and 5ef5e63.

Files selected for processing: 3

• crates/einfold-kernels/src/dense.rs
• crates/einfold-kernels/src/lib.rs
• crates/einfold-kernels/tests/kernels_vs_sql.rs

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

Walkthrough

The kernel crate adds a dense implementation for eligible two-operand folds. The public fold function tries this implementation first and uses the hash implementation when dense execution declines the fold. Randomized tests compare results and check deterministic output across thread counts and input orderings.

Changes: Kernel execution

Layer / File(s)	Summary
Dense kernel contract and positional indexing crates/einfold-kernels/src/lib.rs, crates/einfold-kernels/src/dense.rs	The crate adds Options and the dense module. The dense kernel converts eligible integer keys into positions and defines density and block-size rules.
Dense fold calculation crates/einfold-kernels/src/dense.rs	The kernel checks fold eligibility, builds dense storage for operand b, accumulates results in ordered blocks, and maps results to domain batches or output groups.
Algorithm selection and randomized validation crates/einfold-kernels/src/lib.rs, crates/einfold-kernels/tests/kernels_vs_sql.rs	fold tries the dense kernel before the hash kernel and returns the selected algorithm. Randomized tests vary thread counts and input order, compare outputs, and check dense-path coverage.

Priority: Low

Estimated code review effort: 4 (Complex) | ~45 minutes

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant fold
  participant dense as dense::fold
  participant hash as hash::fold
  Caller->>fold: fold with Options
  fold->>dense: attempt dense execution
  alt Dense kernel handles the fold
    dense-->>fold: output batches
  else Dense kernel declines the fold
    dense-->>fold: None
    fold->>hash: execute with configured Host
    hash-->>fold: output batches
  end
  fold-->>Caller: selected Algorithm and output batches

Loading

Possibly related issues

• xqlsystems/einfold#39: This PR adds the positional dense kernel described by the issue, but does not add the provider or host integration.
• xqlsystems/xarray-sql#285: This PR implements dense fold execution, one of the workstreams described by the issue, but does not complete the wider benchmark integration.

Possibly related PRs

• xqlsystems/einfold#44: This PR adds dense execution and fallback to the hash implementation introduced in PR #44.
• xqlsystems/einfold#43: This PR consumes the fold IR types defined in PR #43.
• xqlsystems/einfold#42: This PR implements the dense execution and fallback architecture specified in PR #42.

Merge Risk: Minimal · up to 5ef5e

No merge-blocking issue is established; the change is ready to merge after normal checks.

Security architecture risk: Low · up to 5ef5e

The change remains within an existing library boundary and does not establish a new service or privilege boundary. Eligibility checks constrain the new execution path. No material security regression was established, but downstream deployment exposure remains unknown.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The demonstrated execution scope is the embedding process and its supplied batches. Repository callers establish test and documentation usage, not a deployed network endpoint or a cross-tenant exposure boundary.

Trust Boundaries and Controls

• observed — Caller-selected host comparison rules continue to reach hash execution. Dense execution accepts only integer positional keys, so floating-point key comparison policy does not apply there. The requested thread count is normalized to at least one and capped by the number of dense tasks.

Resilience and Maintainability Implications

• observed — Scoped workers complete before successful result publication, and partial outputs remain local. A worker panic propagates through join rather than becoming a recoverable Result or selecting hash; no concrete ordinary-input worker panic was established.

Pre-merge checks: 5

Check name	Status	Explanation
Title check		The title clearly and concisely describes the main change: adding the dense algorithm to einfold-kernels.
Description check		The description explains what changed, why it changed, fallback behavior, determinism, and test coverage. It is mostly complete, but the required "Closes #" issue reference is empty and the testing de…
Docstring Coverage		Docstring coverage is 91.67% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Documentation-only edits and deletions within surviv…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Description check

Explanation

The description explains what changed, why it changed, fallback behavior, determinism, and test coverage. It is mostly complete, but the required "Closes #" issue reference is empty and the testing details are not under the "How it is tested" heading.

Full details: Docstring Coverage

Explanation

Docstring coverage is 91.67% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 12 functions across 3 files.

Finishing Touches

📝 Generate docstrings

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{Comment @coderabbitai help to get the list of available commands.}

[alxmrs]

🪤 Claude (fuzz adversary)

No bugs found in the dense algorithm, here or in its speedups (#53, #56), after a search aimed at where I expected them: the edges where it changes how it splits work.

dense_metamorphic.rs (#64) draws folds with zero to two dimensions per role, and extents at and around the block sizes: 1, 2, 3, 16, 63–65, 255–257, 2047–2049 and 4095–4097. So outputs fall on both sides of 4,096 cells, row and column blocks end exactly at and one past an edge, and z-blocks split at 4,096. Half the cases have a domain, with keys one past each extent and repeated rows. Factors include NULL, NaN, ∞, -0.0 and ±1e16. Over 1,114 cases (313 on the dense path), every case satisfied:

• [oracle]: the result equals a nested-loop evaluation of SQL's semantics, written in the test with no code shared with the kernels, within 2·(n − 1)·u·Σ|terms|;
• [key-type]: keys cast to Int32, Int16 or UInt32 give identical bits;
• [swap]: swapping the operands (so X and Y, and which side may have holes) gives the same values;
• [hash]: one row of b that joins nothing moves the fold to the hash path without changing any group or value beyond the bound;
• [domain]: each domain row gets exactly its group's bits from the same fold without a domain, or the default;
• [threads]: identical bits for any thread count and batching, empty batches included.

Two observations, neither a bug:

• worth() declines most of the largest edge shapes (dense ran in 313 of 1,114), so the big-block code runs on few of them. If those shapes matter for ddx, the dense path's coverage there rests on the three fixed LARGE shapes in kernels_vs_sql.rs and on this test.
• The absent-row and NaN logic (holes in b only when every factor of a is finite, and 0.0-default domains) held up under [swap]. That's the test most likely to catch a regression in it, since swapping moves the holes to the other side.

[alxmrs]

Some ideas.

[alxmrs]

Does SIMD fit here or in this file?

High-level question: how will this run on CUDA or another accelerator?

[alxmrs]

🪭 Claude (einfold)

SIMD. It already applies here, through the compiler: accumulate's inner loop, o += v * b over two contiguous slices, autovectorizes. Two things limit it:

• The build targets baseline x86-64, which is SSE2, two doubles per instruction. AVX2 has four, and AVX-512 eight. We set no target-cpu, and calling a function compiled for AVX2 from one that isn't needs unsafe (we forbid it), or a crate that does the runtime dispatch for us, such as multiversion or pulp.
• This kernel is mostly memory-bound. Each o += v * b reads b's row and reads and writes o's. In the gate, the folds' time is mostly reading 800,000 rows' keys and factors and writing the output (see Add M1's gate benchmark: ddx's matmul gradient with einfold on and off #49's README). Wider vectors would help the arithmetic, not that.

So SIMD matters most for the compute-bound kernels: M2's fused attention, whose time is in the two matrix products per tile. There I'd like runtime dispatch to AVX2 + FMA, measured against what we have. One catch: FMA rounds a·b + c once, not twice. That's within the error bound and still deterministic on one machine, but the bits differ between machines with and without FMA. I haven't measured target-cpu=native yet: it rebuilds DataFusion too, a second ~10 GB build cache, so I'll do it when M2's kernel is ready.

CUDA and other accelerators. The design (§16, M8) answers it this way: einfold doesn't run on the GPU itself. It produces plans, and GPU hosts (Sirius, NVIDIA's GQE, gpudb) run them. EinFoldExec and these kernels are the CPU reference executor. They define what a host's Fold operator must compute, bit for bit, and its tests carry over as a conformance suite. What carries over to a GPU kernel:

• Dense positions: a fold with dense keys is a batched GEMM or GEMV over arrays built by a scatter, which is cuBLAS's or CUTLASS's job.
• Fixed blocks chosen from the shape: the same thing keeps a GPU kernel deterministic, as long as each output tile has one owner and adds its terms in order.
• Where it can't: a kernel that combines partial sums with atomics needs an order-independent accumulator. Design §8.6 measured a binned one at about 3.5× a plain sum on a GTX 1080 Ti.

If you'd rather have einfold drive a GPU directly, through cudarc or wgpu from EinFoldExec, that's a design change, since principle 1 is "einfold rewrites, hosts execute". It's a reasonable one for the reference executor, and I'd spike it after M2.

[alxmrs]

LGTM