tamnd / tamnd/firepanda

Integer division by zero is null here and infinity in pandas

Open
#252 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Mojo
Stars
1
Forks
0
PR merge metrics
PR metrics pending

Description

Measured against pandas 3.0.5 and firepanda at the head of the Python arithmetic work.

```python
pd.Series([1, 2]) // pd.Series([1, 0]) # [1.0, inf], dtype float64
fp.Series([1, 2]) // fp.Series([1, 0]) # [1, None], dtype int64

pd.Series([1, 2]) % pd.Series([1, 0]) # [0.0, nan], dtype float64
fp.Series([1, 2]) % fp.Series([1, 0]) # [0, None], dtype int64
```

Two things differ and only one of them is obvious. The obvious one is that the undefined row is null here and infinity there. The other is the dtype of the whole column: pandas widens an integer floor division to float64 as soon as any divisor is zero, so a caller who wrote `//` on two integer columns and got an integer column back on every input they tested gets a float column back on the one that has a zero in it. Every row moves, not just the row that divided by zero.

True division already agrees. `pd.Series([1, 2]) / pd.Series([1, 0])` and the firepanda equivalent both give `[1.0, inf]`, because that operation answers float64 whatever its inputs were, so the zero case has somewhere to put an infinity without changing the type of the result.

### Which answer is right

pandas' is not obviously it. A null says the answer does not exist, which is true of one divided by zero on the integers, and it keeps the column's type stable across inputs. An infinity says the answer is a float that no integer column can hold, which is why the type has to move.

The compatibility goal decides it rather than the argument does. Code that ports to firepanda and gets a null where it expected an infinity will take a different branch, and code that expected float64 back and gets int64 will fail at whatever reads the dtype. Both are silent, and a silent difference in an answer is the class of divergence worth the most.

### What it touches

`firepanda/kernel/binary.mojo`, in the FLOORDIV and MOD paths for the integer dtypes. The float paths already agree and should not move.

This wants the same treatment as the widening: the result type of an integer floor division has to be decided by looking for a zero in the divisor before the kernel runs, which is a pass over the divisor that the operation does not otherwise need. That cost is the reason to discuss this rather than just do it, and it is worth measuring whether the check can ride along with the null mask the kernel already builds.

Related to #170, which is the same question asked of NaN rather than of infinity.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.