Repository navigation
[CPU] Make ReduceSum exact for integer types - #33599
shibaacharya wants to merge 2 commits into
Conversation
Accumulate integer sums in a 128-bit accumulator instead of double so int64 values above 2^53 keep full precision. Only the final result saturates. ReduceMean keeps its double accumulation. Fixes microsoft#33579 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
There was a problem hiding this comment.
🟡 Changes recommended
The new tests have a compilation error, include incompatible CUDA expectations, and do not exercise the modified fast reduction paths.
3 open issues
What changed in this PR
Addresses #33579 by making CPU integer ReduceSum exact above 2^53, while preserving final-result saturation and existing ReduceMean behavior.
Changes:
- Adds a two-word integer accumulator for scalar and fast reduction paths.
- Keeps double/Kahan accumulation in
ReduceMean. - Adds int64 regressions for precision loss and cancellation after intermediate overflow.
| File | Description |
|---|---|
| onnxruntime/test/providers/cpu/reduction/reduction_ops_test.cc | Adds integer exactness and cancellation regression cases. |
| onnxruntime/core/providers/cpu/reduction/reduction_ops.h | Implements exact integer sums while preserving mean accumulation. |
Review effort: Balanced
|
|
@microsoft-github-policy-service agree |
4c8d3eb to
d3d2bb0
Compare
|
/azp run |
|
Commenter does not have sufficient privileges for PR 33599 in repo microsoft/onnxruntime |
|
Hi, I've updated the PR to address the review comments and pushed the changes. I tried running the Azure Pipelines using /azp run, but the bot reported that I don't have sufficient privileges for this PR. Could a maintainer please trigger the required pipelines? Thanks! |


CPU ReduceSum for integer types accumulated in double, so int64/uint64 inputs above 2^53 lost precision before summation started. Kahan compensation can't recover bits already dropped in the conversion.
This PR:
Adds ExactIntSum, a two-word (128-bit, two's complement) accumulator in reduction_ops.h.
Uses it in ReduceAggregatorSum for the scalar update/get_value path and aggall. The same accumulator is used in the fast RK, KRK and RKR paths.
Saturates only the final result to T::max()/T::lowest(), so overflow behaviour is unchanged. Intermediate partial sums outside the range of T no longer affect the result.
Moves the existing double/Kahan accumulation into ReduceAggregatorMean, so ReduceMean behaves as before.
Adds ReduceSum_int64_ExactAbove2Pow53 with these cases:
[2^53+1, -2^53] → 1
[2^53, 1] → 2^53+1
the RK fast path
cancellation after the partial sum exceeds int64
Motivation and Context
Fixes #33579. Before this change, ReduceSum on [2^53+1, -2^53] returned 0 instead of 1, even though the inputs and the result all fit in int64.