Skip to content

Commit 4eb757c

Browse files
authored
Clarify Kahan condition (microsoft#28790)
This pull request refactors the logic for determining when to use Kahan compensated summation in reduction aggregators for integer types in `onnxruntime/core/providers/cpu/reduction/reduction_ops.h`. Instead of relying on the size of the type, a new type trait, `kReduceUseKahan<T>`, is introduced to explicitly indicate when Kahan summation should be used (currently for `int64_t` and `uint64_t`). This improves code clarity and maintainability by making the condition more explicit and less error-prone. **Refactoring of Kahan Summation Condition:** * Introduced the `kReduceUseKahan<T>` constexpr variable to indicate when Kahan compensated summation should be used, replacing the previous `sizeof(T) >= sizeof(int64_t)` checks. This trait currently evaluates to true for `int64_t` and `uint64_t` types. * Updated all reduction aggregator classes (`ReduceAggregatorSum`, `ReduceAggregatorSumSquare`, `ReduceAggregatorMean`, `ReduceAggregatorL1`, `ReduceAggregatorL2`, `ReduceAggregatorLogSum`) to use `kReduceUseKahan<T>` instead of the previous size-based condition when deciding to apply Kahan summation in both `aggall` and `update` methods. [[1]](diffhunk://#diff-ca0c9224442a3c46251b0fb7326aacc1469bdee20ab409b930556f439d560015L232-R239) [[2]](diffhunk://#diff-ca0c9224442a3c46251b0fb7326aacc1469bdee20ab409b930556f439d560015L247-R254) [[3]](diffhunk://#diff-ca0c9224442a3c46251b0fb7326aacc1469bdee20ab409b930556f439d560015L457-R464) [[4]](diffhunk://#diff-ca0c9224442a3c46251b0fb7326aacc1469bdee20ab409b930556f439d560015L484-R491) [[5]](diffhunk://#diff-ca0c9224442a3c46251b0fb7326aacc1469bdee20ab409b930556f439d560015L518-R525) [[6]](diffhunk://#diff-ca0c9224442a3c46251b0fb7326aacc1469bdee20ab409b930556f439d560015L1164-R1171) [[7]](diffhunk://#diff-ca0c9224442a3c46251b0fb7326aacc1469bdee20ab409b930556f439d560015L1188-R1195) [[8]](diffhunk://#diff-ca0c9224442a3c46251b0fb7326aacc1469bdee20ab409b930556f439d560015L1248-R1255) [[9]](diffhunk://#diff-ca0c9224442a3c46251b0fb7326aacc1469bdee20ab409b930556f439d560015L1276-R1283) [[10]](diffhunk://#diff-ca0c9224442a3c46251b0fb7326aacc1469bdee20ab409b930556f439d560015L1316-R1323) [[11]](diffhunk://#diff-ca0c9224442a3c46251b0fb7326aacc1469bdee20ab409b930556f439d560015L1347-R1354)
1 parent d43a48b commit 4eb757c

1 file changed

Lines changed: 18 additions & 11 deletions

File tree

onnxruntime/core/providers/cpu/reduction/reduction_ops.h

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,13 @@ inline bool reduce_isnan<int32_t>(int32_t) { return false; }
154154
template <>
155155
inline bool reduce_isnan<int64_t>(int64_t) { return false; }
156156

157+
// Integer types whose precision exceeds double's 53-bit mantissa, where summing in double
158+
// loses precision for large values. For these, reduction aggregators use Kahan compensated
159+
// summation. int32_t/uint32_t (and narrower) fit exactly in double, so plain double
160+
// accumulation is sufficient.
161+
template <typename T>
162+
inline constexpr bool kReduceUseKahan = std::is_same_v<T, int64_t> || std::is_same_v<T, uint64_t>;
163+
157164
class ReduceAggregatorBase {
158165
public:
159166
// Fast reduction: see OptimizeShapeForFastReduce's comment.
@@ -229,7 +236,7 @@ class ReduceAggregatorSum : public ReduceAggregator<T, T> {
229236
inline ReduceAggregatorSum(int64_t N, const T&) : ReduceAggregator<T, T>(N, 0) {}
230237
inline void update(const T& v) {
231238
if constexpr (std::is_integral_v<T>) {
232-
if constexpr (sizeof(T) >= sizeof(int64_t)) {
239+
if constexpr (kReduceUseKahan<T>) {
233240
double y = static_cast<double>(v) - kahan_compensation_;
234241
double t = double_accumulator_ + y;
235242
kahan_compensation_ = (t - double_accumulator_) - y;
@@ -244,7 +251,7 @@ class ReduceAggregatorSum : public ReduceAggregator<T, T> {
244251
static T aggall(const T* from_data, int64_t size) {
245252
if constexpr (std::is_integral_v<T>) {
246253
double sum = 0.0;
247-
if constexpr (sizeof(T) >= sizeof(int64_t)) {
254+
if constexpr (kReduceUseKahan<T>) {
248255
double comp = 0.0;
249256
for (size_t i = 0, n = onnxruntime::narrow<size_t>(size); i < n; ++i) {
250257
double y = static_cast<double>(from_data[i]) - comp;
@@ -454,7 +461,7 @@ class ReduceAggregatorSumSquare : public ReduceAggregator<T, TVAL> {
454461
inline TVAL aggall(const T* from_data) {
455462
if constexpr (std::is_integral_v<T>) {
456463
double sum = 0.0;
457-
if constexpr (sizeof(T) >= sizeof(int64_t)) {
464+
if constexpr (kReduceUseKahan<T>) {
458465
double comp = 0.0;
459466
for (size_t i = 0, n = onnxruntime::narrow<size_t>(this->N_); i < n; ++i) {
460467
double dv = static_cast<double>(from_data[i]);
@@ -481,7 +488,7 @@ class ReduceAggregatorSumSquare : public ReduceAggregator<T, TVAL> {
481488
inline void update(const T& v) {
482489
if constexpr (std::is_integral_v<T>) {
483490
double dv = static_cast<double>(v);
484-
if constexpr (sizeof(T) >= sizeof(int64_t)) {
491+
if constexpr (kReduceUseKahan<T>) {
485492
double y = dv * dv - kahan_compensation_;
486493
double t = double_accumulator_ + y;
487494
kahan_compensation_ = (t - double_accumulator_) - y;
@@ -515,7 +522,7 @@ class ReduceAggregatorMean : public ReduceAggregatorSum<T> {
515522
if constexpr (std::is_integral_v<T>) {
516523
// Accumulate in double, divide, then saturate back to T.
517524
double sum = 0.0;
518-
if constexpr (sizeof(T) >= sizeof(int64_t)) {
525+
if constexpr (kReduceUseKahan<T>) {
519526
double comp = 0.0;
520527
for (size_t i = 0, n = onnxruntime::narrow<size_t>(size); i < n; ++i) {
521528
double y = static_cast<double>(from_data[i]) - comp;
@@ -1161,7 +1168,7 @@ class ReduceAggregatorL1 : public ReduceAggregator<T, T> {
11611168
inline T aggall(const T* from_data) {
11621169
if constexpr (std::is_integral_v<T>) {
11631170
double sum = 0.0;
1164-
if constexpr (sizeof(T) >= sizeof(int64_t)) {
1171+
if constexpr (kReduceUseKahan<T>) {
11651172
// Kahan compensated summation for int64+ to minimize precision loss.
11661173
double comp = 0.0;
11671174
for (size_t i = 0, n = onnxruntime::narrow<size_t>(this->N_); i < n; ++i) {
@@ -1185,7 +1192,7 @@ class ReduceAggregatorL1 : public ReduceAggregator<T, T> {
11851192
}
11861193
inline void update(const T& v) {
11871194
if constexpr (std::is_integral_v<T>) {
1188-
if constexpr (sizeof(T) >= sizeof(int64_t)) {
1195+
if constexpr (kReduceUseKahan<T>) {
11891196
// Kahan compensated summation for int64+.
11901197
double y = std::abs(static_cast<double>(v)) - kahan_compensation_;
11911198
double t = double_accumulator_ + y;
@@ -1245,7 +1252,7 @@ class ReduceAggregatorL2 : public ReduceAggregator<T, T> {
12451252
inline T aggall(const T* from_data) {
12461253
if constexpr (std::is_integral_v<T>) {
12471254
double sum = 0.0;
1248-
if constexpr (sizeof(T) >= sizeof(int64_t)) {
1255+
if constexpr (kReduceUseKahan<T>) {
12491256
// Kahan compensated summation for int64+ to minimize precision loss.
12501257
double comp = 0.0;
12511258
for (size_t i = 0, n = onnxruntime::narrow<size_t>(this->N_); i < n; ++i) {
@@ -1273,7 +1280,7 @@ class ReduceAggregatorL2 : public ReduceAggregator<T, T> {
12731280
inline void update(const T& v) {
12741281
if constexpr (std::is_integral_v<T>) {
12751282
double dv = static_cast<double>(v);
1276-
if constexpr (sizeof(T) >= sizeof(int64_t)) {
1283+
if constexpr (kReduceUseKahan<T>) {
12771284
// Kahan compensated summation for int64+.
12781285
double y = dv * dv - kahan_compensation_;
12791286
double t = double_accumulator_ + y;
@@ -1313,7 +1320,7 @@ class ReduceAggregatorLogSum : public ReduceAggregator<T, T> {
13131320
inline T aggall(const T* from_data) {
13141321
if constexpr (std::is_integral_v<T>) {
13151322
double sum = 0.0;
1316-
if constexpr (sizeof(T) >= sizeof(int64_t)) {
1323+
if constexpr (kReduceUseKahan<T>) {
13171324
double comp = 0.0;
13181325
for (size_t i = 0, n = onnxruntime::narrow<size_t>(this->N_); i < n; ++i) {
13191326
double y = static_cast<double>(from_data[i]) - comp;
@@ -1344,7 +1351,7 @@ class ReduceAggregatorLogSum : public ReduceAggregator<T, T> {
13441351
}
13451352
inline void update(const T& v) {
13461353
if constexpr (std::is_integral_v<T>) {
1347-
if constexpr (sizeof(T) >= sizeof(int64_t)) {
1354+
if constexpr (kReduceUseKahan<T>) {
13481355
double y = static_cast<double>(v) - kahan_compensation_;
13491356
double t = double_accumulator_ + y;
13501357
kahan_compensation_ = (t - double_accumulator_) - y;

0 commit comments

Comments
 (0)