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

Subtracting a range from another range with the same step #10391

Closed
simonster opened this issue Mar 3, 2015 · 10 comments · Fixed by #40320
Closed

Subtracting a range from another range with the same step #10391

simonster opened this issue Mar 3, 2015 · 10 comments · Fixed by #40320

Comments

@simonster
Copy link
Member

I'm not sure if there's anything we can do about this, but this throws an error:

julia> (1:2) - (1:2)
ERROR: ArgumentError: step cannot be zero
 in steprange_last at ./range.jl:47
 in - at operators.jl:331

This can break code that expects that any two AbstractVector{T<:Number}s of the same length can be subtracted from each other.

@simonster
Copy link
Member Author

For two UnitRanges, it seems that presently subtraction will always throw an error. For that case we should probably return an array or some kind of AbstractVector that represents a repeated element. But this doesn't resolve other cases of subtracting ranges with the same step, e.g. (1:1:2) - (1:1:2).

@ivarne
Copy link
Member

ivarne commented Mar 3, 2015

We can have all the - operations on ranges return Array?

@simonster
Copy link
Member Author

We could. Depends whether there are enough cases where people subtract ranges with different steps that the efficiency of returning a range from - is worth it.

@simonster
Copy link
Member Author

This isn't necessarily limited to subtraction, e.g.:

julia> (5:-1:1) + (1:5)
ERROR: ArgumentError: step cannot be zero
 in steprange_last at ./range.jl:47
 in + at operators.jl:331

but that's probably less common in practice.

@simonster simonster changed the title Subtracting a range from itself Subtracting a range from another range with the same step Mar 5, 2015
@mbauman
Copy link
Member

mbauman commented Mar 5, 2015

Or multiplication:

julia> (1:5)*0
ERROR: ArgumentError: step cannot be zero
 in steprange_last at ./range.jl:47
 in .* at range.jl:465
 in * at abstractarray.jl:466

@mbauman
Copy link
Member

mbauman commented Mar 5, 2015

FloatRanges don't have this restriction and actually behave as you'd want:

julia> (2.:6.) - (1.:5.)
1.0:0.0:1.0

julia> collect(ans)
5-element Array{Float64,1}:
 1.0
 1.0
 1.0
 1.0
 1.0

julia> (1:5) * 0.0
0.0:0.0:0.0

julia> collect(ans)
5-element Array{Float64,1}:
 0.0
 0.0
 0.0
 0.0
 0.0

@m-wells
Copy link

m-wells commented Apr 16, 2019

Instead of returning Arrays you can just promote from a StepRange to StepRangeLen

Given the following issue

julia> versioninfo()
Julia Version 1.1.0
Commit 80516ca202 (2019-01-21 21:24 UTC)
Platform Info:
  OS: Linux (x86_64-pc-linux-gnu)
  CPU: Intel(R) Core(TM) i7-6700HQ CPU @ 2.60GHz
  WORD_SIZE: 64
  LIBM: libopenlibm
  LLVM: libLLVM-6.0.1 (ORCJIT, skylake)

julia> a=1:1:5
1:1:5

julia> a[2:end] .- a[1:end-1]
ERROR: ArgumentError: step cannot be zero
Stacktrace:
 [1] steprange_last(::Int64, ::Int64, ::Int64) at ./range.jl:209
 [2] _rangestyle at ./range.jl:199 [inlined]
 [3] _range at ./range.jl:110 [inlined]
 [4] #range#31 at ./range.jl:87 [inlined]
 [5] #range at ./none:0 [inlined]
 [6] - at ./range.jl:1006 [inlined]
 [7] broadcasted at ./broadcast.jl:988 [inlined]
 [8] broadcasted(::Function, ::StepRange{Int64,Int64}, ::StepRange{Int64,Int64}) at ./broadcast.jl:1167
 [9] top-level scope at none:0

julia> a[2] - a[1]
1

Is there a reason we can't implement something like

import Base.-

function -( x :: UnitRange
          , y :: UnitRange
          )
    length(x) != length(y) && throw(DimensionMismatch)
    return StepRangeLen(x.start-y.start, 0, length(x))
end

function -( x :: StepRange
          , y :: StepRange
          )
    length(x) != length(y) && throw(DimensionMismatch)
    if x.step == y.step
        return StepRangeLen(x.start-y.start, 0, length(x))
    else
        return (x.start-y.start):(x.step-y.step):(x.stop-y.stop)
    end
end

Also this seems related to

julia> range(1,step=0,length=10)
ERROR: ArgumentError: step cannot be zero
Stacktrace:
 [1] steprange_last(::Int64, ::Int64, ::Int64) at ./range.jl:209
 [2] Type at ./range.jl:199 [inlined]
 [3] _rangestyle at ./range.jl:112 [inlined]
 [4] _range at ./range.jl:110 [inlined]
 [5] #range#31 at ./range.jl:87 [inlined]
 [6] (::getfield(Base, Symbol("#kw##range")))(::NamedTuple{(:step, :length),Tuple{Int64,Int64}}, ::typeof(range), ::Int64) at ./none:0
 [7] top-level scope at none:0

julia> range(1.0,step=0,length=10)
1.0:0.0:1.0

which could also be promoted to a StepRangeLen type.

@mbauman
Copy link
Member

mbauman commented Apr 16, 2019

Thanks for bringing that here. Yes, I do think we could use StepRangeLen{Int} as the return type for computations like subtracting two UnitRanges. I don't think we've done much testing with StepRangeLen{Int} though, so it'd be good to just double check its test coverage before returning it from range and -(::UnitRange, ::UnitRange).

Unfortunately, implementing support for other permutations that can sometimes return a thing with step size zero is a bigger challenge (e.g., things like (1:2:3) - (4:2:6) and (1:2) * 0 and (1:2:3) * 0 and (1:-1:0) + (1:2), etc). The step size is determined by the values of the range and not just the types, thus sometimes returning StepRange and sometimes returning StepRangeLen introduces a type instability. Everything that uses the returned value has to check if it got a StepRange or a StepRangeLen before doing any computation with it. Now, we've gotten much more efficient at doing this, but it will still introduce a slight performance regression for folks that hadn't been using zero-step values at all. It'd be a tradeoff we'd want to weigh.

The alternative path forward would be to simply wholesale replace StepRange with StepRangeLen (#28072 (comment)) but I'm not sure how feasible that would be given that the latter holds four fields (instead of three) and has a bit more complexity.

@cjdoris
Copy link
Contributor

cjdoris commented May 2, 2019

I've just come up against this problem too, when trying to (fairly naturally) call isapprox on two ranges.

Changing range to return a StepRangeLen whenever step and length are both given would fix all the problems @mbauman mentioned. It's also probably the type the user expects to get when using that combination of arguments.

I believe this is achieved by replacing in range.jl:

# Range from start to stop: range(a, [step=s,] stop=b), no length
_range(start, step,      stop, ::Nothing) = (:)(start, step, stop)
_range(start, ::Nothing, stop, ::Nothing) = (:)(start, stop)

# Range of a given length: range(a, [step=s,] length=l), no stop
_range(a::Real,          ::Nothing,         ::Nothing, len::Integer) = UnitRange{typeof(a)}(a, oftype(a, a+len-1))
_range(a::AbstractFloat, ::Nothing,         ::Nothing, len::Integer) = _range(a, oftype(a, 1),   nothing, len)
_range(a::AbstractFloat, st::AbstractFloat, ::Nothing, len::Integer) = _range(promote(a, st)..., nothing, len)
_range(a::Real,          st::AbstractFloat, ::Nothing, len::Integer) = _range(float(a), st,      nothing, len)
_range(a::AbstractFloat, st::Real,          ::Nothing, len::Integer) = _range(a, float(st),      nothing, len)
_range(a,                ::Nothing,         ::Nothing, len::Integer) = _range(a, oftype(a-a, 1), nothing, len)

_range(a::T, step, ::Nothing, len::Integer) where {T} =
    _rangestyle(OrderStyle(T), ArithmeticStyle(T), a, step, len)
_rangestyle(::Ordered, ::ArithmeticWraps, a::T, step::S, len::Integer) where {T,S} =
    StepRange{T,S}(a, step, convert(T, a+step*(len-1)))
_rangestyle(::Any, ::Any, a::T, step::S, len::Integer) where {T,S} =
    StepRangeLen{typeof(a+0*step),T,S}(a, step, len)

with just

_range(start, step,      stop, ::Nothing) = (:)(start, step, stop)
_range(start, ::Nothing, stop, ::Nothing) = (:)(start, stop)
_range(start, step, ::Nothing, len) = StepRangeLen(start, step, len)
_range(start, ::Nothing, ::Nothing, len) = UnitRange(start, oftype(start,start+len-1))

@bkamins
Copy link
Member

bkamins commented Apr 6, 2020

Ah - it was deep in the past. I will just add a problem I reported in #35370 that is not duplicate of what already is reported here as also is a bug (so when redesigning Base it should be taken also into consideration):

julia> a = 2^60
1152921504606846976

julia> a:-a
1152921504606846976:1152921504606846975

julia> BigFloat(a):BigFloat(-a)
ERROR: ArgumentError: length cannot be negative, got -2305843009213693951

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

Successfully merging a pull request may close this issue.

6 participants