divmod(1.0, 0.0) bug

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

divmod(1.0, 0.0) bug

Anirudh Subramanian
Hi all,

There has been a discussion about divmod (1.0, 0.0) bug here : https://github.com/numpy/numpy/issues/14900 and https://github.com/numpy/numpy/pull/16161 . 

SUMMARY

Currently divmod(1.0, 0.0) sets the "Invalid error" and returns (nan, nan). This is not consistent with IEEE 754 standard which says that 1.0/0.0 divisions should return inf and raise dividebyzero error. Although this may not apply to divmod, it should apply to floor_divide and mod. 
I have summarized in the table below, summarizing current state and expected state.

OperatorWarning messageExpected warningResultExpected Result
np.divmodInvalid errorinvalid and dividebyzero ??nan, naninf, nan
np.fmod(1.0, 0.0)Invalid errorInvalidnannan
np.floor_divide(1.0, 0.0)Invalid errorDividebyzeronaninf
np.remainder(1.0, 0.0)Invalid error Invalidnannan


For remainder and fmod above, according to the standard, it is supposed to raise invalid error. We need to change the code to also raise dividebyzero error for floor_divide.

The question is what to do for np.divmod (since this is not defined by standard). My opinion is in this case we need to set both dividebyzero and invalid error flags since its a combination of these two operations.

USER IMPACT

This is going to cause a breaking change/silent incorrect results to atleast some users who are either doing one or two of the following:
1. expecting nans from their output and check isnan but not isinf in their code and accordingly do further computations. 
2. who currently call raise only on invalid and not dividebyzero errors for any of  the above listed operations.

Considering this we can try one of the two things:
1. Create a futurewarning for every divmod(1.0, 0.0) path. This may be very noisy and I cannot think of an action for a user to take to suppress this.
2. Since bug fixes are exempt from backwards compatibility policy just notify in the release notes, maybe after a couple of releases. (More Impactful)

OTHER ISSUES?

Depending on the compiler, and if it implements annex F of the C standard, it may not support 1.0/0.0 operation and may crash. Also this is the case for true_divide also, so we wont be breaking more users than we currently are.

Would like to hear your thoughts about this!

Anirudh


_______________________________________________
NumPy-Discussion mailing list
[hidden email]
https://mail.python.org/mailman/listinfo/numpy-discussion
Reply | Threaded
Open this post in threaded view
|

Re: divmod(1.0, 0.0) bug

Brock Mendel
FWIW in pandas we post-process floordiv (and divmod) ops to get the "Expected Result" behavior from the OP.


On Fri, May 8, 2020 at 11:56 AM Anirudh Subramanian <[hidden email]> wrote:
Hi all,

There has been a discussion about divmod (1.0, 0.0) bug here : https://github.com/numpy/numpy/issues/14900 and https://github.com/numpy/numpy/pull/16161 . 

SUMMARY

Currently divmod(1.0, 0.0) sets the "Invalid error" and returns (nan, nan). This is not consistent with IEEE 754 standard which says that 1.0/0.0 divisions should return inf and raise dividebyzero error. Although this may not apply to divmod, it should apply to floor_divide and mod. 
I have summarized in the table below, summarizing current state and expected state.

OperatorWarning messageExpected warningResultExpected Result
np.divmodInvalid errorinvalid and dividebyzero ??nan, naninf, nan
np.fmod(1.0, 0.0)Invalid errorInvalidnannan
np.floor_divide(1.0, 0.0)Invalid errorDividebyzeronaninf
np.remainder(1.0, 0.0)Invalid error Invalidnannan


For remainder and fmod above, according to the standard, it is supposed to raise invalid error. We need to change the code to also raise dividebyzero error for floor_divide.

The question is what to do for np.divmod (since this is not defined by standard). My opinion is in this case we need to set both dividebyzero and invalid error flags since its a combination of these two operations.

USER IMPACT

This is going to cause a breaking change/silent incorrect results to atleast some users who are either doing one or two of the following:
1. expecting nans from their output and check isnan but not isinf in their code and accordingly do further computations. 
2. who currently call raise only on invalid and not dividebyzero errors for any of  the above listed operations.

Considering this we can try one of the two things:
1. Create a futurewarning for every divmod(1.0, 0.0) path. This may be very noisy and I cannot think of an action for a user to take to suppress this.
2. Since bug fixes are exempt from backwards compatibility policy just notify in the release notes, maybe after a couple of releases. (More Impactful)

OTHER ISSUES?

Depending on the compiler, and if it implements annex F of the C standard, it may not support 1.0/0.0 operation and may crash. Also this is the case for true_divide also, so we wont be breaking more users than we currently are.

Would like to hear your thoughts about this!

Anirudh

_______________________________________________
NumPy-Discussion mailing list
[hidden email]
https://mail.python.org/mailman/listinfo/numpy-discussion

_______________________________________________
NumPy-Discussion mailing list
[hidden email]
https://mail.python.org/mailman/listinfo/numpy-discussion
Reply | Threaded
Open this post in threaded view
|

Re: divmod(1.0, 0.0) bug

Stephan Hoyer-2
On Fri, May 8, 2020 at 4:10 PM Brock Mendel <[hidden email]> wrote:
FWIW in pandas we post-process floordiv (and divmod) ops to get the "Expected Result" behavior from the OP.


On Fri, May 8, 2020 at 11:56 AM Anirudh Subramanian <[hidden email]> wrote:
Hi all,

There has been a discussion about divmod (1.0, 0.0) bug here : https://github.com/numpy/numpy/issues/14900 and https://github.com/numpy/numpy/pull/16161 . 

SUMMARY

Currently divmod(1.0, 0.0) sets the "Invalid error" and returns (nan, nan). This is not consistent with IEEE 754 standard which says that 1.0/0.0 divisions should return inf and raise dividebyzero error. Although this may not apply to divmod, it should apply to floor_divide and mod. 
I have summarized in the table below, summarizing current state and expected state.

OperatorWarning messageExpected warningResultExpected Result
np.divmodInvalid errorinvalid and dividebyzero ??nan, naninf, nan
np.fmod(1.0, 0.0)Invalid errorInvalidnannan
np.floor_divide(1.0, 0.0)Invalid errorDividebyzeronaninf
np.remainder(1.0, 0.0)Invalid error Invalidnannan


For remainder and fmod above, according to the standard, it is supposed to raise invalid error. We need to change the code to also raise dividebyzero error for floor_divide.

The question is what to do for np.divmod (since this is not defined by standard). My opinion is in this case we need to set both dividebyzero and invalid error flags since its a combination of these two operations.

USER IMPACT

This is going to cause a breaking change/silent incorrect results to atleast some users who are either doing one or two of the following:
1. expecting nans from their output and check isnan but not isinf in their code and accordingly do further computations. 
2. who currently call raise only on invalid and not dividebyzero errors for any of  the above listed operations.

Considering this we can try one of the two things:
1. Create a futurewarning for every divmod(1.0, 0.0) path. This may be very noisy and I cannot think of an action for a user to take to suppress this.
2. Since bug fixes are exempt from backwards compatibility policy just notify in the release notes, maybe after a couple of releases. (More Impactful)

I agree, I think these behaviors could be considered bugs and fixed without warning. (However, note that the backwards compatibility policy you link to is only a draft, not officially accepted.)

My guess is that these code paths have been rarely exercised, because floor division and divmod are most useful for integers.
 

OTHER ISSUES?

Depending on the compiler, and if it implements annex F of the C standard, it may not support 1.0/0.0 operation and may crash. Also this is the case for true_divide also, so we wont be breaking more users than we currently are.

Would like to hear your thoughts about this!

Anirudh

_______________________________________________
NumPy-Discussion mailing list
[hidden email]
https://mail.python.org/mailman/listinfo/numpy-discussion
_______________________________________________
NumPy-Discussion mailing list
[hidden email]
https://mail.python.org/mailman/listinfo/numpy-discussion

