Skip to content

skip length-one axes when choosing flat stride in init_iter_all#569

Merged
neutrinoceros merged 5 commits into
pydata:masterfrom
sahvx655-wq:iter-all-size-one-stride
Jun 3, 2026
Merged

skip length-one axes when choosing flat stride in init_iter_all#569
neutrinoceros merged 5 commits into
pydata:masterfrom
sahvx655-wq:iter-all-size-one-stride

Conversation

@sahvx655-wq

Copy link
Copy Markdown
Contributor

When a reduce-all function gets a transposed view whose size-1 axis carries a large stride (np.zeros((1,500,2)).transpose(1,2,0), np.zeros((1,192,121)).transpose(0,2,1)), the stride-picking loops in init_iter_all skip only zero strides and grab that axis's huge stride, so iteration runs off the buffer (issues #381, #393: garbage from nanmax or a segfault). Skip length-one axes there too, since their stride is never traversed.

@neutrinoceros

Copy link
Copy Markdown
Collaborator

Nice. Thank you for addressing these. Can you add regression tests too ? The example reproducers from issues you're fixing are probably good enough already.

@sahvx655-wq

Copy link
Copy Markdown
Contributor Author

Added a regression test in reduce_test.py covering both reproducers (#381 C-contiguous, #393 F-contiguous), checking bn.nanmax against bn.slow.nanmax on the transposed views.

@neutrinoceros

Copy link
Copy Markdown
Collaborator

Great. Though these are really two different test cases. I suggest splitting them or, if it makes sense, to parametrize the one test function.

@sahvx655-wq

Copy link
Copy Markdown
Contributor Author

Parametrized it into the two cases (issue381, issue393).

Comment thread bottleneck/tests/reduce_test.py Outdated
# issue #393 (F-contiguous branch)
np.arange(1 * 192 * 121, dtype=np.float64).reshape(1, 192, 121).transpose(0, 2, 1),
),
ids=("issue381", "issue393"),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
ids=("issue381", "issue393"),
ids=("C-contiguous", "F-contiguous"),

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, switched the ids to C-contiguous/F-contiguous.

@neutrinoceros neutrinoceros merged commit 75e994b into pydata:master Jun 3, 2026
42 checks passed
@neutrinoceros

Copy link
Copy Markdown
Collaborator

Thanks a lot for triaging and fixing these @sahvx655-wq !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

2 participants