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

Add ispositive, etc. #53677

Open
wants to merge 45 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 32 commits
Commits
Show all changes
45 commits
Select commit Hold shift + click to select a range
d0323ac
add ispositive, etc.
putianyi889 Mar 9, 2024
9b47c02
Update number.jl
putianyi889 Mar 9, 2024
b774f97
specialize Integer and IEEEFloat
putianyi889 Mar 17, 2024
5f76224
add docs
putianyi889 Mar 17, 2024
0cddd58
add tests
putianyi889 Mar 17, 2024
84ac2f8
Update number.jl
putianyi889 Mar 17, 2024
b1b86a5
Update floatfuncs.jl
putianyi889 Mar 17, 2024
f09a032
export methods
putianyi889 Mar 17, 2024
a2bbe5e
update tests
putianyi889 Mar 17, 2024
ff0704f
whitespace
putianyi889 Mar 18, 2024
a2aee14
fix tests
putianyi889 Mar 18, 2024
9276fb4
Update test/numbers.jl
putianyi889 Mar 18, 2024
c63b3ce
Update gmp.jl
putianyi889 Mar 19, 2024
7c03d55
Update mpfr.jl
putianyi889 Mar 19, 2024
bb1201a
Update number.jl
putianyi889 Mar 19, 2024
049133a
Update int.jl
putianyi889 Mar 19, 2024
645ceb6
Update floatfuncs.jl
putianyi889 Mar 19, 2024
2e5941f
Update numbers.jl
putianyi889 Mar 19, 2024
4540780
Update bool.jl
putianyi889 Mar 19, 2024
bfd28c5
Update numbers.jl
putianyi889 Mar 19, 2024
1b71527
Update numbers.jl
putianyi889 Mar 19, 2024
e09fec9
Update base/number.jl
putianyi889 Mar 19, 2024
86c7224
Update gmp.jl
putianyi889 Mar 19, 2024
7a1e53f
Update gmp.jl
putianyi889 Mar 19, 2024
24dd8be
Update mpfr.jl
putianyi889 Mar 19, 2024
f303313
Update gmp.jl
putianyi889 Mar 19, 2024
d8d1baa
Update base/number.jl
putianyi889 Mar 19, 2024
099680e
Update base/number.jl
putianyi889 Mar 19, 2024
5ed79da
add compat note
putianyi889 Mar 20, 2024
bd2dd5f
Update test/numbers.jl
putianyi889 Mar 21, 2024
54dd7c7
Update base/number.jl
putianyi889 Mar 21, 2024
a66321a
Update base/number.jl
putianyi889 Mar 21, 2024
761ed61
Merge branch 'master' into patch-6
putianyi889 Oct 21, 2024
b6c1d88
fix
putianyi889 Oct 21, 2024
d6b9be6
Merge branch 'master' into patch-6
putianyi889 Oct 21, 2024
9ebc0a7
Update NEWS.md
putianyi889 Oct 21, 2024
dfa057e
deprecate `ispos` and `isneg`
putianyi889 Oct 21, 2024
e922bf5
Update bool.jl
putianyi889 Oct 21, 2024
02dec43
remove generic fallback
putianyi889 Oct 21, 2024
7db7a2b
Update NEWS.md
putianyi889 Oct 21, 2024
fc3a0de
Update gmp.jl
putianyi889 Oct 21, 2024
c372cb2
Update gmp.jl
putianyi889 Oct 21, 2024
9e26047
Update gmp.jl
putianyi889 Oct 21, 2024
bfbb69c
Merge branch 'master' into patch-6
putianyi889 Oct 21, 2024
85b3e56
Update missing.jl
putianyi889 Oct 22, 2024
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
1 change: 1 addition & 0 deletions base/bool.jl
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,7 @@ abs(x::Bool) = x
abs2(x::Bool) = x
iszero(x::Bool) = !x
isone(x::Bool) = x
ispositive(x::Bool) = x
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
ispositive(x::Bool) = x
ispositive(x::Bool) = isone(x)

I can't imagine that this ever in the future of forever makes any difference. But I think this is arguably cleaner.

Copy link
Member

Choose a reason for hiding this comment

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

I think it's simpler and clear to see that it's the identity function for booleans, but I guess the definition of isone is right there. You can also probably just let the compiler optimize the fallback.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You can also probably just let the compiler optimize the fallback.

