add init argument to count

[simeonschaub]

Inspired by this StackOverflow answer. Unfortunately, count! also already takes an init argument, which has a different meaning, so this will be the same situation as we currently have with sum/sum!, see #36266.

[simeonschaub]

This is actually a bugfix (although it doesn't occur in 1.5), because count(::Array{Bool}) currently errors when passed a dims argument.

[nalimilan]

Interesting idea. Though I wonder whether the speedup obtained by using a smaller integer type isn't just due to a compiler bug in this particular case: see #37473. I don't see any speedup when counting non-missing values or the number of zeros:

julia> function count_nonmissing(x, T)
           c = zero(T)
           for i in 1:length(x)
               c += @inbounds x[i] !== missing
           end
           return Int(c)
       end
count_nonmissing (generic function with 1 method)

julia> x = rand([missing, rand(Int, 100)...], 1_000_000);

julia> @btime count_nonmissing(x, Int);
  25.500 ms (999485 allocations: 15.25 MiB)

julia> @btime count_nonmissing(x, Int16);
  27.453 ms (983978 allocations: 15.01 MiB)

julia> function count_zero(x, T)
           c = zero(T)
           for i in 1:length(x)
               c += @inbounds x[i] == 0
           end
           return Int(c)
       end

julia> x = rand(1_000_000);

julia> @btime count_zero(x, Int);
  21.660 ms (0 allocations: 0 bytes)

julia> @btime count_zero(x, Int16);
  19.200 ms (0 allocations: 0 bytes)

[simeonschaub]

Hmm, interesting! I would still argue that this is worth it for the consistency alone. It still seems useful to be able to control the accumulation type, perhaps someone wants to count using finite fields, or explicitely needs 64 bits of accuracy on 32-bit platforms.

[simeonschaub]

I don't know who would be best to review this. @tkf, would you be willing to take a quick look at this, since I think you worked a lot on our reduce functions lately?

[tkf]

For now, I guess you can just use sum instead of count?

TBH, after reading #35947 (comment), I've been wondering how much we should be doing for supporting count. Should it even be a target for deprecation? Likely not, but it could be, depending on what goes in Julia 2.0.

[simeonschaub]

I think currently count still has the advantage of being optimized for BitArrays and Array{Bool}, but I don't see why we couldn't apply this optimization to the more general mapreduce case. I guess count still has some merit over sum purely semantically, because "counting" logical values feels more natural than "summing" logical values, but I agree that it otherwise really doesn't add much over sum. A hard deprecation would probably still be a bit out of scope for 1.x, since I'd imagined it's used quite a lot.

[tkf]

being optimized for BitArrays and Array{Bool}

Isn't it better to use these methods from sum as well?

count still has some merit over sum

I feel this is, in principle, minor than assigning different semantics to count as discussed in #35947. Having said that, now that we are in the post-1.0 era, I still don't think such minor renaming should happen, even for 2.0.

[StefanKarpinski]

Bump?

[simeonschaub]

@tkf Are you fine with this as-is, or do you still think, init should be restricted to Integer? Other than this, I think this should be good to go.

[simeonschaub]

Bump.

[simeonschaub]

@rfourquet Would you mind taking another look at this before the branching for 1.6 happens, because count(::Array{Bool}, dims=...) is still broken currently?

[rfourquet]

Would you mind taking another look

Oh sorry, I had missed your previous review request. I will have a look later today.

[rfourquet]

LGTM! Just the docstring needs to document init.

[simeonschaub]

Thank you very much for the thorough review! Hope I addressed it all.

[rfourquet]

Just a couple more nitpicks :)

[simeonschaub]

Is this good to go?

[rfourquet]

I will merge in a couple of days if no objection :)