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

replace number parsing functions with parse and tryparse #10543

Merged
merged 7 commits into from
Mar 17, 2015
Merged
Show file tree
Hide file tree
Changes from 5 commits
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
1 change: 0 additions & 1 deletion base/base.jl
Original file line number Diff line number Diff line change
Expand Up @@ -277,4 +277,3 @@ immutable Nullable{T}
Nullable() = new(true)
Nullable(value::T) = new(false, value)
end

40 changes: 0 additions & 40 deletions base/combinatorics.jl
Original file line number Diff line number Diff line change
Expand Up @@ -49,18 +49,6 @@ function gamma(n::Union(Int8,UInt8,Int16,UInt16,Int32,UInt32,Int64,UInt64))
@inbounds return Float64(_fact_table64[n-1])
end

function factorial(n::Integer)
n < 0 && throw(DomainError())
local f::typeof(n*n), i::typeof(n*n)
f = 1
for i = 2:n
f *= i
end
return f
end

factorial(x::Number) = gamma(x + 1) # fallback for x not Integer

# computes n!/k!
function factorial{T<:Integer}(n::T, k::T)
if k < 0 || n < 0 || k > n
Expand All @@ -75,34 +63,6 @@ function factorial{T<:Integer}(n::T, k::T)
end
factorial(n::Integer, k::Integer) = factorial(promote(n, k)...)

function binomial{T<:Integer}(n::T, k::T)
k < 0 && return zero(T)
sgn = one(T)
if n < 0
n = -n + k -1
if isodd(k)
sgn = -sgn
end
end
k > n && return zero(T)
(k == 0 || k == n) && return sgn
k == 1 && return sgn*n
if k > (n>>1)
k = (n - k)
end
x::T = nn = n - k + 1
nn += 1
rr = 2
while rr <= k
xt = div(widemul(x, nn), rr)
x = xt
x == xt || throw(OverflowError())
rr += 1
nn += 1
end
convert(T, copysign(x, sgn))
end

## other ordering related functions ##
function nthperm!(a::AbstractVector, k::Integer)
k -= 1 # make k 1-indexed
Expand Down
16 changes: 16 additions & 0 deletions base/deprecated.jl
Original file line number Diff line number Diff line change
Expand Up @@ -496,3 +496,19 @@ function to_index{T<:Real}(A::AbstractArray{T})
depwarn("indexing with non Integer AbstractArrays is deprecated", :to_index)
Int[to_index_nodep(x) for x in A]
end

function float_isvalid{T<:Union(Float32,Float64)}(s::AbstractString, out::Array{T,1})
tf = tryparse(T, s)
isnull(tf) || (out[1] = get(tf))
!isnull(tf)
end
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

Why no depwarn here?

A comment refering to #10543 would also be great so that it is easier to see where a deprecation is comming from (without digging into git history).

Copy link
Sponsor Member Author

Choose a reason for hiding this comment

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

This is actually just a utility used by actual deprecated functions. There never was a float_isvalid before.

Copy link

Choose a reason for hiding this comment

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

Unrelated question, in this function isnull is called twice, would it in
general offer a performance gain to save the value from the first call in a
local variable or is the execution time equivalent to that of fetching the
variable? Is there a definable turning point in function complexity at
which saving its output becomes beneficial? Can saving array values as
local variables offer improvement or does Julia automatically detect and
optimize reusable local values?
On Mar 17, 2015 8:01 PM, "Jeff Bezanson" notifications@github.com wrote:

In base/deprecated.jl
#10543 (comment):

@@ -496,3 +496,19 @@ function to_index{T<:Real}(A::AbstractArray{T})
depwarn("indexing with non Integer AbstractArrays is deprecated", :to_index)
Int[to_index_nodep(x) for x in A]
end
+
+function float_isvalid{T<:Union(Float32,Float64)}(s::AbstractString, out::Array{T,1})

  • tf = tryparse(T, s)
  • isnull(tf) || (out[1] = get(tf))
  • !isnull(tf)
    +end

This is actually just a utility used by actual deprecated functions. There
never was a float_isvalid before.


Reply to this email directly or view it on GitHub
https://github.com/JuliaLang/julia/pull/10543/files#r26598614.

Copy link
Sponsor Member Author

Choose a reason for hiding this comment

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

Yes, in general re-using a result is faster than calling a function twice.
However in this case, isnull is so trivial:

isnull(x::Nullable) = x.isnull

that it will be inlined and I'm pretty sure there'd be nothing to gain by saving its result.

In a very small number of cases Julia can automatically avoid redundant work from calling a function multiple times with the same arguments. However this is hard to do in general. For example Arrays are mutable, so it's meaningful to allocate two different ones with the same contents --- you might be planning to mutate them differently. So we can't do this optimization in such cases. Much more can be said but this topic gets complicated quickly.


function float32_isvalid(s::AbstractString, out::Array{Float32,1})
depwarn("float32_isvalid is deprecated, use tryparse(Float32,s) instead", :float32_isvalid)
float_isvalid(s, out)
end

function float64_isvalid(s::AbstractString, out::Array{Float64,1})
depwarn("float64_isvalid is deprecated, use tryparse(Float64,s) instead", :float64_isvalid)
float_isvalid(s, out)
end
1 change: 1 addition & 0 deletions base/exports.jl
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,7 @@ export
fldmod,
flipsign,
float,
tryparse,
floor,
fma,
frexp,
Expand Down
18 changes: 13 additions & 5 deletions base/gmp.jl
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ export BigInt
import Base: *, +, -, /, <, <<, >>, >>>, <=, ==, >, >=, ^, (~), (&), (|), ($),
binomial, cmp, convert, div, divrem, factorial, fld, gcd, gcdx, lcm, mod,
ndigits, promote_rule, rem, show, isqrt, string, isprime, powermod,
sum, trailing_zeros, trailing_ones, count_ones, base, parseint,
sum, trailing_zeros, trailing_ones, count_ones, base, parseint, tryparse_internal,
serialize, deserialize, bin, oct, dec, hex, isequal, invmod,
prevpow2, nextpow2, ndigits0z, widen, signed

Expand Down Expand Up @@ -76,15 +76,23 @@ signed(x::BigInt) = x
BigInt(x::BigInt) = x
BigInt(s::AbstractString) = parseint(BigInt,s)

function Base.parseint_nocheck(::Type{BigInt}, s::AbstractString, base::Int)
function tryparse_internal(::Type{BigInt}, s::AbstractString, base::Int, raise::Bool)
_n = Nullable{BigInt}()
s = bytestring(s)
sgn, base, i = Base.parseint_preamble(true,s,base)
if i == 0
raise && throw(ArgumentError("premature end of integer: $(repr(s))"))
return _n
end
z = BigInt()
err = ccall((:__gmpz_set_str, :libgmp),
Int32, (Ptr{BigInt}, Ptr{UInt8}, Int32),
&z, SubString(s,i), base)
err == 0 || throw(ArgumentError("invalid BigInt: $(repr(s))"))
return sgn < 0 ? -z : z
if err != 0
raise && throw(ArgumentError("invalid BigInt: $(repr(s))"))
return _n
end
Nullable(sgn < 0 ? -z : z)
end

function BigInt(x::Union(Clong,Int32))
Expand Down Expand Up @@ -217,7 +225,7 @@ function serialize(s, n::BigInt)
serialize(s, base(62,n))
end

deserialize(s, ::Type{BigInt}) = Base.parseint_nocheck(BigInt, deserialize(s), 62)
deserialize(s, ::Type{BigInt}) = get(tryparse_internal(BigInt, deserialize(s), 62, true))

# Binary ops
for (fJ, fC) in ((:+, :add), (:-,:sub), (:*, :mul),
Expand Down
38 changes: 38 additions & 0 deletions base/intfuncs.jl
Original file line number Diff line number Diff line change
Expand Up @@ -333,3 +333,41 @@ function isqrt(x::Union(Int64,UInt64,Int128,UInt128))
s = (s + div(x,s)) >> 1
s*s > x ? s-1 : s
end

function factorial(n::Integer)
n < 0 && throw(DomainError())
local f::typeof(n*n), i::typeof(n*n)
f = 1
for i = 2:n
f *= i
end
return f
end

function binomial{T<:Integer}(n::T, k::T)
k < 0 && return zero(T)
sgn = one(T)
if n < 0
n = -n + k -1
if isodd(k)
sgn = -sgn
end
end
k > n && return zero(T)
(k == 0 || k == n) && return sgn
k == 1 && return sgn*n
if k > (n>>1)
k = (n - k)
end
x::T = nn = n - k + 1
nn += 1
rr = 2
while rr <= k
xt = div(widemul(x, nn), rr)
x = xt
x == xt || throw(OverflowError())
rr += 1
nn += 1
end
convert(T, copysign(x, sgn))
end
4 changes: 2 additions & 2 deletions base/nullable.jl
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@ convert( ::Type{Nullable }, ::Void) = Nullable{Union()}()

function show{T}(io::IO, x::Nullable{T})
if x.isnull
@printf(io, "Nullable{%s}()", repr(T))
print(io, "Nullable{"); show(io, T); print(io, "}()")
else
@printf(io, "Nullable(%s)", repr(x.value))
print(io, "Nullable("); show(io, x.value); print(io, ")")
end
end

Expand Down
2 changes: 2 additions & 0 deletions base/number.jl
Original file line number Diff line number Diff line change
Expand Up @@ -45,3 +45,5 @@ zero(x::Number) = oftype(x,0)
zero{T<:Number}(::Type{T}) = convert(T,0)
one(x::Number) = oftype(x,1)
one{T<:Number}(::Type{T}) = convert(T,1)

factorial(x::Number) = gamma(x + 1) # fallback for x not Integer
120 changes: 72 additions & 48 deletions base/string.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1487,27 +1487,33 @@ parseint{T<:Integer}(::Type{T}, c::Char, base::Integer) = convert(T,parseint(c,b
parseint{T<:Integer}(::Type{T}, c::Char) = convert(T,parseint(c))

function parseint_next(s::AbstractString, i::Int=start(s))
done(s,i) && throw(ArgumentError("premature end of integer: $(repr(s))"))
done(s,i) && (return Char(0), 0, 0)
j = i
c, i = next(s,i)
c, i, j
end

function parseint_preamble(signed::Bool, s::AbstractString, base::Int)
c, i, j = parseint_next(s)

while isspace(c)
c, i, j = parseint_next(s,i)
end
(j == 0) && (return 0, 0, 0)

sgn = 1
if signed
if c == '-' || c == '+'
(c == '-') && (sgn = -1)
c, i, j = parseint_next(s,i)
end
end

while isspace(c)
c, i, j = parseint_next(s,i)
end
(j == 0) && (return 0, 0, 0)

if base == 0
if c == '0' && !done(s,i)
c, i = next(s,i)
Expand All @@ -1522,91 +1528,109 @@ function parseint_preamble(signed::Bool, s::AbstractString, base::Int)
return sgn, base, j
end

function parseint_nocheck{T<:Integer}(::Type{T}, s::AbstractString, base::Int, a::Int)
safe_add{T<:Integer}(n1::T, n2::T) = ((n2 > 0) ? (n1 > (typemax(T) - n2)) : (n1 < (typemin(T) - n2))) ? Nullable{T}() : Nullable{T}(n1 + n2)
safe_mul{T<:Integer}(n1::T, n2::T) = ((n2 > 0) ? ((n1 > div(typemax(T),n2)) || (n1 < div(typemin(T),n2))) :
(n2 < -1) ? ((n1 > div(typemin(T),n2)) || (n1 < div(typemax(T),n2))) :
((n2 == -1) && n1 == typemin(T))) ? Nullable{T}() : Nullable{T}(n1 * n2)

function tryparse_internal{T<:Integer}(::Type{T}, s::AbstractString, base::Int, a::Int, raise::Bool)
_n = Nullable{T}()
sgn, base, i = parseint_preamble(T<:Signed,s,base)
if i == 0
raise && throw(ArgumentError("premature end of integer: $(repr(s))"))
return _n
end
c, i = parseint_next(s,i)
if i == 0
raise && throw(ArgumentError("premature end of integer: $(repr(s))"))
return _n
end

base = convert(T,base)
## FIXME: remove 128-bit specific code once 128-bit div doesn't rely on BigInt
m::T = T===UInt128 || T===Int128 ? typemax(T) : div(typemax(T)-base+1,base)
m::T = div(typemax(T)-base+1,base)
n::T = 0
while n <= m
d::T = '0' <= c <= '9' ? c-'0' :
'A' <= c <= 'Z' ? c-'A'+10 :
'a' <= c <= 'z' ? c-'a'+a : base
d < base || throw(ArgumentError("invalid base $base digit $(repr(c)) in $(repr(s))"))
if d >= base
raise && throw(ArgumentError("invalid base $base digit $(repr(c)) in $(repr(s))"))
return _n
end
n *= base
n += d
if done(s,i)
n *= sgn
return n
return Nullable{T}(n)
end
c, i = next(s,i)
isspace(c) && break
end
(T <: Signed) && (n *= sgn)
while !isspace(c)
d::T = '0' <= c <= '9' ? c-'0' :
'A' <= c <= 'Z' ? c-'A'+10 :
'a' <= c <= 'z' ? c-'a'+a : base
d < base || throw(ArgumentError("invalid base $base digit $(repr(c)) in $(repr(s))"))
'A' <= c <= 'Z' ? c-'A'+10 :
'a' <= c <= 'z' ? c-'a'+a : base
if d >= base
raise && throw(ArgumentError("invalid base $base digit $(repr(c)) in $(repr(s))"))
return _n
end
(T <: Signed) && (d *= sgn)
n = checked_mul(n,base)
n = checked_add(n,d)
done(s,i) && return n

safe_n = safe_mul(n, base)
isnull(safe_n) || (safe_n = safe_add(get(safe_n), d))
if isnull(safe_n)
raise && throw(OverflowError())
return _n
end
n = get(safe_n)
done(s,i) && return Nullable{T}(n)
c, i = next(s,i)
end
while !done(s,i)
c, i = next(s,i)
isspace(c) || throw(ArgumentError("extra characters after whitespace in $(repr(s))"))
if !isspace(c)
raise && throw(ArgumentError("extra characters after whitespace in $(repr(s))"))
return _n
end
end
return n
return Nullable{T}(n)
end
parseint_nocheck{T<:Integer}(::Type{T}, s::AbstractString, base::Int) =
parseint_nocheck(T, s, base, base <= 36 ? 10 : 36)
tryparse_internal{T<:Integer}(::Type{T}, s::AbstractString, base::Int, raise::Bool) =
tryparse_internal(T, s, base, base <= 36 ? 10 : 36, raise)
tryparse{T<:Integer}(::Type{T}, s::AbstractString, base::Int) =
2 <= base <= 62 ? tryparse_internal(T,s,Int(base),false) : throw(ArgumentError("invalid base: base must be 2 ≤ base ≤ 62, got $base"))
tryparse{T<:Integer}(::Type{T}, s::AbstractString) = tryparse_internal(T,s,0,false)

parseint{T<:Integer}(::Type{T}, s::AbstractString, base::Integer) =
2 <= base <= 62 ? parseint_nocheck(T,s,Int(base)) : throw(ArgumentError("invalid base: base must be 2 ≤ base ≤ 62, got $base"))
parseint{T<:Integer}(::Type{T}, s::AbstractString) = parseint_nocheck(T,s,0)
function parseint{T<:Integer}(::Type{T}, s::AbstractString, base::Integer)
(2 <= base <= 62) || throw(ArgumentError("invalid base: base must be 2 ≤ base ≤ 62, got $base"))
get(tryparse_internal(T, s, base, true))
end
parseint{T<:Integer}(::Type{T}, s::AbstractString) = get(tryparse_internal(T, s, 0, true))
parseint(s::AbstractString, base::Integer) = parseint(Int,s,base)
parseint(s::AbstractString) = parseint_nocheck(Int,s,0)
parseint(s::AbstractString) = parseint(Int,s)

## stringifying integers more efficiently ##

string(x::Union(Int8,Int16,Int32,Int64,Int128)) = dec(x)

## string to float functions ##

float64_isvalid(s::AbstractString, out::Array{Float64,1}) =
ccall(:jl_strtod, Int32, (Ptr{UInt8},Ptr{Float64}), s, out) == 0
float32_isvalid(s::AbstractString, out::Array{Float32,1}) =
ccall(:jl_strtof, Int32, (Ptr{UInt8},Ptr{Float32}), s, out) == 0

float64_isvalid(s::SubString, out::Array{Float64,1}) =
ccall(:jl_substrtod, Int32, (Ptr{UInt8},Csize_t,Cint,Ptr{Float64}), s.string, s.offset, s.endof, out) == 0
float32_isvalid(s::SubString, out::Array{Float32,1}) =
ccall(:jl_substrtof, Int32, (Ptr{UInt8},Csize_t,Cint,Ptr{Float32}), s.string, s.offset, s.endof, out) == 0

begin
local tmp::Array{Float64,1} = Array(Float64,1)
local tmpf::Array{Float32,1} = Array(Float32,1)
global parsefloat
function parsefloat(::Type{Float64}, s::AbstractString)
if !float64_isvalid(s, tmp)
throw(ArgumentError("parsefloat(Float64,::AbstractString): invalid number format $(repr(s))"))
end
return tmp[1]
end
tryparse(::Type{Float64}, s::AbstractString) = ccall(:jl_try_strtod, Nullable{Float64}, (Ptr{UInt8},), s)
tryparse(::Type{Float64}, s::SubString) = ccall(:jl_try_substrtod, Nullable{Float64}, (Ptr{UInt8},Csize_t,Cint), s.string, s.offset, s.endof)

function parsefloat(::Type{Float32}, s::AbstractString)
if !float32_isvalid(s, tmpf)
throw(ArgumentError("parsefloat(Float32,::AbstractString): invalid number format $(repr(s))"))
end
return tmpf[1]
end
tryparse(::Type{Float32}, s::AbstractString) = ccall(:jl_try_strtof, Nullable{Float32}, (Ptr{UInt8},), s)
tryparse(::Type{Float32}, s::SubString) = ccall(:jl_try_substrtof, Nullable{Float32}, (Ptr{UInt8},Csize_t,Cint), s.string, s.offset, s.endof)

function parse{T<:Union(Float32,Float64)}(::Type{T}, s::AbstractString)
nf = tryparse(T, s)
isnull(nf) ? throw(ArgumentError("invalid number format $(repr(s)) for $T")) : get(nf)
end

float(x::AbstractString) = parsefloat(x)
parsefloat(x::AbstractString) = parsefloat(Float64,x)
parsefloat{T<:Union(Float32,Float64)}(::Type{T}, s::AbstractString) = parse(T,s)

float(x::AbstractString) = parse(Float64,x)
parsefloat(x::AbstractString) = parse(Float64,x)

float{S<:AbstractString}(a::AbstractArray{S}) = map!(float, similar(a,typeof(float(0))), a)

Expand Down
Loading