ENH: Support integer dtype inputs in rounding functions

[mtsokol]

Hi @seberg @ngoldbaum,

This PR adds support for integer dtype inputs in floor, ceil, and trunc ufuncs to match the Array API standard.

For integer dtype inputs an identity is returned in a loop instead of casting to the float dtype.

[jakevdp]

What about booleans? The Array API spec doesn't mention these here, but it seems like it would be consistent to return boolean outputs for boolean inputs to these functions.

[seberg]

I suppose we have to make a choice for them, and it could go three ways:

1. round is math, and math behaves like integers, as it does for Python for better or worse.
2. We just raise, because rounding booleans is weird. (I.e. a normal deprecation.)
3. Propagate booleans because it is nice to be able to no-op on non-floats.

But if you propagate, does round(True, -1) actually return False? I am not sure what I prefer, but since I doubt we will change True + True in NumPy, maybe just going to the default integer is actually pretty sane?

[mattip]

round is not one of the ufuncs we are dealing with here. Only floor, ceil and trunc. So in that case can we make both bool and int no-ops here? Or am I missing a corner case.

[mtsokol]

I think no-op support for bool dtype also makes sense to me.

[mhvk]

Not a major worry, but just wondering: Aren't there loops for np.positive already? Is it possible to reuse those rather than define new ones?

[mtsokol]

I think there's a loop per each dtype and function, like @TYPE@_@kind@.

Here, for positive we have:

NPY_NO_EXPORT void NPY_CPU_DISPATCH_CURFX(@TYPE@_positive)
(char **args, npy_intp const *dimensions, npy_intp const *steps, void *NPY_UNUSED(func))
{
    UNARY_LOOP_FAST(@type@, @type@, *out = +in);
}

I think for floor, ceil, and trunc we need separate ones anyway to follow the convention dtype_func. Or do you mean changing @TYPE@_positive to @TYPE@_@kind@ and adding an inner repeat1 loop here? (with #kind = floor, ceil, trunc, positive#)

[seberg]

You can just add an alias define to the header file.

[mhvk]

yes, an alias seems the right solution, to avoid compiling the same code four times!

[mtsokol]

Done! I added #define alias pointing to positive loops for floor, ceil, and trunc ufuncs.

But this alias covers only ints, as positive doesn't support boolean dtype. For boolean dtype I added separate no-op loops (I didn't find other ufunc that would be a no-op for booleans), is it OK?

[mtsokol]

Sure! But raising would be a new backward incompatible behavior. Then I propose to leave bool and continue to cast (floor, ceil, and trunc cast bool dtype right now in main). Is it correct?

[mhvk]

Ah, I see what you mean, I hadn't quite realized currently it cast to float. If you do nothing about bool in your PR, does it still cast to float, or does it become int? If we don't pass on bool or raise, I prefer casting to int to casting to float...

If this becomes unclear, one way out is simply not to touch it here, but raise a separate issue to discuss, hoping still to decide before 5.1.

[seberg]

It would go to the first type that matches, which is either uint8 or int8 (dunno).

[mtsokol]

It casts to int8:

In [1]: np.ceil(np.array([1, 0, 1]).astype(bool))
Out[1]: array([1, 0, 1], dtype=int8)

Then let me update it here.

[mtsokol]

Done! Ready from my side!

[mtsokol]

It's ready for another review! (I think CI failures are unrelated as they occur in other PRs as well.)

[seberg]

@mhvk, we discussed this a bit and slightly leaned towards pass through being maybe the simplest (even if we don't touch positive)...

I'll say that I might slightly prefer just deprecating it, but I just don't think it matters much and I won't have squirms with deprecating it later.

I do not like the current int8, but would be just as happy (maybe even more so?) with pass through as default integer, i.e. ??->n loop, that at least aligns with math.ceil(True).

---

There are a lot more float only functions with similar wonky behavior for ints and bools. However, for most of them, the return dtype should just be changed to promote to float64.

[mhvk]

@seberg - I don't really mind much either way, just felt the easiest way to get this (nice) addition in was not to worry about bool at all in this PR, and discuss separately. If we are going to deal with it here, then, given that np.negative and np.positive both do not work, my very slight preference would be to just bail (i.e., deprecate), but, really, I do not care as I think in the end this is a corner case that will happen essentially never.

[mtsokol]

@seberg I added no-op for boolean dtype - it's completed from my side.

[ngoldbaum]

This looks good to me given the decision to make the bool ops a no-op. Also nice and simple.

[mattip]

Thanks @mtsokol