Repository navigation
PowerSums / CrossPowerSums: checked_affine — power sums of an affine image from the sums alone - #344
Conversation
…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
Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01EFw2WdKr1oxvaKCJC2ua2R
Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01EFw2WdKr1oxvaKCJC2ua2R
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (2)
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. 📝 WalkthroughWalkthroughAdds checked affine transforms for univariate and two-variable power-sum summaries. The transforms use checked arithmetic and return ChangesChecked affine power-sum transforms
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Feature Suggested reviewers: Merge Risk: ⚪ Minimal · up to The affine transforms retain their stated overflow behavior. No issue identified here needs resolution before merging. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
A rabbit checks each sum with care Comment |
|
What failed. rustc crashed with The same matrix passed on master at 36ce111. This PR only adds two scalar methods and their tests in Why. The nightly row builds under the repo's 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 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 |
There was a problem hiding this comment.
💡 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".
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
… where only the stepwise form fails Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01EFw2WdKr1oxvaKCJC2ua2R
What
Two methods next to the existing
checked_merge, both reached throughndarray::simd:PowerSums::checked_affine(a, b)returns the power sums ofa·x + b.CrossPowerSums::checked_affine([a, b, c, d], [tx, ty])returns the joint sums ofA·(x, y) + t, usingBoth are exact
i128arithmetic with every step checked. They returnNoneinstead of wrapping when an intermediate or a narrowed field (i64/u128/i128) overflows.Chains compose: two calls equal one call with
A2·A1andA2·t1 + t2whenever both returnSome. Failure need not agree, because a step can overflow a field that the composed map never produces. Translating byi64::MAXand 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:Nothing in either repo did this before. I read every
implblock onorigin/master(36ce111): there are onlychecked_merge,x()andy(). lance-graph'sjc::statsonly 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:
2·tx·(A·S)xterm dropped;ty·(A·S)xcross term dropped;n·txdropped;2ab·Σxterm dropped;as i64instead of checked narrowing, separately forsum_xandsum_y.The first overflow fixture was vacuous: its translation already overflowed the square, so the
i64narrowing never ran. It was rebuilt so that only the narrowing can refuse.Full run:
cargo test -p ndarray --libpassed 2547 (32 ignored), with debug 0. clippy-D warningsand 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_64failure on the first head was aSIGILLinside the cachedpasteproc-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