Skip to content

PowerSums / CrossPowerSums: checked_affine — power sums of an affine image from the sums alone - #344

Merged
AdaWorldAPI merged 4 commits into
masterfrom
ccr-b2e415d9-4jfvyk-powersums-affine
Oct 8, 2026
Merged

AdaWorldAPI merged 4 commits into
masterfrom
ccr-b2e415d9-4jfvyk-powersums-affine

Conversation

@AdaWorldAPI

@AdaWorldAPI AdaWorldAPI commented Oct 8, 2026 •

Copy link
Copy Markdown
Owner

What

Two methods next to the existing checked_merge, both reached through ndarray::simd:

  • PowerSums::checked_affine(a, b) returns the power sums of a·x + b.

  • CrossPowerSums::checked_affine([a, b, c, d], [tx, ty]) returns the joint sums of A·(x, y) + t, using

    S' = A·S + n·t
    M' = A·M·Aᵀ + A·S·tᵀ + t·Sᵀ·Aᵀ + n·t·tᵀ
    

Both are exact i128 arithmetic with every step checked. They return None instead of wrapping when an intermediate or a narrowed field (i64 / u128 / i128) overflows.

Chains compose: two calls equal one call with A2·A1 and A2·t1 + t2 whenever both return Some. Failure need not agree, because a step can overflow a field that the composed map never produces. Translating by i64::MAX and then back fails stepwise, but composes to the identity.

Why

The lance-graph algebraic_recipe_probe (D-ART-1, merged in lance-graph #1418) measured three things:

  • composed affine chains over these summaries are bitwise equal to materialise-and-fold;
  • the summary path is 2.5–4× cheaper than materialising the transformed rows;
  • a warm summary transform costs 14 ns, independent of the row count.

Nothing in either repo did this before. I read every impl block on origin/master (36ce111): there are only checked_merge, x() and y(). lance-graph's jc::stats only reads summaries.

Precondition

The group key must not depend on a transformed column. This is documented, and a test pins it: with the key x ≥ 0, shifting x by 20 moves rows between groups, so per-group transforms differ from the truth.

Evidence

  • Equivalence: 400 random cases match folding the transformed rows through masked_group_cross_power_sums_i32. Each case has 1–300 rows, coefficients in [−5, 5] (reflections included) and translations in [−100, 100], under random masks and 3 groups. The univariate form is checked against the x marginal.

  • Composition: two calls equal the composed call when both succeed. The asymmetric-failure case is pinned. The identity map leaves the sums unchanged, and the zero map leaves only the translation.

  • Overflow: refused in every narrowed field.

  • Doctests: 2.

  • Disable runs, each red then restored:

    • the 2·tx·(A·S)x term dropped;
    • the ty·(A·S)x cross term dropped;
    • n·tx dropped;
    • the univariate 2ab·Σx term dropped;
    • as i64 instead of checked narrowing, separately for sum_x and sum_y.

    The first overflow fixture was vacuous: its translation already overflowed the square, so the i64 narrowing never ran. It was rebuilt so that only the narrowing can refuse.

  • Full run: cargo test -p ndarray --lib passed 2547 (32 ignored), with debug 0. clippy -D warnings and fmt are clean.

This is scalar code with no target-specific paths. It is a struct method on a summary type, like checked_merge, not a new SIMD primitive.

CI note: the realization/nightly × x86_64 failure on the first head was a SIGILL inside the cached paste proc-macro. It was not caused by this change; see the comment below for the cause and a proposed workflow fix.

🤖 Generated with Claude Code

https://claude.ai/code/session_01EFw2WdKr1oxvaKCJC2ua2R

claude added 3 commits October 8, 2026 20:09
…image from the sums alone

Exact integer transform of the degree-≤2 sufficient statistics:
S' = A·S + n·t, M' = A·M·Aᵀ + A·S·tᵀ + t·Sᵀ·Aᵀ + n·t·tᵀ (and the
univariate a·x + b). Checked i128 arithmetic: None instead of wrapping.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01EFw2WdKr1oxvaKCJC2ua2R
@coderabbitai

coderabbitai Bot commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Essentials
  • Run ID: be05478e-160a-47aa-9650-f98c03bc7af8
📥 Commits

Reviewing files that changed from the base of the PR and between 36ce111 and 25d009c.

📒 Files selected for processing (2)
  • .claude/blackboard.md
  • src/simd_masking_ops.rs

Included review availability: This review used your included allowance. 0 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 1 review per hour.


📝 Walkthrough

Walkthrough

Adds checked affine transforms for univariate and two-variable power-sum summaries. The transforms use checked arithmetic and return None when a result cannot fit its destination type. Tests cover transformed values, composition, overflow, and grouping-key behavior.

Changes

Checked affine power-sum transforms

Layer / File(s) Summary
Transform methods
src/simd_masking_ops.rs
Adds PowerSums::checked_affine and CrossPowerSums::checked_affine. Both use checked i128 arithmetic and return None when intermediate arithmetic or result conversion fails.
Transform validation and notes
src/simd_masking_ops.rs, .claude/blackboard.md
Adds randomized comparisons with transformed rows, composition and identity checks, overflow-boundary tests, and a test of the group-key precondition. The blackboard records the methods and validation notes.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Feature

Suggested reviewers: claude

Merge Risk: ⚪ Minimal · up to 25d00

The affine transforms retain their stated overflow behavior. No issue identified here needs resolution before merging.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 90.91% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 11 functions across 1 files. (1 skipped: 1 …
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the main change: adding checked_affine operations to PowerSums and CrossPowerSums for affine images.
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

A rabbit checks each sum with care
Through affine turns that numbers share
If bounds are crossed, it says “not so”
The tests trace where the values go
Then hops away, its sums in tow

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

@AdaWorldAPI
AdaWorldAPI marked this pull request as ready for review October 8, 2026 20:26

Copy link
Copy Markdown
Owner Author

realization/nightly × x86_64 failure: not this PR's change.

What failed. rustc crashed with SIGILL while compiling the ndarray lib test, inside the paste proc-macro, before any test ran:

target/debug/build/paste/7c6b8732f67511fc/out/libpaste-….so(+0x3595f)
error: rustc interrupted by SIGILL

The same matrix passed on master at 36ce111. This PR only adds two scalar methods and their tests in simd_masking_ops.rs.

Why. The nightly row builds under the repo's target-cpu=native default and caches with the fixed key key: nightly. The workflow's own comment in simd-matrix.yaml records that the ubuntu-latest pool is heterogeneous: in one run, the nightly job got avx512f=TRUE and host-native got avx512f=FALSE. A proc-macro .so compiled natively on an AVX-512 runner is restored on a runner without AVX-512, and rustc dies loading it.

No fix exists yet. Proposed patch (not in this PR, to keep it narrow): key the cache by the runner's CPU flags.

      - name: cpu flags for the cache key
        id: cpu
        run: echo "flags=$(grep -m1 '^flags' /proc/cpuinfo | sha256sum | cut -c1-12)" >> "$GITHUB_OUTPUT"
      - uses: Swatinem/rust-cache@v2
        with:
          key: nightly-${{ steps.cpu.outputs.flags }}

Any other job that relies on the native default and shares a cache key across runners has the same exposure.

I'll re-run the failed job once when the rest of the run finishes; the first attempt was refused with "workflow is already running".


Generated by Claude Code

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 25d009c034

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/simd_masking_ops.rs Outdated
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-08T20:29:13.642461Z 25d009c Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@AdaWorldAPI
AdaWorldAPI merged commit 334dee5 into master Oct 8, 2026
25 checks passed
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.

2 participants