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

Symmetric inplace operations #28

Open
kurpicz opened this issue Dec 2, 2021 · 13 comments
Open

Symmetric inplace operations #28

kurpicz opened this issue Dec 2, 2021 · 13 comments
Assignees
Milestone

Comments

@kurpicz
Copy link
Member

kurpicz commented Dec 2, 2021

No description provided.

@lukashuebner
Copy link
Member

We have to think about three cases:

  1. The user provides the same buffer to send_buf(...) and receive_buf(...). Possibly via a Span<> object or otherwise encapsulated. We could decide to either not support this or to require the passed objects to have an underlying_buffer_equals_that_of(...) if the user wants the possibility for us to detect this. Note, that MPI forbids passing pointers to overlapping memory regions as send and recv buffer; if you want this, you have to use MPI_INPLACE. I would prefer not to use operator== for the comparison, as two object might be unequal even if they have the same underlying buffer (e.g. a std::vector and a Span<> pointing to the same memory region).
  2. The user explicitly specifies kamping::in_place or MPI_INPLACE as the receive buffer. Imho, it would be nice to support specifying kamping::in_place as a tag, e.g., comm.scan(send_buf(value), kamping::in_place);
  3. The user assigns the recv_buf to the same variable used as the send_buf, i.e., value = comm.reduce(send_buf(value));. This should always yield the expected result, and we can do nothing to optimize this anyway; so we do not need to consider this further.

@Hespian
Copy link
Member

Hespian commented Jul 4, 2022

  1. Can't we just compare .data() for the first case?
  2. I like that proposal. It would be even cooler if we could do that in
  3. Seems sketchy and I'd have to think about this in more detail to see if it would work. I would advice against it ;)

@lukashuebner
Copy link
Member

lukashuebner commented Jul 4, 2022

I like that proposal. It would be even cooler if we could do that in

In ...? :-)

Can't we just compare .data() for the first case?

I thought about that, but wasn't sure if every object has a .data() member. But it probably has, as we're using .data(...) to get the pointer we pass to the MPI function. So 👍

@Hespian
Copy link
Member

Hespian commented Jul 4, 2022

In ...? :-)

Hm.. I forgot what I wanted to say there :/

I thought about that, but wasn't sure if every object has a .data() member. But it probably has, as we're using .data(...) to get the pointer we pass to the MPI function. So +1

The important part is that the parameter object always has .data(), which it does because we wrote it ;)

@Hespian
Copy link
Member

Hespian commented Jul 4, 2022

I like that proposal. It would be even cooler if we could do that in

Actually, let me fix that: I like the idea (which is very close to how the normal MPI interface does it), but I don't like passing a tag without the named parameter syntax ;)

@lukashuebner
Copy link
Member

One of the reasons we decided against the (imho) way prettier send_buf = value interface was exactly such that we can use tags instead of the (imho) ugly kamping::in_place() syntax. Also, tags are “more C++” than our named parameters.

@Hespian
Copy link
Member

Hespian commented Jul 4, 2022

Well yes, but named parameters are always used in kamping.
I don't recall that being a reason against the = syntax (in fact, I don't see a difference between a tag in the ()-Syntax and in the =-Syntax.

@niklas-uhl
Copy link
Member

niklas-uhl commented Feb 13, 2024

Operations supporting inplace:

Asymmmetric: moved to #666

@lukashuebner
Copy link
Member

Thank you, Niklas! :-)

A few small comments:

  • Does the MPI standard enforce, that MPI_INPLACE may only be passed on one or on all ranks (e.g., for allreduce)?
  • I'd prefer if the user simply passed kamping::inplace and not recv_buf(kamping::inplace).
  • Do we explicitly check if the same two buffers are passed to send_buf and recv_buf (either to automatically do the operation inplace or to emit an error message); not zero-overhead

@niklas-uhl
Copy link
Member

  • Does the MPI standard enforce, that MPI_INPLACE may only be passed on one or on all ranks (e.g., for allreduce)?

This depends on the operation, see the list above

@niklas-uhl
Copy link
Member

  • I'd prefer if the user simply passed kamping::inplace and not recv_buf(kamping::inplace).
  • Do we explicitly check if the same two buffers are passed to send_buf and recv_buf (either to automatically do the operation inplace or to emit an error message); not zero-overhead

I ditched the idea of providing inplace explicitely (see #626). The same semantics can be achieved with a lot less confusion by passing send_recv_buf and no send_buf and recv_buf. When a send_recv_buf is passed, we detect that inplace should be used, no implicit comparison of send and recv buffers. If users pass them explicitely instead of using a send_recv_buf, they might have a reason to do so ...

@lukashuebner
Copy link
Member

  • Does the MPI standard enforce, that MPI_INPLACE may only be passed on one or on all ranks (e.g., for allreduce)?

This depends on the operation, see the list above

I see, the list is what the MPI standard enforces; I read it as what we want to support. 👍

@lukashuebner
Copy link
Member

  • I'd prefer if the user simply passed kamping::inplace and not recv_buf(kamping::inplace).
  • Do we explicitly check if the same two buffers are passed to send_buf and recv_buf (either to automatically do the operation inplace or to emit an error message); not zero-overhead

I ditched the idea of providing inplace explicitely (see #626). The same semantics can be achieved with a lot less confusion by passing send_recv_buf and no send_buf and recv_buf. When a send_recv_buf is passed, we detect that inplace should be used, no implicit comparison of send and recv buffers. If users pass them explicitely instead of using a send_recv_buf, they might have a reason to do so ...

Sounds good. It's harder to use the API wrong this way. :-)

@niklas-uhl niklas-uhl changed the title Inplace operations Symmetric inplace operations Mar 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants