Make mapreduce also work with Range{BigInt}

[ghost]

I noticed the following behavior: when calling one of

prod(1:big(16))
reduce(*, 1:big(16))

this results in

ERROR: `mapreduce_impl` has no method matching mapreduce_impl(::IdFun, ::MulFun, ::UnitRange{BigInt}, ::Int64, ::BigInt)
 in _mapreduce at reduce.jl:168
 in prod at reduce.jl:265

(On the other hand,

prod(1:big(15))
reduce(*, 1:big(15))
foldl(*, 1:big(16))
factorial(big(16))

all work fine.)

I propose this commit, which fixes that behavior, but should

mapreduce_pairwise_impl(f, op, A::AbstractArray, ifirst::Int, ilast::Int, blksize::Int)
mapreduce_seq_impl(f, op::AddFun, a::AbstractArray, ifirst::Int, ilast::Int)
mapreduce_impl(f, op::AddFun, A::AbstractArray, ifirst::Int, ilast::Int)
mapreduce_impl(f, op::MaxFun, A::AbstractArray, first::Int, last::Int)
mapreduce_impl(f, op::MinFun, A::AbstractArray, first::Int, last::Int)
mapreduce_impl(f, op::AndFun, A::AbstractArray, ifirst::Int, ilast::Int)
mapreduce_impl(f, op::OrFun, A::AbstractArray, ifirst::Int, ilast::Int)

possibly also be changed in the same way, even though their last arguments' being Int does not break anything (that I could find)?

[ivarne]

Looks reasonable and bonus for adding a test!

I feel that this is only fixing a symptom rather than a the general issue. AbstractArrays that can't use native Ints for indexing is kind of tricky. Not sure if we can really fix that tough.

[ivarne]

In case anyone else is confused why 15 works and not 16, it should be pretty obvious from the implementation of _mapreduce. It looks like we have some invalid assumptions there.

[ghost]

Given isa(1:(big(typemax(Int)) + 1), AbstractArray) == true, how else would one index all the data?

And is there any particular reason why AbstractArray is used in this case instead of the iterator interface, like in foldl? Is it because this is potentially meant to be computed in a distributed fashion? If so, where does this happen?

[ivarne]

I didn't say it was wrong for AbstractArrays to use larger types for indexing. I just said that I would like a better fix.

I looked at your code again now, and got a feeling that with your patch prod(big(typemax(Int)):big(typemax(Int))+16) will hang forever.

I think we have 3 good reasons to not use a foldl style implementation

• Keep the associativity undefined so that we can utilize multithreading in the future.
• It has better error accumulating behaviour for many FloatingPoint operations
• It keeps BigInt working more with smaller (=faster) numbers.

[ivarne]

Sorry, I was wrong about the hang. You'd need a range with length(range) > typemax(Int) for the hang to occur, and a 3 GHz processor will spend 10 years doing that many clock cycles. Maybe we should just convert n to an Int in _mapreduce, with a overflow checking conversion?

[ivarne]

NOT good to follow the style of broken tests from the previous lines!

We need to add @test in front of all of those. (I'm working on a fix)

[ghost]

Ah ok I did not notice your commit. Feel free to close this.

[ivarne]

Yeah! We crossed each other. I had guessed so much on this issue, that I had to actually try and see that I was not just speaking nonsense. Currently I think my PR is more complete, so unless you want to clean this PR up to get a commit in the history of Julia, I'll just merge mine and close this.

We love and value new contributors tough, and I don't want you to feel I "stole" your PR. Please ask if you have any trouble cleaning up the PR.

[ghost]

No problem ;)