The compiler doesn't do that at present.

julia> code_llvm(x->x>0, (Bool,))
;  @ REPL[6]:1 within `#5`
; Function Attrs: uwtable
define i8 @"julia_#5_5646"(i8 zeroext %0) #0 {
top:
; ┌ @ operators.jl:378 within `>`
; │┌ @ promotion.jl:462 within `<`
; ││┌ @ promotion.jl:393 within `promote`
; │││┌ @ promotion.jl:370 within `_promote`
; ││││┌ @ number.jl:7 within `convert`
; │││││┌ @ boot.jl:784 within `Int64`
; ││││││┌ @ boot.jl:711 within `toInt64`
         %1 = and i8 %0, 1
; ││└└└└└
; ││ @ promotion.jl:462 within `<` @ int.jl:83
    ret i8 %1
; └└
}

julia> code_llvm(x->x, (Bool,))
;  @ REPL[7]:1 within `#7`
; Function Attrs: uwtable
define i8 @"julia_#7_5660"(i8 zeroext %0) #0 {
top:
  ret i8 %0
}

Copy link
Contributor

Choose a reason for hiding this comment

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

I think the compiler is being a bit silly here

julia> code_llvm(x -> x && true, (Bool,))
;  @ REPL[4]:1 within `#5`
define i8 @"julia_#5_249"(i8 zeroext %0) #0 {
top:
  %1 = and i8 %0, 1
  ret i8 %1
}

it doesn't realise could simply return the input argument.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, that's #21712


<(x::Bool, y::Bool) = y&!x
<=(x::Bool, y::Bool) = y|!x
Expand Down
2 changes: 2 additions & 0 deletions base/exports.jl
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,9 @@ export
isinf,
isinteger,
isnan,
isnegative,
isodd,
ispositive,
ispow2,
isqrt,
isreal,
Expand Down
41 changes: 21 additions & 20 deletions base/gmp.jl
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ import .Base: *, +, -, /, <, <<, >>, >>>, <=, ==, >, >=, ^, (~), (&), (|), xor,
trailing_zeros, trailing_ones, count_ones, count_zeros, tryparse_internal,
bin, oct, dec, hex, isequal, invmod, _prevpow2, _nextpow2, ndigits0zpb,
widen, signed, unsafe_trunc, trunc, iszero, isone, big, flipsign, signbit,
sign, hastypemax, isodd, iseven, digits!, hash, hash_integer, top_set_bit
sign, hastypemax, isodd, iseven, digits!, hash, hash_integer, top_set_bit,
ispositive, isnegative

if Clong == Int32
const ClongMax = Union{Int8, Int16, Int32}
Expand Down Expand Up @@ -378,7 +379,7 @@ function (::Type{T})(x::BigInt) where T<:Base.BitSigned
else
0 <= n <= cld(sizeof(T),sizeof(Limb)) || throw(InexactError(nameof(T), T, x))
y = x % T
ispos(x) ⊻ (y > 0) && throw(InexactError(nameof(T), T, x)) # catch overflow
ispositive(x) ⊻ (y > 0) && throw(InexactError(nameof(T), T, x)) # catch overflow
y
end
end
Expand Down Expand Up @@ -601,7 +602,7 @@ Number of ones in the binary representation of abs(x).
count_ones_abs(x::BigInt) = iszero(x) ? 0 : MPZ.mpn_popcount(x)

function top_set_bit(x::BigInt)
isneg(x) && throw(DomainError(x, "top_set_bit only supports negative arguments when they have type BitSigned."))
isnegative(x) && throw(DomainError(x, "top_set_bit only supports negative arguments when they have type BitSigned."))
iszero(x) && return 0
x.size * sizeof(Limb) << 3 - leading_zeros(GC.@preserve x unsafe_load(x.d, x.size))
end
Expand Down Expand Up @@ -700,7 +701,7 @@ function prod(arr::AbstractArray{BigInt})
foldl(MPZ.mul!, arr; init)
end

factorial(n::BigInt) = !isneg(n) ? MPZ.fac_ui(n) : throw(DomainError(n, "`n` must not be negative."))
factorial(n::BigInt) = !isnegative(n) ? MPZ.fac_ui(n) : throw(DomainError(n, "`n` must not be negative."))

