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

ispow2(x) for non-Integer x #37635

Merged
merged 3 commits into from
Sep 21, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
ispow2(x) for non-Integer x
  • Loading branch information
stevengj committed Sep 17, 2020
commit 056ecdb82a79d95446c99c65c4151faa1bcb0e30
1 change: 1 addition & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ Standard library changes
by passing a tuple for `dims`, and defaults to reversing all dimensions; there is also a multidimensional
in-place `reverse!(A; dims)` ([#37367]).
* The function `isapprox(x,y)` now accepts the `norm` keyword argument also for numeric (i.e., non-array) arguments `x` and `y` ([#35883]).
* `ispow2(x)` now supports non-`Integer` arguments `x` ([#37635]).
* `view`, `@view`, and `@views` now work on `AbstractString`s, returning a `SubString` when appropriate ([#35879]).
* All `AbstractUnitRange{<:Integer}`s now work with `SubString`, `view`, `@view` and `@views` on strings ([#35879]).
* `sum`, `prod`, `maximum`, and `minimum` now support `init` keyword argument ([#36188], [#35839]).
Expand Down
2 changes: 2 additions & 0 deletions base/float.jl
Original file line number Diff line number Diff line change
Expand Up @@ -725,6 +725,8 @@ function issubnormal(x::T) where {T<:IEEEFloat}
(y & exponent_mask(T) == 0) & (y & significand_mask(T) != 0)
end

ispow2(x::AbstractFloat) = !iszero(x) && frexp(x)[1] == 0.5

@eval begin
typemin(::Type{Float16}) = $(bitcast(Float16, 0xfc00))
typemax(::Type{Float16}) = $(Inf16)
Expand Down
15 changes: 13 additions & 2 deletions base/intfuncs.jl
Original file line number Diff line number Diff line change
Expand Up @@ -368,9 +368,9 @@ _prevpow2(x::Unsigned) = one(x) << unsigned((sizeof(x)<<3)-leading_zeros(x)-1)
_prevpow2(x::Integer) = reinterpret(typeof(x),x < 0 ? -_prevpow2(unsigned(-x)) : _prevpow2(unsigned(x)))

"""
ispow2(n::Integer) -> Bool
ispow2(n::Number) -> Bool

Test whether `n` is a power of two.
Test whether `n` is an integer power of two.
stevengj marked this conversation as resolved.
Show resolved Hide resolved

# Examples
```jldoctest
Expand All @@ -379,8 +379,19 @@ true

julia> ispow2(5)
false

julia> ispow2(4.5)
false

julia> ispow2(0.25)
true

julia> ispow2(1//8)
true
```
"""
ispow2(x::Number) = isreal(x) && ispow2(real(x))
Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this should be restricted to Complex, since there isn't a fallback covering all Real values.

Copy link
Member Author

@stevengj stevengj Sep 19, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like there should be a way to throw a MethodError for common patterns like this, rather than StackOverflowError. I've bumped into similar issues many times, e.g. with f(x) = f(float(x)) patterns.

Could we implement something like

@norecurse ispow2(x::Number) = isreal(x) && ispow2(real(x))

?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(For example, this Number method would work perfectly well for quaternion types. It seems a shame to lose genericity due to dispatch limitations.)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree.

Copy link
Contributor

@JeffreySarnoff JeffreySarnoff Sep 19, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With @stevengj's approach ^, the work of making types consistent inside and collaboratively correct outside is a touch simpler.

@norecurse fn(x::Real) = fn(float(x))

dispatch resolves giving attentive safety


ispow2(x::Integer) = x > 0 && count_ones(x) == 1

"""
Expand Down
1 change: 1 addition & 0 deletions base/rational.jl
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,7 @@ typemin(::Type{Rational{T}}) where {T<:Integer} = unsafe_rational(T, zero(T), on
typemax(::Type{Rational{T}}) where {T<:Integer} = unsafe_rational(T, one(T), zero(T))

isinteger(x::Rational) = x.den == 1
ispow2(x::Rational) = (x.den == 1 && ispow2(x.num)) || (x.num == 1 && ispow2(x.den))

+(x::Rational) = unsafe_rational(+x.num, x.den)
-(x::Rational) = unsafe_rational(-x.num, x.den)
Expand Down
7 changes: 7 additions & 0 deletions test/complex.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1187,3 +1187,10 @@ end

# complex with non-concrete eltype
@test_throws ErrorException complex(Union{Complex{Int}, Nothing}[])

@testset "ispow2" begin
@test ispow2(4+0im)
@test ispow2(0.25+0im)
@test !ispow2(4+5im)
@test !ispow2(7+0im)
end
11 changes: 11 additions & 0 deletions test/floatfuncs.jl
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,17 @@ end
end
end

@testset "ispow2" begin
for T in (Float16,Float32,Float64,BigFloat)
for x in (0.25, 1.0, 4.0, exp2(T(exponent(floatmax(T)))), exp2(T(exponent(floatmin(T)))))
@test ispow2(T(x))
end
for x in (1.5, 0.0, 7.0, NaN, Inf)
@test !ispow2(T(x))
end
end
end

@testset "round" begin
for elty in (Float32, Float64)
x = rand(elty)
Expand Down
8 changes: 8 additions & 0 deletions test/rational.jl
Original file line number Diff line number Diff line change
Expand Up @@ -594,3 +594,11 @@ end
@test -1//5 * 0x3//0x2 == 0x3//0x2 * -1//5 == -3//10
@test -2//3 * 0x1 == 0x1 * -2//3 == -2//3
end


@testset "ispow2" begin
@test ispow2(4//1)
@test ispow2(1//8)
@test !ispow2(3//8)
@test !ispow2(0//1)
end