data-apis / array-api

RFC document, tooling and other content related to the array API standard
https://data-apis.github.io/array-api/latest/
MIT License
205 stars 42 forks source link

Add `signbit` specification for determining whether a sign bit is set #705

Closed kgryte closed 7 months ago

kgryte commented 7 months ago

This PR

kgryte commented 7 months ago

One sentence I'd add to the returns section is that for negative values the result is expected to be True, and for positive values False. That won't be completely obvious otherwise to readers not familiar with the low-level details.

@rgommers Isn't that already covered in the special cases section?

    - If ``x_i`` is a positive (i.e., greater than ``0``) finite number, the result is ``False``.
    - If ``x_i`` is a negative (i.e., less than ``0``) finite number, the result is ``True``.

Ref: https://github.com/data-apis/array-api/pull/705/files#diff-153d068dcea18980d39e9465019663509adce3ee4842a48738b37a2a64c693f5R2244-R2245

kgryte commented 7 months ago

I assume input shape-preserving is an implicit assumption, given that signbit just follows other elementwise docs?

@leofang That is correct. We don't explicitly mention the expected output shape across the element-wise specifications, but we should probably be explicit. I can submit a follow-up PR to address this more broadly.

rgommers commented 7 months ago

Isn't that already covered in the special cases section?

No, special cases are special for a reason, and should not be needed to deduce the main behavior of a function. The text always has to make sense if you'd simply delete the special cases section.

kgryte commented 7 months ago

The text always has to make sense if you'd simply delete the special cases section.

IMO this is splitting hairs. The main behavior is precisely as described, as the API should not be confused with the signum function. Meaning, using signbit to test less than or greater than zero is likely an anti-pattern; one can use one of sign, less, or greater instead. Based on a survey of docs,

rgommers commented 7 months ago

It is not a big ask to add something like: "if the sign bit of a value is set, i.e. has value 1 or the value has a minus sign, the result will be False". Or add the math expression like the sign function (x / |x|) for clarity. I didn't ask for fun, I needed to actually double check when I first read it.

kgryte commented 7 months ago

@rgommers I've added an extended description.

rgommers commented 7 months ago

Two approvals and everyone was happy with this addition, so I'll hit the green button here.