Skip to content

Require len > 1 axis for layout preference #977

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 2 commits into from
Apr 8, 2021
Merged

Require len > 1 axis for layout preference #977

merged 2 commits into from
Apr 8, 2021

Conversation

bluss
Copy link
Member

@bluss bluss commented Apr 8, 2021

This change means that we are not drawn to prefer an axis with length 1 as the "innermost/tightest" axis in Zip.

This fixes the bad performance reported in #932 (comment), specifically select_axis1. The runtime for that benchmark decreases from 21 to 3.4 μs, which is quite substantial and just due to misidentification (in that case) of which axis to prefer for the loop.

bluss added 2 commits April 8, 2021 22:22
These tests - w.r.t C/F preference, failed when they were introduced.
The issue was that otherwise, this array:
shape: [1, 256]
strides: [1, 256]

Which was "correctly" determined to be F-preference because it's
sliced out of an f-order array. In reality it is discontiguous; every
element is 256 steps from the next. This should have layout None, the
same as this array would have (ignoring 1-len axes).

shape: [256]
strides: [256]
@bluss bluss changed the title Fix Require len > 1 axis for layout preference Apr 8, 2021
@bluss bluss merged commit eb0dfa8 into master Apr 8, 2021
@bluss bluss deleted the cf-preference branch April 8, 2021 20:34
@bluss bluss added this to the 0.15.2 milestone Apr 8, 2021
@bluss
Copy link
Member Author

bluss commented Apr 8, 2021

This commit (on a pr branch - don't know if the commit will disappear) is relevant to the same problem 4878d1b (Turns out I fixed an assertion for this scenario). After the present PR was merged, the situation described in this message has changed, we fixed it to remove the "leaning f" judgment. Still, the array could be "contiguous with stride 2."

 FIX: Fix iteration order debug assertion in Zip

We're trying to assert the iteration order is like C-contig or
C-preferred. The old assertion tripped when combining these two arrays:

- shape 1, 7 strides 7, 1  (layout: CFcf)
- shape 1, 7 strides 1, 2  (layout: f)

The result was that the first was judged to have "no preference" and
second "leaning f", total "leaning f".

The debug assertion tripped a false positive because the traversal is
one-dimensional, so elements are still visited in the required order.

The layout judgment for the second array should ideally be "CFcf" too,
but then it needs an understanding of being one-dimensional with
contiguous stride of 2, which is not implemented.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant