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

Add buffer objects for collective operations #335

Merged
merged 11 commits into from
Nov 10, 2020
Merged

Conversation

simonbyrne
Copy link
Member

Extending #329 to support collective operations. Thoughts/feedback appreciated.

@simonbyrne simonbyrne force-pushed the sb/buffer-collective branch from 945f4d1 to 18f2149 Compare October 20, 2020 04:50
@simonbyrne simonbyrne marked this pull request as ready for review October 20, 2020 04:50
@simonbyrne
Copy link
Member Author

I've updated this, and tests are now passing. All that's left to do is update the tests to get rid of the deprecation warnings, and polish up the docs.

I think it's a much more elegant design, as it completely separates the send and receive buffers (potentially allowing for different datatypes to be used for each), and fixes #362.

Feedback on buffer names would be appreciated. For now I've gone with UBuffer (for uniformly-sized), and VBuffer (for variable-sized, also matching the V suffix used in the function names). We can also add WBuffer (for MPI_Alltoallw).

Reduction buffers are a bit tricky, since the send and receive buffers have to use the same count and datatype, so I created a new RBuffer type, but this will be hidden from the users in most cases.

I've had to deprecate the allocating functions MPI.Scatter and MPI.Scatterv, as there wasn't a convenient way to figure out the buffer element type and count on a non-root node (and it did seem a bit pointless allocating an empty array just to use as a template).

One potential thing this does allow is using dispatch to combine some functions, e.g. MPI.Gather! and MPI.Gatherv!, since we can dispatch on buffer types. This would require a slight change in that when passing nothing as an unused buffer argument (for a non-root node) you would have to wrap it as MPI.UBuffer(nothing) or MPI.VBuffer(nothing) so that it can correctly determine which method should be called. I haven't done it yet, but it would be a fairly small change to make.

@simonbyrne simonbyrne changed the title RFC: Add ChunkBuffers for collective operations Add buffer objects for collective operations Oct 20, 2020
@simonbyrne
Copy link
Member Author

The other question is whether to change root to be a keyword argument (#423).

@sloede
Copy link
Member

sloede commented Oct 21, 2020

The other question is whether to change root to be a keyword argument (#423).

I would probably not mix this with the buffer changes, as these two components seem to be orthogonal features (at least to me).

@simonbyrne
Copy link
Member Author

I would probably not mix this with the buffer changes, as these two components seem to be orthogonal features (at least to me).

True, but it does avoid another round of deprecations.

@simonbyrne simonbyrne force-pushed the sb/buffer-collective branch from 0020d53 to 3125598 Compare October 27, 2020 19:30
@simonbyrne simonbyrne merged commit 3411efc into master Nov 10, 2020
@simonbyrne simonbyrne deleted the sb/buffer-collective branch November 10, 2020 16:39
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

Successfully merging this pull request may close these issues.

2 participants