You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
ak.sum's reducer already promotes integer/bool leaves to the platform integer
(int64/uint64) accumulator — see _promote_integer_rank in src/awkward/_reducers.py and the awkward_reduce_sum<result, input, offsets>
kernel templated separately on accumulator and input dtype. So:
ak.sum(int32_array) already accumulates in int64. A plain integer mean
without weights (sum(x) / count) is overflow-safe up to int64 today and
needs no cast.
The real overflow is the element-wise products computed in Python before
the reduction: x * x, x * weight, x * x * weight in var, std, covar, corr, moment. int32 * int32 stays int32 and overflows
per-element — long before int64 accumulation can help.
So the original #3527 PR's "cast the whole input to float64 up front, always" is
heavier than needed: it copies floating inputs that never overflow, and it casts
in a spot that isn't the only thing that matters.
"Binary floats can't represent decimals exactly." True, but that's an
argument against floating-point in general — it applies equally to x / 2,
which we already promote. The alternative here is silent integer overflow,
which is strictly worse than rounding error. NumPy makes exactly this choice
for mean/var/std.
"The cast triggers device-to-host copies on GPU." A dtype cast via ak.values_astype runs on the array's own backend — cupy.ndarray.astype on
the cuda backend. It stays on-device; there is no host round-trip. The cost on
GPU is float64 memory (2× an int32 buffer) and lower float64 throughput,
not a D2H transfer. The solution below only pays that cost for integral
inputs and only once per call.
Solution
Two tiers. Tier 1 is a small, self-contained Python PR. Tier 2 is the fully
copy-free version and needs a reducer tweak -- a separate PR to follow.
Tier 1 — promote only integral leaves, once, on-backend
A shared helper (e.g. in src/awkward/_layout.py):
importawkwardasakfromawkward._nplikes.numpy_likeimportNumpyMetadatanp=NumpyMetadata.instance()
def_has_integral_leaf(layout) ->bool:
"""True if any NumpyArray leaf is bool/integer (kinds 'b', 'i', 'u')."""found=Falsedefaction(node, **kwargs):
nonlocalfoundifnode.is_numpyandnode.dtype.kindin"biu":
found=TruereturnNoneak._do.recursively_apply(layout, action, return_array=False)
returnfounddefpromote_integral_to_float64(array):
"""Cast bool/integer data to float64 to avoid overflow in summations, matching NumPy's reduction dtype. Floating and complex data is returned unchanged (no copy). The cast runs on the array's own backend, so on GPU it stays on device — no host transfer. """if_has_integral_leaf(array.layout):
returnak.operations.values_astype(array, np.float64, highlevel=True)
returnarray
Then, in each statistic that forms products, promote once and reuse the
result across every reduction. For ak.var (src/awkward/operations/ak_var.py),
the change is one line plus reuse:
x=ctx.wrap(x_layout)
weight=ctx.wrap(weight_layout, allow_other=True)
# Overflow guard: integer squares/products overflow before reduction.# Promote integral leaves to float64 once (no-op + zero copy for floats).x=promote_integral_to_float64(x)
ifweightisnotNone:
weight=promote_integral_to_float64(weight)
...
# sumwx, sumwxx (and sumw) now reduce float64 data — one promotion, reused.
The actual scope is smaller than "every stat," because most paths are already
safe:
ak_var.py — needs it (sum(x*x)). ak_std.py inherits the fix for
free: it just calls ak.var and takes sqrt.
ak_moment.py — needs it (sum(x**n)).
ak_mean.py — needs it only in the weighted branch (sum(x*weight)).
The unweighted path (sum(x)/count) is already overflow-safe to int64 via
the reducer's int64 accumulator, and the final division yields float64, so
it is left copy-free.
ak_covar.py / ak_corr.py — need nothing. They center first, (x - xmean) * (y - ymean), and xmean is already float64 (it comes from ak.mean), so the products are float before any multiply. Their internal ak.mean call inherits the weighted-branch fix if integer weights are passed.
So the real change is three files: ak_var.py, ak_moment.py, and the weighted
branch of ak_mean.py — plus the shared helper.
If you additionally want unweighted integer mean to match NumPy's float64-accumulator behavior (guarding int64 overflow too), use Tier 2 rather
than adding a copy.
Net effect vs. the #3527 PR: floating inputs are untouched (zero copy — the common
case in analysis), integral inputs are promoted exactly once instead of
implicitly re-cast inside each x*x / x*weight, and the cast provably stays on
the GPU backend.
I've been doing exactly the same in #3527 and you were arguing against it since it has to copy the whole array at once. Which is true, when it's an array of integers, a whole array of floats has to be allocated at once of the same size to do so. It's not like numpy's np.sum(..., dtype=np.float64) which does the casting while iterating inside the loop.
I've been doing exactly the same in #3527 and you were arguing against it since it has to copy the whole array at once. Which is true, when it's an array of integers, a whole array of floats has to be allocated at once of the same size to do so. It's not like numpy's np.sum(..., dtype=np.float64) which does the casting while iterating inside the loop.
✅ All modified and coverable lines are covered by tests. ⚠️ Please upload report for BASE (main@27a23cd). Learn more about missing BASE report. ⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Where the overflow actually is
ak.sum's reducer already promotes integer/bool leaves to the platform integer(
int64/uint64) accumulator — see_promote_integer_rankinsrc/awkward/_reducers.pyand theawkward_reduce_sum<result, input, offsets>kernel templated separately on accumulator and input dtype. So:
ak.sum(int32_array)already accumulates inint64. A plain integer meanwithout weights (
sum(x) / count) is overflow-safe up toint64today andneeds no cast.
the reduction:
x * x,x * weight,x * x * weightinvar,std,covar,corr,moment.int32 * int32staysint32and overflowsper-element — long before
int64accumulation can help.So the original #3527 PR's "cast the whole input to
float64up front, always" isheavier than needed: it copies floating inputs that never overflow, and it casts
in a spot that isn't the only thing that matters.
The two objections on the #3527 PR, addressed
argument against floating-point in general — it applies equally to
x / 2,which we already promote. The alternative here is silent integer overflow,
which is strictly worse than rounding error. NumPy makes exactly this choice
for
mean/var/std.ak.values_astyperuns on the array's own backend —cupy.ndarray.astypeonthe cuda backend. It stays on-device; there is no host round-trip. The cost on
GPU is
float64memory (2× anint32buffer) and lowerfloat64throughput,not a D2H transfer. The solution below only pays that cost for integral
inputs and only once per call.
Solution
Two tiers. Tier 1 is a small, self-contained Python PR. Tier 2 is the fully
copy-free version and needs a reducer tweak -- a separate PR to follow.
Tier 1 — promote only integral leaves, once, on-backend
A shared helper (e.g. in
src/awkward/_layout.py):Then, in each statistic that forms products, promote once and reuse the
result across every reduction. For
ak.var(src/awkward/operations/ak_var.py),the change is one line plus reuse:
The actual scope is smaller than "every stat," because most paths are already
safe:
ak_var.py— needs it (sum(x*x)).ak_std.pyinherits the fix forfree: it just calls
ak.varand takessqrt.ak_moment.py— needs it (sum(x**n)).ak_mean.py— needs it only in the weighted branch (sum(x*weight)).The unweighted path (
sum(x)/count) is already overflow-safe toint64viathe reducer's
int64accumulator, and the final division yieldsfloat64, soit is left copy-free.
ak_covar.py/ak_corr.py— need nothing. They center first,(x - xmean) * (y - ymean), andxmeanis alreadyfloat64(it comes fromak.mean), so the products are float before any multiply. Their internalak.meancall inherits the weighted-branch fix if integer weights are passed.So the real change is three files:
ak_var.py,ak_moment.py, and the weightedbranch of
ak_mean.py— plus the shared helper.If you additionally want unweighted integer
meanto match NumPy'sfloat64-accumulator behavior (guardingint64overflow too), use Tier 2 ratherthan adding a copy.
Net effect vs. the #3527 PR: floating inputs are untouched (zero copy — the common
case in analysis), integral inputs are promoted exactly once instead of
implicitly re-cast inside each
x*x/x*weight, and the cast provably stays onthe GPU backend.