8000 MAINT,ENH: Reorganize buffered iteration setup by seberg · Pull Request #27883 · numpy/numpy · GitHub
[go: up one dir, main page]

Skip to content

MAINT,ENH: Reorganize buffered iteration setup #27883

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 23 commits into from
Dec 17, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
8d69f05
MAINT: Refactor buffered loop to do an initial setup
seberg Nov 28, 2024
7d19f2d
TST: Fixup tests to pass (for now not correct)
seberg Nov 30, 2024
809cf0c
BUG: Avoid a coresize of 0
seberg Dec 2, 2024
9302b30
TST: The nullpointer doesn't print uniformly accross platforms
seberg Dec 2, 2024
54d967f
BUG: Add missing pointer dereference
seberg Dec 2, 2024
87ad5a2
BUG: Fix coreoffset mechanism and unset buf-reuse on different (parti…
seberg Dec 2, 2024
67cddf7
TST: Forgot one more nditer debug_print() fixup
seberg Dec 2, 2024
facd237
Apply suggestions from code review
seberg Dec 2, 2024
22f335c
MAINT: Remove bufnever logic from allocate-arrays (we do it later any…
seberg Dec 2, 2024
ac282fc
MAINT: Rename to BUF_SINGLESTRIDE and use the new gap in the flags
seberg Dec 2, 2024
6ed3b99
MAINT: Remove unnecessary fixed-stride logic
seberg Dec 3, 2024
162a951
MAINT: Fill in correct buffer transferstride always
seberg Dec 3, 2024
0cc37b9
MAINT: Don't use static zero (initialize by caller instead)
seberg Dec 3, 2024
786e546
DOC: Improve the function documentation somewhat.
seberg Dec 3, 2024
d09be24
BUG: Fix typo in new assert
seberg Dec 3, 2024
92ecbaf
MAINT: Reinstante the CONTIG flag (as buggy as it was)
seberg Dec 5, 2024
5bad357
remove debug print (was this part of the sanitizer problem?!)
seberg Dec 5, 2024
67cb5c5
TST: See if deleting a, b makes santizer tests pass
seberg Dec 6, 2024
5508cc5
MAINT: Move buffersize init (new code always limits it to itersize)
seberg Dec 10, 2024
be6749d
DOC: Explain buffer-setup cost more (maybe should rethink it...)
seberg Dec 12, 2024
426934d
DOC: Expand/change iteration explanation based on Marten's review
seberg Dec 15, 2024
1836fd1
MAINT: More review comments (mostly code clarifications/simplifications)
seberg Dec 15, 2024
e758983
MAINT: Smaller changes based on review
seberg Dec 16, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
MAINT: More review comments (mostly code clarifications/simplifications)
  • Loading branch information
seberg committed Dec 15, 2024
commit 1836fd14eba0e0c6466c884c5c8f6ab41392fda4
2 changes: 1 addition & 1 deletion numpy/_core/src/multiarray/nditer_api.c
Original file line number Diff line number Diff line change
Expand Up @@ -2185,7 +2185,7 @@ npyiter_copy_to_buffers(NpyIter *iter, char **prev_dataptrs)
&& op_itflags[iop]&NPY_OP_ITFLAG_REDUCE
&& NAD_STRIDES(outer_axisdata)[iop] == 0)) {
/*
* If we have a fully copy or a reduce with 0 stride outer and
* If we have a full copy or a reduce with 0 stride outer and
* a copy larger than the coresize, this is now re-usable.
* NB: With a core-offset, we always copy less than the core-size.
*/
Expand Down
85 changes: 39 additions & 46 deletions numpy/_core/src/multiarray/nditer_constr.c
Original file line number Diff line number Diff line change
Expand Up @@ -2027,8 +2027,9 @@ npyiter_find_buffering_setup(NpyIter *iter, npy_intp buffersize)
NpyIter_BufferData *bufferdata = NIT_BUFFERDATA(iter);

/*
* We check two things here, first whether the core is single strided
* and second, whether we found a reduce stride dimension for the operand.
* We check two things here, first how many operand dimensions can be
* iterated using a single stride (all dimensions are consistent),
* and second, whether we found a reduce dimension for the operand.
* That is an outer dimension a reduce would have to take place on.
*/
int op_single_stride_dims[NPY_MAXARGS];
Expand Down Expand Up @@ -2094,19 +2095,17 @@ npyiter_find_buffering_setup(NpyIter *iter, npy_intp buffersize)
NIT_ADVANCE_AXISDATA(axisdata, 1);
npy_intp *strides = NAD_STRIDES(axisdata);

int iop;
for (iop = 0; iop < nop; iop++) {
for (int iop = 0; iop < nop; iop++) {
/* Check that we set things up nicely (if shape is ever 1) */
assert((axisdata->shape == 1) ? (prev_strides[iop] == strides[iop]) : 1);
/* Best case: the strides collapse for this operand. */
if (prev_strides[iop] * prev_shape == strides[iop]) {
if (op_single_stride_dims[iop] == idim) {

if (op_single_stride_dims[iop] == idim) {
/* Best case: the strides still collapse for this operand. */
if (prev_strides[iop] * prev_shape == strides[iop]) {
op_single_stride_dims[iop] += 1;
continue;
}
continue;
}

if (op_single_stride_dims[iop] == idim) {
/*
* Operand now requires buffering (if it was not already).
* NOTE: This is technically not true since we may still use
Expand All @@ -2120,28 +2119,19 @@ npyiter_find_buffering_setup(NpyIter *iter, npy_intp buffersize)
}

/*
* If this operand is a reduction operand and this is not the
* outer_reduce_dim, then we must stop.
* If this operand is a reduction operand and the stride swapped
* between !=0 and ==0 then this is the `outer_reduce_dim` and
* we will never continue further (see break at start of op loop).
*/
if (op_itflags[iop] & NPY_OP_ITFLAG_REDUCE) {
/*
* We swap between !=0/==0 and thus make it a reduce
* (it is OK if another op started a reduce at this dimension)
*/
if (strides[iop] == 0 || prev_strides[iop] == 0) {
if (outer_reduce_dim == 0 || outer_reduce_dim == idim) {
op_reduce_outer_dim[iop] = idim;
outer_reduce_dim = idim;
}
}
if ((op_itflags[iop] & NPY_OP_ITFLAG_REDUCE)
&& (strides[iop] == 0 || prev_strides[iop] == 0)) {
assert(outer_reduce_dim == 0 || outer_reduce_dim == idim);
op_reduce_outer_dim[iop] = idim;
outer_reduce_dim = idim;
}
/* For clarity: op_reduce_outer_dim[iop] if set always matches. */
assert(!op_reduce_outer_dim[iop] || op_reduce_outer_dim[iop] == outer_reduce_dim);
}
if (iop != nop) {
/* Including this dimension was invalid due to a reduction. */
break;
}

npy_intp coresize = size; /* if we iterate here, this is the core */
size *= axisdata->shape;
Expand Down Expand Up @@ -2209,7 +2199,7 @@ npyiter_find_buffering_setup(NpyIter *iter, npy_intp buffersize)

/* If contig was requested and this is not writeable avoid zero strides */
npy_bool avoid_zero_strides = (
(op_itflags[iop] & NPY_OP_ITFLAG_CONTIG) != 0
(op_itflags[iop] & NPY_OP_ITFLAG_CONTIG)
&& !(op_itflags[iop] & NPY_OP_ITFLAG_WRITE));

/*
Expand Down Expand Up @@ -2267,26 +2257,29 @@ npyiter_find_buffering_setup(NpyIter *iter, npy_intp buffersize)
* reduce logic. In that case, either the inner or outer stride
* is 0.
*/
if (!is_reduce_op
&& NIT_OPITFLAGS(iter)[iop] & NPY_OP_ITFLAG_BUF_SINGLESTRIDE
&& NAD_STRIDES(axisdata)[iop] == 0
&& !avoid_zero_strides) {
/* This op is always 0 strides, so even the buffer is that. */
inner_stride = 0;
reduce_outer_stride = 0;
}
else if (!is_reduce_op) {
/* normal buffered op */
inner_stride = itemsize;
reduce_outer_stride = itemsize * best_coresize;
}
else if (NAD_STRIDES(reduce_axisdata)[iop] == 0) {
inner_stride = itemsize;
reduce_outer_stride = 0;
if (is_reduce_op) {
if (NAD_STRIDES(reduce_axisdata)[iop] == 0) {
inner_stride = itemsize;
reduce_outer_stride = 0;
}
else {
inner_stride = 0;
reduce_outer_stride = itemsize;
}
}
else {
inner_stride = 0;
reduce_outer_stride = itemsize;
if (NIT_OPITFLAGS(iter)[iop] & NPY_OP_ITFLAG_BUF_SINGLESTRIDE
&& NAD_STRIDES(axisdata)[iop] == 0
&& !avoid_zero_strides) {
/* This op is always 0 strides, so even the buffer is that. */
inner_stride = 0;
reduce_outer_stride = 0;
}
else {
/* normal buffered op */
inner_stride = itemsize;
reduce_outer_stride = itemsize * best_coresize;
}
}
}
else {
Expand Down
4 changes: 2 additions & 2 deletions numpy/_core/src/multiarray/nditer_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -131,15 +131,15 @@
/* The operand requires masking when copying buffer -> array */
#define NPY_OP_ITFLAG_WRITEMASKED 0x0080
/*
* Whether the buffer is *fully* filled and thus ready for re-usable.
* Whether the buffer is *fully* filled and thus ready for reuse.
* (Must check if the start pointer matches until copy-from-buffer checks)
*/
#define NPY_OP_ITFLAG_BUF_REUSABLE 0x0100
/* The operand must be copied (with UPDATEIFCOPY if also ITFLAG_WRITE) */
#define NPY_OP_ITFLAG_FORCECOPY 0x0200
/* The operand has temporary data, write it back at dealloc */
#define NPY_OP_ITFLAG_HAS_WRITEBACK 0x0400
/* Whether the user request a contiguous operand */
/* Whether the user requested a contiguous operand */
#define NPY_OP_ITFLAG_CONTIG 0x0800

/*
Expand Down
Loading
0