_______________________________________________
NumPy-Discussion mailing list
[hidden email]
https://mail.python.org/mailman/listinfo/numpy-discussion
Reply | Threaded
Open this post in threaded view
|

Re: divmod(1.0, 0.0) bug

Anirudh Subramanian
Thanks for your inputs, Brock and Stephan.

> (However, note that the backwards compatibility policy you link to is only a draft, not officially accepted.)

Yes, I didn't realize that. Thanks for pointing that out. In practice from the little I have seen, I find the NEP very close to what is being followed in numpy.

Anirudh

On Fri, May 8, 2020 at 5:14 PM Stephan Hoyer <[hidden email]> wrote:
On Fri, May 8, 2020 at 4:10 PM Brock Mendel <[hidden email]> wrote:
FWIW in pandas we post-process floordiv (and divmod) ops to get the "Expected Result" behavior from the OP.


On Fri, May 8, 2020 at 11:56 AM Anirudh Subramanian <[hidden email]> wrote:
Hi all,

There has been a discussion about divmod (1.0, 0.0) bug here : https://github.com/numpy/numpy/issues/14900 and https://github.com/numpy/numpy/pull/16161 . 

SUMMARY

Currently divmod(1.0, 0.0) sets the "Invalid error" and returns (nan, nan). This is not consistent with IEEE 754 standard which says that 1.0/0.0 divisions should return inf and raise dividebyzero error. Although this may not apply to divmod, it should apply to floor_divide and mod. 
I have summarized in the table below, summarizing current state and expected state.

OperatorWarning messageExpected warningResultExpected Result
np.divmodInvalid errorinvalid and dividebyzero ??nan, naninf, nan
np.fmod(1.0, 0.0)Invalid errorInvalidnannan
np.floor_divide(1.0, 0.0)Invalid errorDividebyzeronaninf
np.remainder(1.0, 0.0)Invalid error Invalidnannan


For remainder and fmod above, according to the standard, it is supposed to raise invalid error. We need to change the code to also raise dividebyzero error for floor_divide.

The question is what to do for np.divmod (since this is not defined by standard). My opinion is in this case we need to set both dividebyzero and invalid error flags since its a combination of these two operations.

USER IMPACT

This is going to cause a breaking change/silent incorrect results to atleast some users who are either doing one or two of the following:
1. expecting nans from their output and check isnan but not isinf in their code and accordingly do further computations. 
2. who currently call raise only on invalid and not dividebyzero errors for any of  the above listed operations.

Considering this we can try one of the two things:
1. Create a futurewarning for every divmod(1.0, 0.0) path. This may be very noisy and I cannot think of an action for a user to take to suppress this.
2. Since bug fixes are exempt from backwards compatibility policy just notify in the release notes, maybe after a couple of releases. (More Impactful)

I agree, I think these behaviors could be considered bugs and fixed without warning. (However, note that the backwards compatibility policy you link to is only a draft, not officially accepted.)

My guess is that these code paths have been rarely exercised, because floor division and divmod are most useful for integers.
 

OTHER ISSUES?

Depending on the compiler, and if it implements annex F of the C standard, it may not support 1.0/0.0 operation and may crash. Also this is the case for true_divide also, so we wont be breaking more users than we currently are.

Would like to hear your thoughts about this!

Anirudh

_______________________________________________
NumPy-Discussion mailing list
[hidden email]
https://mail.python.org/mailman/listinfo/numpy-discussion
_______________________________________________
NumPy-Discussion mailing list
[hidden email]
https://mail.python.org/mailman/listinfo/numpy-discussion
_______________________________________________
NumPy-Discussion mailing list
[hidden email]
https://mail.python.org/mailman/listinfo/numpy-discussion

_______________________________________________
NumPy-Discussion mailing list
[hidden email]
https://mail.python.org/mailman/listinfo/numpy-discussion