Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

doc: signbit underspecified in the case of negative zero, docs don't seem to align with implementation #53964

Open
nsajko opened this issue Apr 5, 2024 · 14 comments
Labels
docs This change adds or pertains to documentation maths Mathematical functions

Comments

@nsajko
Copy link
Contributor

nsajko commented Apr 5, 2024

The signbit doc string says:

Return true if the value of the sign of x is negative, otherwise false.

This seems to imply that a correct implementation would be x < 0, i.e. return true iff x is negative? However, for when x is a floating-point negative zero, signbit(-0.0) is true even though -0.0 isn't negative.

I guess it might be expected, from the perspective of the representation of floating-point numbers, that signbit(-0.0) is true, given that a negative zero has a set sign bit in its representation. However then signbit doesn't behave as documented? I guess the doc string need adjusting.

julia> f(x) = x < 0
f (generic function with 1 method)

julia> g(x) = x < 0.0
g (generic function with 1 method)

julia> map(x -> x(-0.0), [f, g, signbit])
3-element Vector{Bool}:
 0
 0
 1

julia> versioninfo()
Julia Version 1.10.2
Commit bd47eca2c8a (2024-03-01 10:14 UTC)
Build Info:
  Official https://julialang.org/ release
Platform Info:
  OS: Linux (x86_64-linux-gnu)
  CPU: 8 × AMD Ryzen 3 5300U with Radeon Graphics
  WORD_SIZE: 64
  LIBM: libopenlibm
  LLVM: libLLVM-15.0.7 (ORCJIT, znver2)
Threads: 8 default, 0 interactive, 4 GC (on 8 virtual cores)
@nsajko nsajko added docs This change adds or pertains to documentation maths Mathematical functions labels Apr 5, 2024
@nsajko
Copy link
Contributor Author

nsajko commented Apr 5, 2024

On the other hand, if signbit is to have semantics specific for the most common floating-point implementation, why would it be defined for non-FP types, e.g., Int, at all?

@andrewjradcliffe
Copy link
Contributor

The floating point implementation is consistent with the IEEE 754 isSignMinus predicate, which specifically states that -0.0 and -NaN correspond to true. I was not around when signbit was introduced, but this seems to be the likely explanation.

signbit for floating point numbers is consistent with is_sign_negative.

The implementation of signbit(x::Integer) is just x < 0, which, being critical, assumes an integer representation that does not permit signed zeros (being pragmatic, this will almost always be a valid assumption).

@nsajko
Copy link
Contributor Author

nsajko commented Apr 5, 2024

I get why signbit returns the sign bit (for specific known types, at least). I'm saying the behavior isn't consistent with the documentation (the doc string).

@nsajko
Copy link
Contributor Author

nsajko commented Apr 5, 2024

Or at least it isn't clear. The mathematical definition of "negative" is "less than zero". Specifically, negative zero isn't negative.

@andrewjradcliffe
Copy link
Contributor

the value of the sign of x

The referent is an entity in a computer, not the mathematical abstraction. However, I think I can see the issue you are pointing to: "the value of" summons a slightly different notion. This could be made more clear by re-writing the docstring to unambiguously reference an entity in a computer:

Return true if the sign of x is negative, otherwise false.

@nsajko
Copy link
Contributor Author

nsajko commented Apr 6, 2024

Should the doc string then also define what "sign" means?

@andrewjradcliffe
Copy link
Contributor

Re-reading this today, I think the best clarity can be achieved by stating the following in the docstring:

  1. for floating point formats that support a signed zero, this corresponds to the IEEE 754 isSignMinus predicate; here's an example of a floating point format that lacks signed zero.
  2. for fixed-width, two's complement integers, this indicates whether the number is negative. While it is possible to define a custom type to emulate ones' complement, fixed-width integers, it seems reasonable to assume these will never be used outside of toy examples; in any case, the behavior would be identical to isSignMinus.
  3. for variable-width integers, this indicates whether the number is negative. That is, a bigint might be capable of representing a signed zero internally, but its public interface does not permit it. If a bigint does include signed zero in its public interface, then the behavior is identical to isSignMinus.

It seems excessive to include the above. For people that do not understand computer representations of numbers, this would most likely be confusing rather than helpful -- for those that do understand computer representations of numbers, it is mere pedantry (in fact, quite poorly executed pedantry, as there are many other number representations).

@mbauman
Copy link
Member

mbauman commented Apr 9, 2024

Specifically, negative zero isn't negative.

I get what you’re saying, but this just seems like an unhelpful sort of technically-correct. Yes negative zero is equal to zero, but it behaves as though it’s negative. That’s its point.