function binomial(n::BigInt, k::Integer)
k < 0 && return BigInt(0)
Expand Down Expand Up @@ -732,17 +733,17 @@ isone(x::BigInt) = x == Culong(1)
<(i::Integer, x::BigInt) = cmp(x,i) > 0
<(x::BigInt, f::CdoubleMax) = isnan(f) ? false : cmp(x,f) < 0
<(f::CdoubleMax, x::BigInt) = isnan(f) ? false : cmp(x,f) > 0
isneg(x::BigInt) = x.size < 0
ispos(x::BigInt) = x.size > 0
isnegative(x::BigInt) = x.size < 0
ispositive(x::BigInt) = x.size > 0

signbit(x::BigInt) = isneg(x)
signbit(x::BigInt) = isnegative(x)
flipsign!(x::BigInt, y::Integer) = (signbit(y) && (x.size = -x.size); x)
flipsign( x::BigInt, y::Integer) = signbit(y) ? -x : x
flipsign( x::BigInt, y::BigInt) = signbit(y) ? -x : x
# above method to resolving ambiguities with flipsign(::T, ::T) where T<:Signed
function sign(x::BigInt)
isneg(x) && return -one(x)
ispos(x) && return one(x)
isnegative(x) && return -one(x)
ispositive(x) && return one(x)
return x
end

Expand All @@ -754,12 +755,12 @@ function string(n::BigInt; base::Integer = 10, pad::Integer = 1)
iszero(n) && pad < 1 && return ""
nd1 = ndigits(n, base=base)
nd = max(nd1, pad)
sv = Base.StringVector(nd + isneg(n))
sv = Base.StringVector(nd + isnegative(n))
GC.@preserve sv MPZ.get_str!(pointer(sv) + nd - nd1, base, n)
@inbounds for i = (1:nd-nd1) .+ isneg(n)
@inbounds for i = (1:nd-nd1) .+ isnegative(n)
sv[i] = '0' % UInt8
end
isneg(n) && (sv[1] = '-' % UInt8)
isnegative(n) && (sv[1] = '-' % UInt8)
String(sv)
end

