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

Keyword arguments for chunks? #19

Closed
carstenbauer opened this issue Dec 20, 2023 · 8 comments
Closed

Keyword arguments for chunks? #19

carstenbauer opened this issue Dec 20, 2023 · 8 comments

Comments

@carstenbauer
Copy link
Member

carstenbauer commented Dec 20, 2023

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 #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
Copy link
Member Author

carstenbauer commented Dec 20, 2023

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
Copy link
Member Author

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

@lmiq
Copy link
Collaborator

lmiq commented Dec 20, 2023

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

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

@lmiq
Copy link
Collaborator

lmiq commented Dec 20, 2023

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

@lmiq lmiq closed this as completed Dec 20, 2023
@carstenbauer
Copy link
Member Author

(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
Copy link
Member Author

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
Copy link
Member Author

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
Copy link
Collaborator

lmiq commented Dec 20, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants