ENH: Return numpy scalars directly from the full-reduction fast path - #32269
ENH: Return numpy scalars directly from the full-reduction fast path#32269eendebakpt wants to merge 1 commit into
Conversation
ngoldbaum
left a comment
There was a problem hiding this comment.
Neat, just one driveby comment about the unrelated docs change.
You said fable helped you - IMO it produces way too many comments without a lot of prompting to discourage it. Also IMO, this PR as-is probably has too many comments.
Agreed. I already have statements in the memory files begging to be concise in comments, but it is still quite verbose. I will adapt the comments |
d654d5a to
9dff34c
Compare
When the caller would convert the 0-d result to a scalar anyway, accumulate into a stack buffer and create the scalar directly with PyArray_Scalar, skipping the temporary 0-d array and the wrap machinery. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
9dff34c to
8e249b8
Compare
| /* Allocate one 0-d result per output so the loop can write into them. | ||
| * With ``return_scalar`` set, accumulate into a stack buffer. | ||
| */ | ||
| npy_clongdouble buffer; /* aligned scratch large enough for any number */ |
There was a problem hiding this comment.
I don't like that this relies on the platform-dependent size and alignment of this type and a type punning pattern that, per the C standard, is UB. We don't compile with -f-strict-aliasing but it would be nice if we could in the future and I'd rather not add even more mechanisms based on type punning.
Instead, this could be an instance of a union:
typedef union {
npy_bool boolean;
npy_byte byte;
... # all the other non-flexible non-reference builtin dtypes
npt timedelta timedelta;
} reduction_scalar_storageThen below you could explicitly switch on descr->type_num and point accum[0] at the corresponding member of the union like &storage.double_ for double-precision float accumulators.
That also allows you to exclude object, StringDType, flexible legacy string dtypes, and void by construction. You also get a stack-allocated accumulator that is automatically the correct type, size, and alignment for the buffer without relying on the platform-dependent size and alignment of npy_clongdouble.
PR summary
When a full reduction (
axis=None) hits the contiguous fast path and the caller would immediately convert the 0-d result to a numpy scalar anyway (plainndarrayinput, noout=, no__array_wrap__to apply), accumulate into a stack buffer and create the scalar directly withPyArray_Scalar. This skips allocating the temporary 0-d array and the wrap machinery.The scalar path is only taken for single-output reductions over dtypes whose scalars are self-contained (
elsize <= sizeof(npy_clongdouble), not flexible, noNPY_USE_GETITEM), on top of the existing fast-path preconditions.Benchmark results:
The two control cases do not use the scalar path. The small difference on the
axis=0control is binary-layout noise.Benchmark script
Run with
python bench_reduce_scalar.py -o result.jsonon each build, thenpython -m pyperf compare_to main.json pr.json --table.AI Disclosure
Claude Fable was used in prototyping, refining and benchmarking the PR.