Skip sum updates once SUM_WITH_OVERFLOW flag is set

[PointKernel]

Description

Originally proposed by @shrshi at #19403 (comment) and follows the subsequent discussions in #22465 (comment)

This PR updates both groupby and reduce SUM_WITH_OVERFLOW so that once a group's (or the reduction's) overflow flag is set, the sum is no longer maintained: groupby short-circuits subsequent atomic adds for that group, and reduce stops zeroing the wrapped result. The sum child is documented as unspecified on overflow; the boolean flag is the sole source of truth.

Note that this is an ABI-breaking change, since the expected overflowed sum value is no longer the wrapped result. However, this should not impact downstream users, as relying on overflowed sum results is not considered valid behavior.

Checklist

• I am familiar with the Contributing Guidelines.
• New or existing tests cover these changes.
• The documentation is up to date with these changes.

[copy-pr-bot]

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

[PointKernel]

/ok to test 6c614f1

[PointKernel]

/ok to test 1760e0c

[PointKernel]

/ok to test 1818675

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: f5e0e2a1-720b-4bac-b2ef-59dcb852daac

📥 Commits

Reviewing files that changed from the base of the PR and between b8a14e2 and 1818675.

📒 Files selected for processing (9)

• cpp/include/cudf/detail/aggregation/device_aggregators.cuh
• cpp/include/cudf/reduction.hpp
• cpp/src/reductions/sum_with_overflow.cu
• cpp/tests/groupby/sum_with_overflow_tests.cpp
• java/src/main/java/ai/rapids/cudf/Aggregation.java
• java/src/main/java/ai/rapids/cudf/GroupByAggregation.java
• java/src/main/java/ai/rapids/cudf/ReductionAggregation.java
• java/src/test/java/ai/rapids/cudf/ReductionTest.java
• java/src/test/java/ai/rapids/cudf/TableTest.java

---

📝 Walkthrough

Summary by CodeRabbit

• Bug Fixes

  • Fixed sum-with-overflow aggregation behavior: the sum value is now computed and returned in all cases, with the overflow boolean flag as the authoritative indicator. On overflow, the sum value is unspecified and should not be used.

• Documentation

  • Updated documentation across C++ and Java APIs to clarify that on overflow, only the boolean flag is trustworthy while the sum value is unspecified.

Walkthrough

This PR updates the SUM_WITH_OVERFLOW aggregation semantics so that when overflow occurs, the returned sum value is unspecified and only the boolean overflow flag is treated as authoritative. Changes span C++ device aggregators, reduction implementation, documentation, and both C++ and Java tests.

Changes

SUM_WITH_OVERFLOW Overflow Semantics

Layer / File(s)	Summary
Device aggregator overflow early-exit cpp/include/cudf/detail/aggregation/device_aggregators.cuh	The overflow-detection check in update_target_element<Source, aggregation::SUM_WITH_OVERFLOW> is reordered to load the overflow flag and return early before computing the source value and atomic sum update.
Reduction return value semantics and documentation cpp/src/reductions/sum_with_overflow.cu, cpp/include/cudf/reduction.hpp	sum_with_overflow_impl now always returns the computed sum value instead of zeroing it on overflow; documentation clarifies that the sum is unspecified on overflow while the overflow flag is the authoritative indicator.
C++ groupby sum-with-overflow test updates cpp/tests/groupby/sum_with_overflow_tests.cpp	Added missing headers and refactored the overflow_detection test to use a check_overflow_flags helper that validates only the overflow boolean flags; test input construction is unified across integer types without signed/unsigned branching.
Java API documentation updates java/src/main/java/ai/rapids/cudf/Aggregation.java, java/src/main/java/ai/rapids/cudf/GroupByAggregation.java, java/src/main/java/ai/rapids/cudf/ReductionAggregation.java	Javadoc updated across all three API surfaces to state that the sum value is unspecified on overflow and the overflow flag is the source of truth.
Java test validation for overflow flags java/src/test/java/ai/rapids/cudf/ReductionTest.java, java/src/test/java/ai/rapids/cudf/TableTest.java	Positive and negative overflow tests in ReductionTest no longer assert the sum value on overflow; TableTest removes precomputed wrapped-sum expectations and focuses on overflow flag and type validation.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• rapidsai/cudf#22593: Also modifies the update_target_element<Source, aggregation::SUM_WITH_OVERFLOW> in device_aggregators.cuh to change overflow handling.
• rapidsai/cudf#22404: Touches the same Java surface and tests for SUM_WITH_OVERFLOW semantics alignment.
• rapidsai/cudf#22465: Related changes to cpp/src/reductions/sum_with_overflow.cu for overflow handling and output semantics.

Suggested reviewers

• ttnghia
• mhaseeb123
• paul-aiyedun

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main behavior change: skipping sum updates once the SUM_WITH_OVERFLOW flag is set.
Description check	✅ Passed	The description clearly explains the changes, references prior discussions, outlines the implementation approach for both groupby and reduce, and acknowledges the ABI-breaking nature with justification.
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.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ast-grep (0.43.0)

java/src/test/java/ai/rapids/cudf/TableTest.java

Warning

Review ran into problems

🔥 Problems

Stopped waiting for pipeline failures after 30000ms. One of your pipelines takes longer than our 30000ms fetch window to run, so review may not consider pipeline-failure results for inline comments if any failures occurred after the fetch window. Increase the timeout if you want to wait longer or run a @coderabbit review after the pipeline has finished.

---

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

[davidwendt]

Approving C++ changes.
Can you do a memcheck and racecheck on this as well?

[PointKernel]

Approving C++ changes. Can you do a memcheck and racecheck on this as well?

All checks passed without hazards.

[PointKernel]

/merge