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: Better report integer division overflow (backport) #22230
BUG: Better report integer division overflow (backport) #22230
Conversation
Overflows for remainder/divmod/fmod * If a types minimum value is divided by -1, an overflow will be raised and result will be set to minimum * Handle overflow and return 0 in case of mod related operations
Co-authored-by: Rafael Cardoso Fernandes Sousa <rafaelcfsousa@ibm.com>
* Changed `raise` to `warns` and test for `RuntimeWarning` * Added results check back
This introduces a helper to iterate through "interesting" array cases that could maybe be used in other places. Keep the other test intact, it adds a check for mixed types (which is just casts currently, but cannot hurt) and is otherwise thorough.
I am not certain the unlikely cases make much sense to begin with, but they are certainly not helpful within an unlikely block.
Ah, this part applies well! The scalar paths probably need manual work as the code is actually different unfortunately... (those were different PRs, although that probably hardly matters due to the changes in main) |
@@ -156,16 +156,25 @@ static NPY_INLINE int | |||
* #type = npy_byte, npy_ubyte, npy_short, npy_ushort, npy_int, npy_uint, | |||
* npy_long, npy_ulong, npy_longlong, npy_ulonglong# | |||
* #neg = (1,0)*5# | |||
* #NAME = BYTE, UBYTE, SHORT, USHORT, INT, UINT, |
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.
Ah yes, it was a small change in one of the loop definitions, didn't expect backport to fail as I only checked the diff :), hopefully, works now.
Oddly the tests passed locally, but the error seems valid as we might need to backport another PR if I remember correctly. Taking a look. |
* If a types minimum value is divided by -1, an overflow will be raised and result will be set to minimum
Co-authored-by: Rafael Sousa <90851201+rafaelcfsousa@users.noreply.github.com>
Ok this needed backport of #21727 as well, I have added the commits here itself. |
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.
Thanks, not so bad after all :). This does change super niche warnings, but I think we can risk that one considering how very niche it is.
The new tests should be very extensive, so I do not think we have to worry about having forgotten something. It would be great if you could double check the SIMD results, though @rafaelcfsousa.
The tests are passing without errors on POWER/VSX4. Thank you! |
Thanks @ganesh-k13 . |
Backport of #21507 and #21727.
Handle SIMD division overflow
Merge before: #21507
Part of: #21506
Raising this separately to not blow up the original PR on scalars.
Changes: #21727
Before:
Changes: #21507
Related: #21506
Finishes: #19260
cc: @seberg @rafaelcfsousa