Yes, it’s strange to have two equal values that have differing sign bits. It’s also strange to have two equal values whose inv diverges to opposite infinities.

@nsajko
Copy link
Contributor Author

nsajko commented Apr 9, 2024

this just seems like an unhelpful sort of technically-correct

Disagree, given the very reasonable is_negative(x) = x < 0, we have !is_negative(-0.0) (as also pointed out above, perhaps less clearly).

So this isn't about some "mathematical" notion of negativity that would perhaps be overly abstract or technical to consider in the docs. This is simply a case of a doc string getting an edge case wrong in its description.

@mbauman
Copy link
Member

mbauman commented Apr 9, 2024

There's also:

julia> signbit.((-NaN,NaN))
(true, false)

julia> sign.((-NaN,NaN))
(NaN, NaN)

julia> signbit.(sign.((-NaN,NaN)))
(true, false)

If we use the word sign, we shouldn't use or define it differently from [`sign`](@ref), which is worded to (obliquely) include sign(-0.0)'s behavior.

The generic definition of signbit(::Real) is simply (<)(0). So I'd say the docs definition should be similar. Perhaps something as simple as:

Return true if x is negative and false if x is positive. Other values depend upon their representation.

@mikmoore
Copy link
Contributor

mikmoore commented Apr 9, 2024

signbit answers a question about the representation rather than value. It's mostly important for type-specific specialized methods where one needs to do bit-twiddling or for branch cuts where one wants to group +0 with the positive reals and -0 with the negative reals. It also can be used to inspect the sign bit of special values like NaN (although that is seldom useful).

It exists for integers because most integer representations also have single exclusive sign bits (or no way of representing negatives at all), although it is used far less often for them since no native Integer subtypes can represent -0. If one defined a SignMagnitude integer type that could represent both +0 and -0, signbit should act for them exactly how it does for signed zeros in Float64.

signbit should not be used for questions about value. One already has comparisons like x < 0 available for this purpose (even if signbit agrees with this comparison for some types). Also see the proposed functions in #53677.

I'm not quite sure how this should be reflected in the docstring, but I'll agree that the current docstring could use some minor improvements. It might be worth recommending that questions about values should use sign, <, or > instead.

@nsajko
Copy link
Contributor Author

nsajko commented Apr 10, 2024

answers a question about the representation rather than value

My point is that this is not currently apparent from the doc string, apart from in the name of the function.

@mbauman
Copy link
Member

mbauman commented Apr 10, 2024

If there's a bit that isn't "true if x is negative and false if x is positive," then it's not a sign bit.

@mikmoore
Copy link
Contributor

I'll propose language like

Return false if the sign of x is positive and true if the sign of x is negative. For values that are neither positive or negative (e.g., zero or NaN), the return value is implementation-defined. If such a value has an explicit feature (such as a "sign bit") that would otherwise affiliate it with positive or negative values, the result should be chosen based on that feature.

Note that under the definition I wrote, <=(0) is also a legal implementation (but not the fallback I'd suggest) for types without NaNs or signed zeros. In other words, it does not strongly require that we return the top bit for Base.BitInteger, although we do allude to this preference by the "would otherwise affiliate it" clause and explicit mention of a "sign bit."

That is to say that this is at least a minor change. I expect it would not be breaking in practice because one wouldn't (maybe merely shouldn't) abuse signbit for <(0) and we don't plan to actually change any definitions.

Any fix to this docstring is technically breaking in that the existing (and important) behavior for AbstractFloat was not previously allowed. It seems the existing docstring was only written with Base.BitInteger and BigInt in mind.

It would be very slightly less breaking if we stated a preference for signbit to return false for sign-less zeros, but this seems overly strict in that it denies a sensible implementation like signbit(x::Negated) = !signbit(x.value) for a hypothetical wrapper that numerically negates its contents. In most situations (that didn't abuse signbit for <(0)), the sign-less zero result should not matter anyway.

I find the current examples in the docstring to be inadequate. They should emphasize the interesting cases as that's the whole reason to use this over <(0). I'll propose the examples

julia> (signbit(5), signbit(-4))
(false, true)

julia> (signbit(+0.0), signbit(-0.0)) # signed zeros exist for this type
(false, true)

julia> signbit(NaN) == signbit(-NaN) # NaN includes a variable sign bit
false

I'll also propose that we delete the signbit(x::Integer) definition from Base, as it's identical to and redundant with signbit(x::Real). It's also not correct for sign-magnitude integer representations and redundant definitions like this can make it harder to maintain.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs This change adds or pertains to documentation maths Mathematical functions
Projects
None yet
Development

No branches or pull requests

4 participants