8000 MAINT: add test for 12-byte alignment by ahaldane · Pull Request #12618 · numpy/numpy · GitHub
[go: up one dir, main page]

Skip to content

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

Merged
merged 1 commit into from
Dec 27, 2018

Conversation

ahaldane
Copy link
Member

Followup to #12611 to add a test for a 12-byte, 12-byte-aligned array.

I checked it crashes on x64 before #12611.

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']:
Copy link
Member

Choose a reason for hiding this comment

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

Why is this needed?

Copy link
Member Author
@ahaldane ahaldane Dec 26, 2018

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).

@charris
Copy link
Member
charris commented Dec 26, 2018

Looks like we have an odd invalid value RuntimeWarning slipping through from somewhere in Windows 32 bit, Python 3.7. Same thing turned up in a different place in PR previous to this one. I think it is irrelevant to this PR, but it would be nice to know where it comes from.

@charris
Copy link
Member
charris commented Dec 26, 2018

Will rebuild azure test.

@charris
Copy link
Member
charris commented Dec 26, 2018

To summarize the odd recurring test failures, the common parts seem to be

  • cgemv
  • Python 3.7
  • OpenBLAS (3.4+)

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?

@eric-wieser
Copy link
Member

np.dtype('i4,i4,i4').alignment == 4. Is that relevant here?

@ahaldane
Copy link
Member Author

np.dtype('i4,i4,i4').alignment == 4. Is that relevant here?

That refers to the "true" alignment, not the uint alignment, so it should be irrelevant for the copy code.

@charris charris merged commit 5e1a891 into numpy:master Dec 27, 2018
@charris
Copy link
Member
charris commented Dec 27, 2018

Thanks Allan.

@charris charris added this to the 1.16.0 release milestone Dec 27, 2018
@charris charris removed the 09 - Backport-Candidate PRs tagged should be backported label Dec 27, 2018
@charris charris removed this from the 1.16.0 release milestone Dec 27, 2018
@charris
Copy link
Member
charris commented Dec 27, 2018

@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?

@charris
Copy link
Member
charris commented Dec 27, 2018

I'm guessing there are some extra flags being set for the merged version, but darned if I know what.

@ahaldane
Copy link
Member Author

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 _aligned_zeros ...

@ahaldane
Copy link
Member Author

OK, I just ran tests with USE_DEBUG and I seem to be able to reproduce the crash.

@charris
Copy link
Member
charris commented Dec 27, 2018

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.

@ahaldane
Copy link
Member Author

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 _aligned_zeros is working properly.

@ahaldane
Copy link
Member Author

@charris, on my local machine I can confirm that the test only fails about half the times I run it

@ahaldane
Copy link
Member Author

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.

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

Successfully merging this pull request may close these issues.

3 participants
0