JuliaFolds2 / ChunkSplitters.jl

Simple chunk splitters for parallel loop executions
MIT License
40 stars 5 forks source link

Keyword arguments for `chunks`? #19

Closed carstenbauer closed 9 months ago

carstenbauer commented 9 months ago

I always felt that type should be a keyword argument, mostly because I generally prefer keyword arguments over optional positional arguments. And in light of https://github.com/m3g/ChunkSplitters.jl/issues/18 it might make sense to even make nchunks a keyword argument, such that one can write:

chunks(array; n=8)
chunks(array; size=8)
chunks(array; n=8, type=:scatter)

Note that this would be similar to range where one can specify either length and/or step and/or start and/or stop.

Only downside is that chunks(array; n=8) is more verbose than chunks(array, 8) but only slightly and also more explicit.

carstenbauer commented 9 months ago

And since I mentioned type, I'm not a fan of this name to begin with. Perhaps distribution/dist is more transparent?

chunks(array; n=8, dist=:scatter)
carstenbauer commented 9 months ago

(I'm bringing these things up now because #17 is anyways breaking 😀)

lmiq commented 9 months ago

Should we default n to Threads.nthreads()? What you think?

(I'm not completely happy by dist or distribution - neither by type).

lmiq commented 9 months ago

Done in the #17 PR. We use n= and distribution= for the keywords.

carstenbauer commented 9 months ago

(I'm not completely happy by dist or distribution - neither by type).

More suggestions: scheme or split

E.g.

chunks(arr; n=8, scheme=:scatter)
carstenbauer commented 9 months ago

Should we default n to Threads.nthreads()?

I don't think we should. First of all, chunking and multithreading are different things. Second of all, it might be strange to call chunks(arr) and get no chunking at all (i.e. 1 chunk) by default (i.e. when running single-threaded).

carstenbauer commented 9 months ago

BTW, similar to range(1,4) (which means range(start=1, stop=4)) we could still have chunks(arr, 8) mean chunks(arr, n=8) if we wanted to.

lmiq commented 9 months ago

I'm fine with the keyword only interface. I actually like it better.

I went for split for now, as the split is in the package name.