-
-
Notifications
You must be signed in to change notification settings - Fork 10.9k
MAINT: add test for 12-byte alignment #12618
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
Conversation
for n in [0, 1, 3, 11]: | ||
for order in ["C", "F", None]: | ||
for dtype in np.typecodes["All"]: | ||
for dtype in list(np.typecodes["All"]) + ['i4,i4,i4']: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought it would be good to throw in a 12-byte-equivalence type, and i4,i4,i4
is 12 bytes.
(if a 12-byte-uint existed it would be uint-aligned to 12 bytes. But actually it is not uint-alignable).
Looks like we have an odd invalid value |
Will rebuild azure test. |
To summarize the odd recurring test failures, the common parts seem to be
I'm not sure what role the Python version might play in this or why it is just now showing up, OpenBLAS has been used on azure since Dec 6. EDIT: @matthew-brett Any ideas about the OpenBLAS part? |
|
That refers to the "true" alignment, not the uint alignment, so it should be irrelevant for the copy code. |
Thanks Allan. |
@ahaldane The tests passed here, but the merge to master is failing, same for the backport. I assume there has been some sort of merge mixup with the previous merges. Could you take a look? |
I'm guessing there are some extra flags being set for the merged version, but darned if I know what. |
Strange. It could be a sporadic error I suppose, that only appears sometimes. Eg, it only happens if malloc gives back an unexpectedly aligned pointer. But I though we were forcing the alignment with |
OK, I just ran tests with USE_DEBUG and I seem to be able to reproduce the crash. |
What is also odd is that I just tested a PR against the 1.16.x branch that now has the troublesome test in it, and it passed. So there is something different about the branch merges as opposed to the PRs. |
OK, I see the bug, it is that the assert I modified in #12511 still isn't quite right: It should be checking uint alignment, not true alignment. But I still want to figure out why the test only fails sporadically. It seems like it should always fail, if |
@charris, on my local machine I can confirm that the test only fails about half the times I run it |
Ok, it only turns up half the time because of malloc alignment variation, combined with the fact that the test was requesting 12 byte alignment and testing 16 byte alignment. But when you request 12 byte alignment, some of the time that is also 16 byte aligned, eg for pointer multiples of 48. |
Followup to #12611 to add a test for a 12-byte, 12-byte-aligned array.
I checked it crashes on x64 before #12611.