BUG: NaT handling change 1.21.2 -> 1.21.3 triggering issue in np.median

[stuartarchibald]

Describe the issue:

It looks like something changed between 1.21.2 and 1.21.3 that has lead to a change in NaT handling which triggers an issue/incorrect result in np.median.

EDIT (seberg): Note that this issue also affects np.quantile and np.percentile, an included NaT should lead to a NaT result. But it does not.

This was found whilst adapting https://github.com/numba/numba to accommodate NumPy 1.21.x series.

xref: numba/numba#7483 (comment)

CC @seberg

Reproduce the code example:

import numpy as np
print(np.__version__)

arr = np.arange(10).astype(dtype='m8[s]')
print(arr)
arr[arr.size // 2] = 'NaT'
print(arr)
print(np.median(arr))

Error message:

Running the reproducer on 1.21.2:

$ python sample.py
1.21.2
[0 1 2 3 4 5 6 7 8 9]
[    0     1     2     3     4 'NaT'     6     7     8     9]
3 seconds

Running the reproducer on 1.21.3:

1.21.3
[0 1 2 3 4 5 6 7 8 9]
[    0     1     2     3     4 'NaT'     6     7     8     9]
5 seconds

### NumPy/Python version information:

1.21.2 3.9.7 | packaged by conda-forge | (default, Sep 29 2021, 19:23:11)

and

1.21.3 3.9.7 | packaged by conda-forge | (default, Sep 29 2021, 19:23:11)

[stuartarchibald]

For the removal of doubt. This also appears in 1.21.4:

$ python sample.py
1.21.4
[0 1 2 3 4 5 6 7 8 9]
[    0     1     2     3     4 'NaT'     6     7     8     9]
5 seconds

[seberg]

Seems like NaT handling is broken! Most likely the sorting/partition changed subtly ending up with a different order (where both are incorrect).

(We did change the NaT sorting I think, but I don't expect in a minor release?)

[seberg]

Ah, must be this PR: #20081 that changed it, but I think it was always broken, so doubt we can revert it.

[WarrenWeckesser]

Shouldn't the result be NaT, just like np.median([1.0, 2.0, np.nan, 3.0, 4.0, 5.0]) returns nan?

[seberg]

@stuartarchibald is this issue important for you for the next release, or should we push this off? I am not sure right immediately that this was a worse result then before (rather than a different one)?

[stuartarchibald]

@seberg many thanks for asking. Numba's test suite is patched against testing np.median with input containing NaT for NumPy 1.21 (and it will be for 1.22 too), so it won't fail in the test suite. The Numba implementation is now effectively not matching NumPy (different result!), which is against Numba's exact replication policy, so it would be nice to get this resolved, but is also not critical.

[charris]

I removed the backport tag as the last planned 1.22.x release will be this week. @stuartarchibald What behavior does Numba implement?

[stuartarchibald]

@charris ok, thanks, also good to know that the 1.22 series ends planned releases this week.

What behavior does Numba implement?

Numba implements the pre-1.21.3 behaviour. I'm happy to patch Numba to switch implementation based on NumPy version (there's already quite a bit of this in the Numba code base). What helps with doing this is to have a defined "new" behaviour, but I gather from the above that the current state may be a side effect of #20081 opposed to a direct intention and other "answers" to the MWR in the OP may be more desirable?

[seberg]

Yeah, it seems pretty clear to me that the correct result would be NaT with np.nanmedian probably already returning a version which just ignores the NaT entries.
So if you want to get ahead of NumPy, that is what I would suggest implementing and ignoring whatever NumPy does :).

[charris]

Going to push this off to 1.25