Skip to content

BUG: Fix arr.flat.index for large arrays and big-endian machines #19276

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.

Sign up for GitHub

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

Jump to bottom
Merged
mattip merged 2 commits into from
Jun 21, 2021
Merged

BUG: Fix arr.flat.index for large arrays and big-endian machines #19276

mattip merged 2 commits into from
Jun 21, 2021

Conversation

@seberg
Copy link
Member

@seberg seberg commented Jun 18, 2021

To do that, I have a first commit to introduce a new PyArray_PyIntFromIntp inline function, just because those #ifdef's everywhere are not very nice IMO.

Closes gh-19153

seberg added 2 commits June 18, 2021 14:04
@seberg
MAINT: Introduce PyArray_PyIntFromIntp to remove ifdef's
3ac7596
@seberg
BUG: Fix arr.flat.index for large or big-endian machines
91cb875 
The type read when exposing was previously int, but has to be
intp.  this would only be visible for >2**31 elements, but is
also visible on big-endian machines.

Closes numpygh-19153
seberg
seberg commented Jun 18, 2021
View reviewed changes
numpy/core/src/multiarray/iterators.c
#if NPY_SIZEOF_INTP <= NPY_SIZEOF_LONG
return PyLong_FromLong((long) self->size);
#else
if (self->size < NPY_MAX_LONG) {
Copy link
Member Author

@seberg seberg Jun 18, 2021

Choose a reason for hiding this comment

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

These must just leftovers from Python 2

@seberg
Copy link
Member Author

seberg commented Jun 18, 2021

Actually, this also needs to be used in at least all of nditer_pywrap.c to unbreak windows for large iterators...

mhvk
mhvk reviewed Jun 20, 2021
View reviewed changes
Copy link
Contributor

@mhvk mhvk left a comment

Choose a reason for hiding this comment

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

That seems all good! But what exactly went wrong on bigendian?

@seberg
Copy link
Member Author

seberg commented Jun 20, 2021

The INT is wrong and has half size, so the actual intp, has the size of <int_upper><int_lower> on big endian and <int_lower><int_upper> on little, so you only get the all zero high bytes on big-endian. (or something along those lines...)

@mhvk
Copy link
Contributor

mhvk commented Jun 20, 2021

Failure unrelated (see #19285)

@mattip mattip merged commit 20e14f3 into numpy:main Jun 21, 2021
@mattip
Copy link
Member

mattip commented Jun 21, 2021

Thanks @seberg

@seberg seberg deleted the issue-19153 branch June 21, 2021 13:16
@pllim pllim mentioned this pull request Jun 21, 2021
Closed
@pllim
Copy link
Contributor

pllim commented Jun 21, 2021

Thank you, all!

@charris charris added the 09 - Backport-Candidate PRs tagged should be backported label Jun 21, 2021
@charris charris added this to the 1.21.1 release milestone Jun 25, 2021
@charris charris mentioned this pull request Jun 25, 2021
Merged
@charris charris removed the 09 - Backport-Candidate PRs tagged should be backported label Jun 25, 2021
@charris charris removed this from the 1.21.1 release milestone Jun 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mhvk mhvk mhvk left review comments

Assignees

No one assigned

Labels

00 - Bug

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

flatiter when big-endian does not update index

5 participants

@seberg @mhvk @mattip @pllim @charris