8000 BUG: `__floordiv__` gives NaN and invalid warning for `1//0` · Issue #14900 · numpy/numpy · GitHub
[go: up one dir, main page]

Skip to content

BUG: __floordiv__ gives NaN and invalid warning for 1//0 #14900

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

Closed
DevinShanahan opened this issue Nov 13, 2019 · 15 comments · Fixed by #16161
Closed

BUG: __floordiv__ gives NaN and invalid warning for 1//0 #14900

DevinShanahan opened this issue Nov 13, 2019 · 15 comments · Fixed by #16161
Labels
00 - Bug component: numpy.ufunc Priority: high High priority, also add milestones for urgent issues

Comments

@DevinShanahan
Copy link
Contributor

The __floordiv__ and __mod__ operators do not raise an exception for zero division with np.errstate(divide='raise'), instead replacing those instances with nan as if errstate is in the default state.

import numpy as np
x = np.ones((3, 3))
y = np.zeros((3, 3))

with np.errstate(divide='raise'):
    fdiv = x // y # expecting exception
    mod = x % y # expecting exception

Numpy: 1.17.3

@Sriharsha-hatwar
Copy link

Very curious about working on this. but seems a little tricky, any inputs ? @eric-wieser ?

@seberg
Copy link
Member
seberg commented Nov 25, 2019

Ah, the issue is that npy_divmod in numpy/core/src/npymath/npy_math_internal.h.src has a special path for b == 0. This does not set the divide by 0 error flag (since it is hit before the divide happens). Maybe even reorganizing the code works (not sure what the compilers will do here).
I suppose the fmod causes an invalid warning to be set before, so I wonder if both should be set or only one?

@Sriharsha-hatwar
Copy link
Sriharsha-hatwar commented Nov 25, 2019

Hey @seberg any place where we can learn the internals of this working or an overall architectural view of the system? and what is this extension *.h.src ?

@seberg
Copy link
Member
seberg commented Nov 25, 2019

The development workflow documents describe the .src extension. It is basically a very very simple templating language used in NumPy. It just repeats the code and fills in the @variable@. The overall architecture is sometimes a bit tricky, I would suggest grepping a bit, you will find this function is used in the umath folder which has everything to define ufuncs in it. Since there are some code generators, etc. please just ask if you are confused by something specific.

@anirudh2290
Copy link
Member

hi @seberg, i started looking more into this. is this still an issue. looks like fmod sets nan and probably causes invalid warning to be set. Tried to dig more into this and found : #7709 where it seems to indicate that this is intended behavior and not a bug. I also tried to dig into the code, to see where the invalid flag is set (either through NPY_FPE_INVALID or npy_set_floatstatus_invalid) for fmod but couldn't find it.

@seberg
Copy link
Member
seberg commented May 4, 2020

Seems there are actually two things here, since we unfortunately never fixed the thing that we should return Inf and not NaN.

@anirudh2290 there probably is no explicit setting, the flag probably gets set by the CPU during the modulo operation written in C (or similar). So the correct behaviour is arguably to return inf and set the "divide" warning.

What we need to figure out is which warning is correct. Python returns ZeroDivisionError also for 1 % 0, but that seems wrong when returning NaN (which is arguably correct). Now np.divmod() returns both, so which warning to pick? (I guess the one we currently have).

@seberg seberg changed the title errstate divide has no effect on __floordiv__ and __mod__ BUG: __floordiv__ gives NaN and invalid warning for 1//0 May 5, 2020
@seberg seberg added the triage review Issue/PR to be discussed at the next triage meeting label May 5, 2020
@anirudh2290
Copy link
Member

thanks @seberg! Currently for np.divmod , it behaves differently for scalars and arrays. for scalar it returns (0, 0) and gives a divide by 0 warning. For arrays, it gives an array of nans (for both quotient and remainder) and gives an invalid warning. Is my understanding correct, that your preference is to return (inf, nan) and set the dividebyzero warning for both scalars and arrays ?

import numpy as np
x = np.ones((3, 3))
y = np.zeros((3, 3))

with np.errstate(all='warn'):
    print(np.divmod(1, 0))  # this will warn divide by 0, and print (0, 0)
    print(np.divmod(x, y))  # this will warn invalid and prints array of nans for both

@seberg
Copy link
Member
seberg commented May 5, 2020

I did not realize the 0, 0 result the issue you linked sounds like it should, but I am not sure, maybe I am missing something about divmod being special. 1//0 should return inf (unless you say that the floor(inf) gives NaN, I guess? So honestly, I am not quite sure immediately :)

@seberg seberg modified the milestone: 1.19.0 release May 5, 2020
@seberg seberg added Priority: high High priority, also add milestones for urgent issues and removed triage review Issue/PR to be discussed at the next triage meeting labels May 5, 2020
@anirudh2290
Copy link
Member
anirudh2290 commented May 5, 2020

I guess I was not completely accurate before. Float division for example np.divmod(1.0, 0.0) do print (inf, nan) print(nan, nan). But it has a different behavior for int or long division which is (0, 0).

EDIT: I meant print (nan, nan).

@seberg
Copy link
Member
seberg commented May 5, 2020

Ah, thanks for making it more clear. Right for integers it is a bit more tricky, since the only other thing we could do is either always raise or return INT_MIN, which is also not ideal... I wonder if we have any prior art somewhere... What does C do?

@anirudh2290
Copy link
Member

for c 1/0 is undefined behavior, on my compiler it gives floating point error core dumped. 1.0/0.0 returns inf, conforming to IEEE754 standard.

@anirudh2290
Copy link
Member

Ah, thanks for making it more clear. Right for integers it is a bit more tricky, since the only other thing we could do is either always raise or return INT_MIN, which is also not ideal... I wonder if we have any prior art somewhere... What does C do?

This may be a stupid question but why can't we have the same behavior for int and float, that is set the div error and return inf and nan.

@anirudh2290
Copy link
Member

Ah, thanks for making it more clear. Right for integers it is a bit more tricky, since the only other thing we could do is either always raise or return INT_MIN, which is also not ideal... I wonder if we have any prior art somewhere... What does C do?

This may be a stupid question but why can't we have the same behavior for int and float, that is set the div error and return inf and nan.

okay, my bad. inf doesnt really make sense for int.

@anirudh2290
Copy link
Member

I wonder if we have any prior art somewhere...

Digging into some history, I found this : #7510 , which raises ValueError, the discussion that happened on the issue seems to have converged to this comment : #7510 (comment) . I guess we decided to incrementally fix this and raise ValueError wherever we find such cases ?

@anirudh2290
Copy link
Member

np.power(0, -1) today:

>>> np.power(0, -1)

Evaluating ufunc power
Getting arguments
Finding inner loop
input types:
dtype('int64') dtype('int64')
output types:
dtype('int64')
Executing legacy inner loop
trivial 2 input with allocated output
three operand loop count 1
Returning failure code -1
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ValueError: Integers to negative integer powers are not allowed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
00 - Bug component: numpy.ufunc Priority: high High priority, also add milestones for urgent issues
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants
0