Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Full scheduler::Symbol support + Keyword argument forwarding #81

Merged
merged 18 commits into from
Mar 13, 2024

Conversation

carstenbauer
Copy link
Member

@carstenbauer carstenbauer commented Mar 12, 2024

Terminology: By "top-level" keyword arguments I refer to keyword arguments that are directly provided to the parallel function API, e.g to tmapreduce, tmap, etc. (in contrast to those provided to a Scheduler constructor).

Principle:
If scheduler isa Symbol, then (most) "top-level" keyword arguments are forwarded to the constructor of the Scheduler that corresponds to the symbol (if any). If scheduler isa Scheduler, to avoid ambiguity, we error if a scheduler option is provided as a "top-level" keyword argument.

Examples (see docstrings and runtests.jl for more):

julia> tmapreduce(sin, +, 1:100; scheduler=:static)
-0.12717101366042027

julia> tmapreduce(sin, +, 1:100; ntasks=2)
-0.12717101366041983

julia> tmapreduce(sin, +, 1:100; chunksize=10, scheduler=:static)
-0.12717101366042027

julia> tmapreduce(sin, +, 1:100; chunksize=10, scheduler=StaticScheduler())
ERROR: ArgumentError: Providing an explicit scheduler as well as direct keyword arguments (e.g. chunksize) is currently not supported.
[...]

julia> @tasks for i in 1:100
           @set ntasks=4*nthreads()
           # non-uniform work...
       end

julia> @tasks for i in 1:100
           @set begin
               scheduler=:static
               nchunks=10
           end
           # work ....
       end

More:

  • StaticScheduler and DynamicScheduler now can also take a ntasks keyword argument which is just an alias for nchunks.
  • DynamicScheduler now defaults to ntasks=nthreads() (again).

Besides implementing these features, I found and fixed some minor code, docstring, and testing bugs.

Closes #76.

@carstenbauer
Copy link
Member Author

I think that tests are failing because of #82.

Copy link
Member

@MasonProtter MasonProtter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow that ended up being quite involved to do! Looks good to me. This is really starting to make me wish there was a package or something that made it easier to deal with "computed kwargs", because doing this right can get quite complex.

@MasonProtter
Copy link
Member

Oops, meant to hit "approve"

@carstenbauer carstenbauer merged commit 7e9ab31 into master Mar 13, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Shorthand for setting number of tasks
2 participants