Skip to content

Commit

Permalink
handle empty collections with tmapreduce and tforeach
Browse files Browse the repository at this point in the history
  • Loading branch information
carstenbauer committed Mar 17, 2024
1 parent fb67f6d commit ffc44c1
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 1 deletion.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
-------------
Expand Down
10 changes: 9 additions & 1 deletion src/implementation.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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,
Expand Down
14 changes: 14 additions & 0 deletions test/runtests.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit ffc44c1

Please sign in to comment.