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

Direct use of Base.signbit considered harmful #1

Open
brainandforce opened this issue Sep 25, 2024 · 0 comments
Open

Direct use of Base.signbit considered harmful #1

brainandforce opened this issue Sep 25, 2024 · 0 comments

Comments

@brainandforce
Copy link
Owner

brainandforce commented Sep 25, 2024

There don't seem to be any guarantees that Base.signbit will be consistently false for positive values and true for negative values:

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.

Originally posted by @mikmoore in JuliaLang/julia#53964 (comment)

The default implementation of Sign(::Real) as of writing is:

Sign(x::T) where T<:Real = reinterpret(Sign, signbit(x))

This works with all Real types provided by Julia, but if one were to define a type that used an opposite sign convention, the answers would be wrong. One possible fallback we could use is

Sign(x::T) where T<:Real = reinterpret(Sign, signbit(Int(sign(x))))

but this runs into a separate problem: negative signed zeros will lose their sign information in the conversion step!

julia> signbit(-0.0)
true

julia> sign(-0.0)
-0.0

julia> Int(sign(-0.0))
0

julia> signbit(Int(sign(-0.0)))
false

The other option is to leave the default definition as-is and require that types with nonstandard sign conventions define their own constructors.

@brainandforce brainandforce changed the title Direct use of Base.signbit is a bad idea Direct use of Base.signbit considered harmful Sep 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant