hdr: recover moments_u32 / MomentsU32 and Cascade::observe_batch (HDR-only harvest from #325) - #327
Conversation
…325 Mechanical recovery of the HDR/popcount hunks that #326 reverted, applied unchanged: - src/hpc/statistics.rs: MomentsU32 (exact integer n / Σx / Σx², order- independent merge, exact u128 variance) and moments_u32 (U64x8 lanes, split 32-bit squares, 2^28-chunk drain) with their tests. - src/hpc/cascade.rs: Cascade::observe_batch (Chan-Golub-LeVeque merge of batch moments into the rolling floor, per-batch drift alert) with its three tests. - src/simd.rs: the moments_u32 / MomentsU32 re-export line only. Everything else from #325 stays reverted. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019HnekoM1EidTwQLS3oFVFm
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Essentials Run ID: 📒 Files selected for processing (2)
Included review availability: 1 review is currently available. Your included PR review attempts over the past 7 days set your current allowance at 3 reviews per hour. 📝 WalkthroughWalkthroughThe change adds public ChangesInteger Moments
Cascade Alert Threshold
Estimated code review effort: 3 (Moderate) | ~25 minutes Suggested reviewers: Merge Risk: 🔵 Low · up to Very large sparse aggregates can report zero variance incorrectly. The impact is narrow and does not appear to block merging. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
A rabbit counts each sum with care Comment |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_e2c61841-6f3b-4707-882a-675485386313) |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_c5b89a13-8743-4d84-b23d-95fb501e2fc1) |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1c31868327
ℹ️ 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".
Two defects in the code recovered from #325, both found in review: - observe_batch gated its drift alert on `old_n > 10`, while observe counts the new element first and tests `> 10`, i.e. >= 10 prior. At exactly 10 prior observations, observe_batch(&[x]) stayed silent where observe(x) alerted. Gate is now `old_n >= 10`. - MomentsU32::variance's fallback (when n*sum_sq or sum^2 overflows u128) subtracted two ~2^64-scale f64 values and could cancel the variance to 0 (2^33 values split between u32::MAX and u32::MAX-1: 0.0 instead of 0.25). It now centres on q = floor(sum/n) exactly in u128 first, so the float step works on variance-sized quantities. Each fix has a regression test that failed before the change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019HnekoM1EidTwQLS3oFVFm
Adds hpc::rolling_floor, the adaptive half of the HDR exposure meter, harvested from lance-graph graph/blasgraph/hdr.rs (the behavioural reference). Constants, cadence, drift rule and reset semantics carry over unchanged; the arithmetic underneath is #327's exact MomentsU32 instead of the reference's approximate integer Welford. - isqrt_u32: the reference integer Newton square root. - ReservoirU32: deterministic Algorithm-R reservoir (splitmix replacement hash keyed on the observation count), u32 empirical quantile, Pearson second skewness, kurtosis x100. Order-defined; no merge law. - RollingFloor: calibrated (mu, sigma), sigma floors mu - k sigma, empirical floors at the reference percentiles, shape evaluation every 1000 observations after the first 1000 (reservoir >= 100), normal iff |skew| < 2 and 200 < kurt < 500, drift at |dmu| > sigma/2 or |dsigma| > sigma/4, recalibration resets moments, reservoir and shape. - observe_batch folds moments_u32 between checkpoints and stops after the first shift, so any batching reproduces the scalar observe/recalibrate loop exactly. - MomentsU32::observe: scalar fold equal to merging a singleton batch. A test keeps the legacy integer Welford as an oracle: across 95 checkpoints on five streams the (mu, sigma) the drift rule sees are identical. examples/hdr_rolling_floor_bench.rs separates the hot path from the periodic shape path. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019HnekoM1EidTwQLS3oFVFm
2 participants
What
A recovery of the HDR/popcount hunks that #326 reverted. Nothing else from #325 returns.
The first commit,
1c31868, applies the #325 hunks unchanged: its diffs of the two recovered files are byte-identical to #325's. The second commit,92adbd4, fixes two defects that Codex review found in that recovered code (see Review fixes), so the final diff is no longer byte-identical to #325.#325 hunks recovered
src/hpc/statistics.rs@@ -360,3 +360,203MomentsU32(exact integer n / Σx / Σx², an exact order-independent merge, exact u128 variance) andmoments_u32(U64x8lanes, split 32-bit squares, 2²⁸-chunk drain), plus testssrc/hpc/cascade.rs@@ -208,6 +208,56Cascade::observe_batch(Chan–Golub–LeVeque merge, per-batch drift alert)src/hpc/cascade.rs@@ -768,6 +818,76observe_batchtestssrc/simd.rs@@ -617,6 +617,8moments_u32/MomentsU32re-export line only; the hunk was splitReview fixes (
92adbd4)observe_batchalert gate. The recovered code testedold_n > 10.observecounts the new element before it tests> 10, so it alerts with exactly 10 prior observations.observe_batch(&[x])therefore stayed silent whereobserve(x)alerted. The gate is nowold_n >= 10. Test:observe_batch_singleton_matches_observe_alert_gate.MomentsU32::variancefallback. Whenn·Σx²or(Σx)²overflowed u128, the fallback subtracted two floats of magnitude around 2⁶⁴ and could cancel the variance to 0. Example: 2³³ values split betweenu32::MAXandu32::MAX - 1returned 0.0 instead of 0.25. The fix centres onq = ⌊Σx/n⌋exactly in u128 first. Test:variance_fallback_does_not_cancel.Both tests failed before the fix and pass after it.
#325 hunks intentionally left reverted
src/hpc/zspace.rs(whole file):ZGamma,ln_det,fisher_z,hyperbolic_depth,hamming_null_z, and the golden digests.src/hpc/mod.rs@@ -25,6 +25,9: thepub mod zspaceline.src/simd.rs: thezspacere-export line.src/hpc/reliability.rs, all 4 hunks: the*_zaccessors and their doc and test.crates/wasm-simd-parity/src/lib.rs, both hunks: theZGammagolden check.Validation
ZGamma|zspace|fisher|hyperbolic|similarity_z|paletteappear in the added lines. The one whole-file hit,cascade.rs"Berry-Esseen Fisher efficiency", is an existingmasterdoc line.hpc::statistics+hpc::cascade): 35/35 on native,config-v3(AVX2),config-v4(AVX-512), and aarch64 under qemu.clippy -D warningsandfmtare clean.🤖 Generated with Claude Code
https://claude.ai/code/session_019HnekoM1EidTwQLS3oFVFm
Summary by CodeRabbit