Expand All @@ -769,7 +770,7 @@ function digits!(a::AbstractVector{T}, n::BigInt; base::Integer = 10) where {T<:
# fast path using mpz_get_str via string(n; base)
s = codeunits(string(n; base))
i, j = firstindex(a)-1, length(s)+1
lasti = min(lastindex(a), firstindex(a) + length(s)-1 - isneg(n))
lasti = min(lastindex(a), firstindex(a) + length(s)-1 - isnegative(n))
while i < lasti
# base ≤ 36: 0-9, plus a-z for 10-35
# base > 36: 0-9, plus A-Z for 10-35 and a-z for 36..61
Expand All @@ -778,14 +779,14 @@ function digits!(a::AbstractVector{T}, n::BigInt; base::Integer = 10) where {T<:
end
lasti = lastindex(a)
while i < lasti; a[i+=1] = zero(T); end
return isneg(n) ? map!(-,a,a) : a
return isnegative(n) ? map!(-,a,a) : a
elseif a isa StridedVector{<:Base.BitInteger} && stride(a,1) == 1 && ispow2(base) && base-1 ≤ typemax(T)
# fast path using mpz_export
origlen = length(a)
_, writelen = MPZ.export!(a, n; nails = 8sizeof(T) - trailing_zeros(base))
length(a) != origlen && resize!(a, origlen) # truncate to least-significant digits
a[begin+writelen:end] .= zero(T)
return isneg(n) ? map!(-,a,a) : a
return isnegative(n) ? map!(-,a,a) : a
end
end
return invoke(digits!, Tuple{typeof(a), Integer}, a, n; base) # slow generic fallback
Expand Down Expand Up @@ -912,7 +913,7 @@ module MPQ

# Rational{BigInt}
import .Base: unsafe_rational, __throw_rational_argerror_zero
import ..GMP: BigInt, MPZ, Limb, isneg, libgmp
import ..GMP: BigInt, MPZ, Limb, libgmp

gmpq(op::Symbol) = (Symbol(:__gmpq_, op), libgmp)

Expand Down Expand Up @@ -990,7 +991,7 @@ end
# define add, sub, mul, div, and their inplace versions
function add!(z::Rational{BigInt}, x::Rational{BigInt}, y::Rational{BigInt})
if iszero(x.den) || iszero(y.den)
if iszero(x.den) && iszero(y.den) && isneg(x.num) != isneg(y.num)
if iszero(x.den) && iszero(y.den) && isnegative(x.num) != isnegative(y.num)
throw(DivideError())
end
return set!(z, iszero(x.den) ? x : y)
Expand All @@ -1003,7 +1004,7 @@ end

function sub!(z::Rational{BigInt}, x::Rational{BigInt}, y::Rational{BigInt})
if iszero(x.den) || iszero(y.den)
if iszero(x.den) && iszero(y.den) && isneg(x.num) == isneg(y.num)
if iszero(x.den) && iszero(y.den) && isnegative(x.num) == isnegative(y.num)
throw(DivideError())
end
iszero(x.den) && return set!(z, x)
Expand All @@ -1020,7 +1021,7 @@ function mul!(z::Rational{BigInt}, x::Rational{BigInt}, y::Rational{BigInt})
if iszero(x.num) || iszero(y.num)
throw(DivideError())
end
return set_si!(z, ifelse(xor(isneg(x.num), isneg(y.num)), -1, 1), 0)
return set_si!(z, ifelse(xor(isnegative(x.num), isnegative(y.num)), -1, 1), 0)
end
zq = _MPQ(z)
ccall((:__gmpq_mul, libgmp), Cvoid,
Expand All @@ -1033,7 +1034,7 @@ function div!(z::Rational{BigInt}, x::Rational{BigInt}, y::Rational{BigInt})
if iszero(y.den)
throw(DivideError())
end
isneg(y.num) || return set!(z, x)
isnegative(y.num) || return set!(z, x)
return set_si!(z, flipsign(-1, x.num), 0)
elseif iszero(y.den)
return set_si!(z, 0, 1)
Expand Down
4 changes: 4 additions & 0 deletions base/int.jl
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,10 @@ iseven(n::Real) = isinteger(n) && iszero(rem(Integer(n), 2))
signbit(x::Integer) = x < 0
signbit(x::Unsigned) = false

ispositive(x::Integer) = x > 0
isnegative(x::Integer) = x < 0
isnegative(x::Unsigned) = false
putianyi889 marked this conversation as resolved.
Show resolved Hide resolved

flipsign(x::T, y::T) where {T<:BitSigned} = flipsign_int(x, y)
flipsign(x::BitSigned, y::BitSigned) = flipsign_int(promote(x, y)...) % typeof(x)

Expand Down
6 changes: 5 additions & 1 deletion base/mpfr.jl
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import
isone, big, _string_n, decompose, minmax, _precision_with_base_2,
sinpi, cospi, sincospi, tanpi, sind, cosd, tand, asind, acosd, atand,
uinttype, exponent_max, exponent_min, ieee754_representation, significand_mask,
RawBigIntRoundingIncrementHelper, truncated, RawBigInt
RawBigIntRoundingIncrementHelper, truncated, RawBigInt, ispositive, isnegative


using .Base.Libc
Expand Down Expand Up @@ -1062,6 +1062,10 @@ isfinite(x::BigFloat) = !isinf(x) && !isnan(x)
iszero(x::BigFloat) = x.exp == mpfr_special_exponent_zero
isone(x::BigFloat) = x == Clong(1)

# In theory, `!iszero(x) && !isnan(x)` should be the same as `x.exp > mpfr_special_exponent_nan`, but this is safer.
ispositive(x::BigFloat) = !signbit(x) && !iszero(x) && !isnan(x)
isnegative(x::BigFloat) = signbit(x) && !iszero(x) && !isnan(x)
putianyi889 marked this conversation as resolved.
Show resolved Hide resolved

@eval typemax(::Type{BigFloat}) = $(BigFloat(Inf))
@eval typemin(::Type{BigFloat}) = $(BigFloat(-Inf))

Expand Down
46 changes: 46 additions & 0 deletions base/number.jl
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,52 @@ true
"""
signbit(x::Real) = x < 0

"""
ispositive(x)

Test whether `x > 0`. See also [`isnegative`](@ref).

!!! compat "Julia 1.12"
This function requires at least Julia 1.12.

# Examples
```jldoctest
julia> ispositive(-4.0)
false

julia> ispositive(99)
true

julia> ispositive(0.0)
false
```
"""
ispositive(x) = x > zero(x) # generic fallback
Copy link
Contributor

@nsajko nsajko Oct 21, 2024

Choose a reason for hiding this comment

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

👎

Why is there an Any method? The definition mostly makes sense for Real.

EDIT: it's for stuff like dates and unitful

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure there should be a method for Number, let alone Any.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is because of #35067

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In any case it should not belong to number.jl. If it has to be added, where should it go?

Copy link
Contributor

Choose a reason for hiding this comment

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

It is because of #35067

I don't see the connection. That issue doesn't seem to discuss that part of the design. It barely discusses design at all.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That issue discussed reasons of having the general interface.

Copy link
Member

Choose a reason for hiding this comment

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

I agree with @nsajko. It's always possible to add more generic functionality later, but it's impossible to go backwards. The most generic zero gets is ::Number. If we have a function in base, then non-numeric types (like Dates) can of course choose to extend it if appropriate.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, I get it, thanks.

ispositive(x::Real) = x > 0
Copy link
Contributor

Choose a reason for hiding this comment

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

I probably missed it in the discussions above, but what would be the difference between x > 0 and x > zero(x)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's not related to the discussions. I just realized

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess the lowest real type on the promote totem pole is Bool, so I would suggest x > false as the generic case (although for most types, no promotion actually occurs). I agree that x > zero(x) doesn't add much and requires a type to explicitly define zero (which virtually all will, but it seems like an unnecessary dependency). For types requiring heap allocation (like BigInt, although it is covered by a separate specialization), zero will necessarily require an allocation whereas 0 or false might avoid it when mixed-type comparisons are defined.

In other words, BigInt is an example indicating that x > 0/x > false is likely a more performant fallback than x > zero(x).

Copy link
Contributor

Choose a reason for hiding this comment

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

I was more wondering why having the two different methods to start with, sounds like we should a single good definition, since Real isn't much more specific than Any.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

as BigInt and BigFloat have specialized methods anyway, I guess using zero(x) universally is fine.

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 that having a ::Any and ::Real definition is overkill. I'd drop the ::Real version and keep the ::Any, because some types that can behave like reals are not actually <: Real. DualNumbers.Dual <: Number being such an example.

I still like comparison with 0 or false just because it avoids cases where zero requires allocation or has side effects. But I don't see this causing issues very often so any of these would be fine.

Copy link
Contributor

@nsajko nsajko Oct 21, 2024

Choose a reason for hiding this comment

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

some types that can behave like reals are not actually <: Real. DualNumbers.Dual <: Number being such an example.

I don't think the dual numbers are ordered though? So I guess we don't want ispositive or isnegative defined for dual numbers. EDIT: I was wrong, they seem to be ordered.


"""
isnegative(x)

Test whether `x < 0`. See also [`ispositive`](@ref).

!!! compat "Julia 1.12"
This function requires at least Julia 1.12.

# Examples
```jldoctest
julia> isnegative(-4.0)
true

julia> isnegative(99)
false

julia> isnegative(-0.0)
false
```
"""
isnegative(x) = x < zero(x) # generic fallback
isnegative(x::Real) = x < 0

"""
sign(x)

Expand Down
20 changes: 20 additions & 0 deletions test/numbers.jl
Original file line number Diff line number Diff line change
Expand Up @@ -830,6 +830,26 @@ end
@test cmp(isless, 1, NaN) == -1
@test cmp(isless, NaN, NaN) == 0
end
@testset "ispositive/isnegative" begin
putianyi889 marked this conversation as resolved.
Show resolved Hide resolved
for T in [Base.uniontypes(Base.BitInteger)..., Bool, Rational{Int}, BigInt, Base.uniontypes(Base.IEEEFloat)..., BigFloat, Missing]
values = T[zero(T), one(T)]
if T <: AbstractFloat
push!(values, Inf, NaN) # also check Infs and NaNs
elseif T <: Rational
push!(values, 1//0) # also check Infs
end
@testset "$T" begin
for value in values
@eval begin
@test ispositive($value) === $value > 0
@test ispositive(-$value) === -$value > 0
@test isnegative($value) === $value < 0
@test isnegative(-$value) === -$value < 0
end
end
end
end
end
@testset "Float vs Integer comparison" begin
for x=-5:5, y=-5:5
@test (x==y)==(Float64(x)==Int64(y))
Expand Down
Loading