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, 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