Repository: JuliaFolds2/OhMyThreads.jl (julia)

Issue:
Problem with nested loops
I am trying to get @tasks to work with a nested loop and am able to
find what I need in the docs. Herewith a MWE:

I've tried using ```@set``` but with no luck.

```julia
function looper(n = 10)
    A = rand(n, n)
    for k = 1:n
        A[:, k] .*= 2.0
        @tasks for j = k+1:n
            for i = k+1:n
                A[i, j] -= A[i, k] * A[k, j]
            end
        end
    end
end
```
I run Julia from a directory with only this file in it.

```julia
% julia --project=.
               _
   _       _ _(_)_     |  Documentation: https://docs.julialang.org
  (_)     | (_) (_)    |
   _ _   _| |_  __ _   |  Type "?" for help, "]?" for Pkg help.
  | | | | | | |/ _` |  |
  | | |_| | | | (_| |  |  Version 1.10.2 (2024-03-01)
 _/ |\__'_|_|_|\__'_|  |  Official https://julialang.org/ release
|__/                   |

julia> using OhMyThreads

julia> include("looper.jl")
looper (generic function with 2 methods)

julia> looper()
ERROR: ArgumentError: You must either indicate the desired number of chunks (n) or the target size of a chunk (size).
Stacktrace:
  [1] missing_input_err()
    @ ChunkSplitters ~/.julia/packages/ChunkSplitters/R5lnx/src/ChunkSplitters.jl:118
  [2] getchunk(itr::UnitRange{Int64}, ichunk::Int64; n::Int64, size::Int64, split::Symbol)
    @ ChunkSplitters ~/.julia/packages/ChunkSplitters/R5lnx/src/ChunkSplitters.jl:274
  [3] getchunk
    @ ~/.julia/packages/ChunkSplitters/R5lnx/src/ChunkSplitters.jl:273 [inlined]
  [4] getchunk
    @ ~/.julia/packages/ChunkSplitters/R5lnx/src/ChunkSplitters.jl:293 [inlined]
  [5] iterate
    @ ~/.julia/packages/ChunkSplitters/R5lnx/src/ChunkSplitters.jl:142 [inlined]
  [6] iterate
    @ ~/.julia/packages/ChunkSplitters/R5lnx/src/ChunkSplitters.jl:141 [inlined]
  [7] iterate
    @ ./generator.jl:44 [inlined]
  [8] collect(itr::Base.Generator{ChunkSplitters.Chunk{…}, OhMyThreads.Implementation.var"#13#21"{…}})
    @ Base ./array.jl:834
  [9] map(f::Function, A::ChunkSplitters.Chunk{UnitRange{Int64}, ChunkSplitters.FixedCount})
    @ Base ./abstractarray.jl:3313
 [10] _tmapreduce(f::Function, op::Function, Arrs::Tuple{…}, ::Type{…}, scheduler::DynamicScheduler{…}, mapreduce_kwargs::@Kwargs{…})
    @ OhMyThreads.Implementation ~/.julia/packages/OhMyThreads/N9muZ/src/implementation.jl:52
 [11] #tmapreduce#10
    @ ~/.julia/packages/OhMyThreads/N9muZ/src/implementation.jl:33 [inlined]
 [12] tmapreduce
    @ ~/.julia/packages/OhMyThreads/N9muZ/src/implementation.jl:25 [inlined]
 [13] #tforeach#70
    @ ~/.julia/packages/OhMyThreads/N9muZ/src/implementation.jl:180 [inlined]
 [14] macro expansion
    @ ~/.julia/packages/OhMyThreads/N9muZ/src/macro_impl.jl:45 [inlined]
 [15] looper(n::Int64)
    @ Main ~/OMTest/looper.jl:5
 [16] looper()
    @ Main ~/OMTest/looper.jl:2
 [17] top-level scope
    @ REPL[3]:1
Some type information was truncated. Use `show(err)` to see complete types.

julia> 
```

Pull request:
Closes #86.

See also the cousin issue #87.

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 4185e872..d482d678 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -18,6 +18,7 @@ Version 0.5.0
 - ![BREAKING][badge-breaking] The (already deprecated) `SpawnAllScheduler` has been dropped.
 - ![BREAKING][badge-breaking] The default value for `ntasks`/`nchunks` for `DynamicScheduler` has been changed from `2*nthreads()` to `nthreads()`. With the new value we now align with `@threads :dynamic`. The old value wasn't giving good load balancing anyways and choosing a higher value penalizes uniform use cases even more. To get the old behavior, set `nchunks=2*nthreads()`.
 - ![Bugfix][badge-bugfix] When using the `GreedyScheduler` in combination with `tmapreduce` (or functions that build upon it) there could be non-deterministic errors in some cases (small input collection, not much work per element, see [#82](https://github.com/JuliaFolds2/OhMyThreads.jl/issues/82)). These cases should be fixed now.
+- ![Bugfix][badge-bugfix] We now handle empty collections as input in `tmapreduce` and `tforeach` explicitly ([#86](https://github.com/JuliaFolds2/OhMyThreads.jl/issues/86)). Our general philosophy is to try match the behavior of the serial `Base` functions.
 
 Version 0.4.6
 -------------
diff --git a/src/implementation.jl b/src/implementation.jl
index 25a0795b..ba4823c6 100644
--- a/src/implementation.jl
+++ b/src/implementation.jl
@@ -60,6 +60,10 @@ function tmapreduce(f, op, Arrs...;
         kwargs...)
     mapreduce_kwargs = isgiven(init) ? (; init) : (;)
     _scheduler = _scheduler_from_userinput(scheduler; kwargs...)
+    if isempty(first(Arrs))
+        isempty(mapreduce_kwargs) && reduce_empty_err()
+        return mapreduce_kwargs.init
+    end
 
     # @show _scheduler
     if _scheduler isa SerialScheduler
@@ -74,6 +78,10 @@ end
     throw(ArgumentError("Providing an explicit scheduler as well as direct keyword arguments (e.g. $(kwargstr)) is currently not supported."))
 end
 
+@noinline function reduce_empty_err()
+    throw(ArgumentError("reducing over an empty collection is not allowed; consider supplying `init`"))
+end
+
 treducemap(op, f, A...; kwargs...) = tmapreduce(f, op, A...; kwargs...)
 
 # DynamicScheduler: AbstractArray/Generic
@@ -174,7 +182,7 @@ function _tmapreduce(f,
 end
 
 # NOTE: once v1.12 releases we should switch this to wait(t; throw=false)
-wait_nothrow(t) = Base._wait(t);
+wait_nothrow(t) = Base._wait(t)
 
 # GreedyScheduler
 function _tmapreduce(f,


Test patch:
diff --git a/test/runtests.jl b/test/runtests.jl
index 9174800a..b1b4515b 100644
--- a/test/runtests.jl
+++ b/test/runtests.jl
@@ -341,4 +341,18 @@ end;
     end
 end;
 
+@testset "empty collections" begin
+    for empty_coll in (11:9, Float64[])
+        for f in (sin, x->im*x, identity)
+            for op in (+, *, min)
+                @test_throws ArgumentError tmapreduce(f, op, empty_coll)
+                for init in (0.0, 0, 0.0*im, 0f0)
+                    @test tmapreduce(f, op, empty_coll; init) == init
+                end
+                @test tforeach(f, empty_coll) |> isnothing
+            end
+        end
+    end
+end;
+
 # Todo way more testing, and easier tests to deal with