Skip to content

Add pruning aggregate functions#8025

Open
gatesn wants to merge 7 commits into
developfrom
ngates/stats-7707/min-max-aggregate-fns
Open

Add pruning aggregate functions#8025
gatesn wants to merge 7 commits into
developfrom
ngates/stats-7707/min-max-aggregate-fns

Conversation

@gatesn
Copy link
Copy Markdown
Contributor

@gatesn gatesn commented May 19, 2026

Adds the pruning aggregate functions used by stats rewrites, bounded string/binary min-max variants, and the aggregate compatibility relation for matching stored bounds to requested bounds.

gatesn added 6 commits May 19, 2026 16:21
Signed-off-by: Nicholas Gates <nick@nickgates.com>
Signed-off-by: Nicholas Gates <nick@nickgates.com>
Signed-off-by: Nicholas Gates <nick@nickgates.com>
Signed-off-by: Nicholas Gates <nick@nickgates.com>
Signed-off-by: Nicholas Gates <nick@nickgates.com>
Signed-off-by: Nicholas Gates <nick@nickgates.com>
@gatesn gatesn added the changelog/feature A new feature label May 19, 2026
@gatesn gatesn mentioned this pull request May 19, 2026
42 tasks
@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented May 19, 2026

Merging this PR will degrade performance by 13.9%

⚠️ Unknown Walltime execution environment detected

Using the Walltime instrument on standard Hosted Runners will lead to inconsistent data.

For the most accurate results, we recommend using CodSpeed Macro Runners: bare-metal machines fine-tuned for performance measurement consistency.

❌ 2 regressed benchmarks
✅ 1235 untouched benchmarks

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
Simulation new_alp_prim_test_between[f32, 16384] 103.9 µs 118.2 µs -12.04%
Simulation new_alp_prim_test_between[f32, 32768] 153.3 µs 181.9 µs -15.72%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.


Comparing ngates/stats-7707/min-max-aggregate-fns (d99cb3a) with develop (ba5064a)

Open in CodSpeed

@gatesn
Copy link
Copy Markdown
Contributor Author

gatesn commented May 19, 2026

Review comments carried over from auto-closed #7934

#7934 was closed/merged into the stack branch while the stack was being repaired. Copying the review notes here so they remain visible on the replacement PR.

Review summaries

Inline review comments

Bot review findings worth preserving

From Claude first pass:

  • Add/confirm tests and docs for empty float AllNan / AllNonNan vacuous semantics.
  • Investigate whether the Min/Max min_max() double-compute is related to CodSpeed regressions.
  • The StatsRewriteRule / session #[allow(dead_code)] plumbing is probably foundation work, but should be documented or exercised when active.

From Claude second pass:

  • BoundedMax may conflate Empty and Unknown in cross-shard merging; keep or add tests covering combine_partials behavior.
  • try_accumulate always short-circuits for the boolean pruning aggregates; document the relationship with accumulate to avoid drift.
  • Non-float AllNan / AllNonNan guards in stat_array were called out as potentially dead or inconsistent depending on intended support.
  • Add comments to bounded min/max noting they delegate to min_max() and compute the unused bound for now.

Signed-off-by: Nicholas Gates <nick@nickgates.com>
Base automatically changed from ngates/stats-7707/typed-stats-rewrite-api to develop May 19, 2026 21:19
@gatesn gatesn marked this pull request as ready for review May 19, 2026 21:20
@gatesn gatesn requested a review from robert3005 May 19, 2026 21:20
@gatesn gatesn enabled auto-merge (squash) May 19, 2026 21:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog/feature A new feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant