Skip to content
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

BUG: abs(np.int8(-128)) should arguably give an OverflowWarning #21289

Closed
hujie369 opened this issue Apr 3, 2022 · 4 comments · Fixed by #21648
Closed

BUG: abs(np.int8(-128)) should arguably give an OverflowWarning #21289

hujie369 opened this issue Apr 3, 2022 · 4 comments · Fixed by #21648
Labels
00 - Bug sprintable Issue fits the time-frame and setting of a sprint

Comments

@hujie369
Copy link

hujie369 commented Apr 3, 2022

Describe the issue:

The range of np.int8 is -128 to 127.
The ideal range of np.abs(np.int8) is -127 to 128.
But apparently 128 overflow.
So np.abs(-128, dtype=int8) = -128, while the rest of the cases are all correct.
This error is hard to detect and is also found in np.int16.

Reproduce the code example:

import numpy as np
a = np.array([-128, -127], dtype=np.int8)
print(a)
# [-128 -127]

print(np.abs(a))
# [-128, 127]

Error message:

No response

NumPy/Python version information:

numpy: 1.18.5
sys: 3.7.10 (default, Jun 4 2021, 14:48:32)
[GCC 7.5.0]

@charris charris changed the title BUG: <Overflow problem of np.abs on np.int8> BUG: Overflow problem of np.abs on np.int8 Apr 3, 2022
@charris
Copy link
Member

charris commented Apr 3, 2022

This is known behavior. One option would be to return uint8, but that has its own problems as it would lead to type promotions when combined with int8. Best to just accept it and do your own promotion if needed.

In [1]: np.abs(np.array(-128, np.int8)).astype(np.uint8)
Out[1]: 128

@hujie369
Copy link
Author

hujie369 commented Apr 5, 2022

Thanks for your response!

@alonme
Copy link

alonme commented Apr 13, 2022

Hey @charris,

This behavior is a bit confusing, what do you think about printing a warning in cases like these?
Or maybe adding a parameter to abs to do the type promotions if needed?

Otherwise - is there an easy way to check if an array contains the max/min of the dtype

@seberg
Copy link
Member

seberg commented Apr 29, 2022

Yeah, this is mainly a duplicate of e.g. gh-21237 (which is very similar). The basic thing, is that this issue is part of a long list of issues around the fact that it would be good if NumPy was able to give (maybe optional) integer overflow/rollover warnings.

However, we do give warnings for scalar operations. That may be slightly strange, but for scalar operations the check does not impose any significant performance cost.

In that sense, the following arguably should already give a warning:

abs(np.int8(-128))

but does not. I think this is a nice easy thing to fix, although would prefer it happening after gh-21188 is merged.

EDIT: Fixing this requires diving into a fairly complex file of C code (scalarmath.c.src), the relvant code in question will not be complicated though.

@seberg seberg changed the title BUG: Overflow problem of np.abs on np.int8 BUG: abs(np.int8(-128)) should arguably give an OverflowWarning Apr 29, 2022
@seberg seberg added the sprintable Issue fits the time-frame and setting of a sprint label Apr 29, 2022
seberg pushed a commit that referenced this issue Jun 13, 2022
Checks condition a == NPY_MIN_@NAME@ to determine whether an overflow error has occurred for np.int8 type. See #21289 and #21188 (comment) for reference.

This also adds error integer overflow handling to the `-scalar` paths and "activates" a test for the unsigned versions.
A few tests are skipped, because the tests were buggy (they never ran).  These paths require followups to fix.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
00 - Bug sprintable Issue fits the time-frame and setting of a sprint
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants