Skip to content

Commit

Permalink
LinSpace: improve length 0 and 1 corner cases.
Browse files Browse the repository at this point in the history
The length zero and one cases for LinSpace are really weird – arguably
they simply shouldn't be allowed at all. However, I think for usability
we really need to support these. This change uses -1 and 0 as special
values of the r.len field, encoding, somewhat counterintuitively, real
lengths of 0 and 1, respectively. The reason for this is so that the
element access computations for the length one case has r.len-1 == -1
and only flips sign, avoiding overflow when r.start is very large. In
the length zero case, r.len-1 == -2, so we need to divide by -2 to get
correct first and last values back. This is susceptible to overflow,
but since these values are kind of nonsense in the zero case, this
seems to be a preferrable situation.

There are still cases where the (r.len-i)*r.start + (i-1)*r.stop can
overflow for longer ranges, and it's unclear to me whether you can
handle those correctly with the same LinSpace type. This plus the
complexity of handling corner cases like lengths zero and one, makes
me wonder if it's really a good idea to make this a type at all.
  • Loading branch information
StefanKarpinski committed Mar 25, 2015
1 parent 848f915 commit 1ef8a7e
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 5 deletions.
14 changes: 10 additions & 4 deletions base/range.jl
Original file line number Diff line number Diff line change
Expand Up @@ -175,8 +175,14 @@ LinSpace(start::FloatingPoint, stop::FloatingPoint, len::Real, divisor::Real) =
LinSpace{promote_type(typeof(start),typeof(stop),typeof(divisor))}(start,stop,len,divisor)

function linspace{T<:FloatingPoint}(start::T, stop::T, len::Int)
0 <= len || error("invalid length: linspace($start, $stop, $len)")
1 != len || start == stop || error("start != stop with len == 1: linspace($start, $stop, $len)")
0 <= len || error("linspace($start, $stop, $len): negative length")
if len == 0
return LinSpace(-start, -stop, -1, convert(T,2))
end
if len == 1
start == stop || error("linspace($start, $stop, $len): endpoints differ")
return LinSpace(-start, -start, 0, one(T))
end
n = len - 1
a0, b = rat(start)
a = convert(T,a0)
Expand Down Expand Up @@ -218,15 +224,15 @@ isempty(r::LinSpace) = length(r) == 0
step(r::StepRange) = r.step
step(r::UnitRange) = 1
step(r::FloatRange) = r.step/r.divisor
step(r::LinSpace) = (r.stop - r.start)/r.divisor
step{T}(r::LinSpace{T}) = ifelse(r.len <= 0, convert(T,NaN), (r.stop-r.start)/r.divisor)

function length(r::StepRange)
n = Integer(div(r.stop+r.step - r.start, r.step))
isempty(r) ? zero(n) : n
end
length(r::UnitRange) = Integer(r.stop - r.start + 1)
length(r::FloatRange) = Integer(r.len)
length(r::LinSpace) = r.len
length(r::LinSpace) = r.len + signbit(r.len - 1)

function length{T<:Union(Int,UInt,Int64,UInt64)}(r::StepRange{T})
isempty(r) && return zero(T)
Expand Down
2 changes: 1 addition & 1 deletion test/ranges.jl
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,7 @@ for T = (Float32, Float64,),# BigFloat),
vals = T[a:s:a+(n-1)*s;]./den
r = start:step:stop
@test [r;] == vals
n == 1 || @test [linspace(start, stop, length(r));] == vals
@test [linspace(start, stop, length(r));] == vals
# issue #7420
n = length(r)
@test [r[1:n];] == [r;]
Expand Down

0 comments on commit 1ef8a7e

Please sign in to comment.