Specify zero division behavior

@asmeurer

numpy and PyTorch diverge on the zero division behavior of integral dtype's:

>>> a1 = np.array(1, dtype=np.int)
>>> a2 = np.array(0, dtype=np.int)
>>> a1 // a2
0
>>> a1 % a2
0
>>> t1 = torch.tensor(1, dtype=torch.int)
>>> t2 = torch.tensor(0, dtype=torch.int)
>>> t1 // t2
RuntimeError: ZeroDivisionError
>>> t1 % t2
RuntimeError: ZeroDivisionError

We should either specify the behavior or fix the test suite to exclude zero as second input when testing these functions. cc @asmeurer

The numpy code does give:

>>> a1 // a2
<ipython-input-42-136abf062072>:1: RuntimeWarning: divide by zero encountered in floor_divide
  a1 // a2
0
>>> a1 % a2
<ipython-input-43-bdabd79cdfbc>:1: RuntimeWarning: divide by zero encountered in remainder
  a1 % a2
0

There's an issue about it here: numpy/numpy#5150

I don't think this should be specified in the array API spec, it falls under the warnings/exceptions behavior which is library-specific.

And for the record: there's something to say for either option, but I personally prefer the PyTorch behavior.

@asmeurer

I don't think this should be specified in the array API spec, it falls under the warnings/exceptions behavior which is library-specific.

@asmeurer In that case, we need to exclude it as second input for these functions in the test suite.

The spec should be updated first. The test suite should be easy to update. We already have similar code for the bitwise shift operators, which only accept nonnegative values for the second argument.

@asmeurer

@asmeurer From #199 (comment)

I don't think this should be specified in the array API spec

Thus, I think we can go ahead and adapt the test suite.

I disagree with that. The spec should specify what range of inputs are required for a specific function. If this confused you, it will confuse other people too.

@asmeurer

@asmeurer Not clear to me how you think the specification should be changed. Are you looking for a note that saying zero-division behavior is undefined (i.e., library-specific)?

Yes, just a note is what I'm expecting. For the test suite I am completely skipping testing zero inputs for x2 for floor_divide and remainder(). The spec says "Rounds the result of dividing each element x1_i of the input array x1 by the respective element x2_i of the input array x2 ..." which is meaningless if x2_i is 0 and x2 has an integer dtype.

Perhaps this should be a separate issue, but I also noticed that floor(x) isn't properly specified for infinities.

I guess floor_divide should also be specified when the result of x1/x2 is an infinity.

NumPy has this behavior on floats

>>> np.divide(np.inf, 1.)
inf
>>> np.floor_divide(np.inf, 1.)
nan
>>> np.floor(np.inf)
inf

Presumably it is supposed to work this way, although I find it a little surprising that floor_divide(x, y) != floor(divide(x, y)) in this case.

I believe that same question can arise with log/arcsin/arccos, though it is more common to just return nan there

@asmeurer

@asmeurer Re: floor_divide and infinities. NumPy's behavior matches Python behavior:

In [1]: math.inf//1.0                                                                       
Out[1]: nan

In [2]: math.inf/1.0                                                                        
Out[2]: inf

In [3]: np.floor_divide(np.inf, 1.0)                                                             
Out[3]: nan

In [4]: np.divide(np.inf, 1.0)                                                        
Out[4]: inf

However, NumPy differs from Python in supporting rounding infinities...

In [5]: math.floor(math.inf)                                                                  
---------------------------------------------------------------------------
OverflowError                             Traceback (most recent call last)
<ipython-input-113-dca52b6338a7> in <module>
----> 1 math.floor(math.inf)

OverflowError: cannot convert float infinity to integer

In [6]: np.floor(np.inf)                                                                         
Out[6]: inf

So, I think we have a choice here in what behavior the specification requires. We can either opt for the more consistent behavior (namely, np.inf//1.0 gives inf, rather than nan in order to be consistent with xp.floor(xp.divide(inf, 1.0)), or side with prior art (i.e., NumPy).

Perhaps you have additional thoughts?

FWIW, TensorFlow diverges from NumPy behavior here:

In [1]: (tf.ones([1,1])*np.inf)//1.0
Out[1]: <tf.Tensor: shape=(1, 1), dtype=float32, numpy=array([[inf]], dtype=float32)>

Further updates, NumPy's floor_divide has additional weird behavior:

In [1]: np.floor_divide(-1.0,np.inf)                                                             
Out[1]: -1.0

while TensorFlow does something more sane

In [2]: (tf.ones([1,1])*-1.0)//(np.inf)
Out[2]: <tf.Tensor: shape=(1, 1), dtype=float32, numpy=array([[-0.]], dtype=float32)>

I've opened #329 to add explicit guidance for floor_divide. Note that this guidance diverges from NumPy's behavior and aligns with TensorFlow's behavior as documented above.

Navigation Menu

Repository navigation

Specify zero division behavior #199

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Specify zero division behavior #199

Description

Activity

rgommers commented on Jun 12, 2021

rgommers commented on Jun 12, 2021

pmeier commented on Jun 14, 2021

asmeurer commented on Jun 14, 2021

pmeier commented on Jun 15, 2021

asmeurer commented on Jun 15, 2021

kgryte commented on Jun 15, 2021

asmeurer commented on Aug 31, 2021

asmeurer commented on Aug 31, 2021

asmeurer commented on Aug 31, 2021

arogozhnikov commented on Oct 2, 2021

kgryte commented on Nov 9, 2021

kgryte commented on Nov 9, 2021

kgryte commented on Nov 9, 2021

kgryte commented on Nov 9, 2021

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions