kokkos / kokkos-comm

Experimental MPI Wrapper for Kokkos
https://kokkos.org/kokkos-comm/
Other
14 stars 9 forks source link

Use MPI_BYTE for unknown types #81

Open aprokop opened 3 months ago

aprokop commented 3 months ago

Fix #79.

masterleinad commented 3 months ago

To actually fix #79 you would also need to make sure that you scale the number of bytes to send and receive accordingly, though.

aprokop commented 3 months ago

To actually fix #79 you would also need to make sure that you scale the number of bytes to send and receive accordingly, though.

I feel sheepish for forgetting this.

cwpearson commented 3 months ago

What if the fallback case was just to construct, register, and cache a contiguous MPI derived datatype that is sizeof T in a static variable.

aprokop commented 3 months ago

What if the fallback case was just to construct, register, and cache a contiguous MPI derived datatype that is sizeof T in a static variable.

That may work. I'm not familiar with it. Could you post a snippet of code to demonstrate what you are thinking?

cwpearson commented 3 months ago

Untested:

else /*constexpr*/ {
  static MPI_Datatype cached = MPI_DATATYPE_NULL;
  if (MPI_DATATYPE_NULL == cached) {
    MPI_Type_contiguous(sizeof(T), MPI_BYTE, &cached);
    MPI_Type_commit(&cached);
  }
  return cached;
}

each call to mpi_type with a new T that reaches the fallback with a different T will have a different cached variable. Each new T will be created and committed once, lazily, the first time it is seen. Otherwise it will be a quick (?) runtime check.

aprokop commented 3 months ago

each call to mpi_type with a new T that reaches the fallback with a different T will have a different cached variable

Oh, that's a cool idea! This sounds like it could work well. Maybe create a templated function to create mpi types, so that users could specialize it on their own. With the default being the two MPI calls you posted.

cwpearson commented 3 months ago

Yeah it would be great if this could be exposed to the user to override it. I think we'd have to refactor this to be a SFINAE-style implementation rather than if constexpr then?

aprokop commented 3 months ago

Yeah it would be great if this could be exposed to the user to override it. I think we'd have to refactor this to be a SFINAE-style implementation rather than if constexpr then?

Yeah, probably just a templated function with the default type creation and specializations.

devreal commented 3 months ago

Who frees the temporary type? It should probably be put into a list of types that get destroyed at the end...

aprokop commented 3 months ago

Can make the static variable a holder class that frees on destruction. Nevermind, it would be called too late, after MPI was finalized.

cwpearson commented 3 months ago

Are we actually required to call MPI_Type_free?

aprokop commented 3 months ago

Are we actually required to call MPI_Type_free?

Some (like this presentation, slide 4) state that "You are not required to use it or deallocate it, but it is recommended (there may be a limit)".

I didn't see anything in the standard saying that it is required.

devreal commented 3 months ago

It's not required. Datatypes are a funny thing in MPI because they can persist between Sessions (mostly for historically reasons, because they are not bound to any Session). Having said that, KokkosComm should strive to exit cleanly with all resources freed. This makes it easier to use leak detection tools and is just good practice.

cwpearson commented 3 months ago

This ties in to the init / finalize discussions I think. We have to have the opportunity to do something before / when MPI_Finalize gets called.

devreal commented 3 months ago

We could have a function that returns a std:pair<MPI_Datatype, std::size_t>. For basic types .second is 1, for derived types it's sizeof(T) and .first is MPI_BYTE. It's all constexpr and with big count in MPI we don't need to worry about integer overflows.

On Mon, Jun 24, 2024, 11:49 Carl Pearson @.***> wrote:

This ties in to the init / finalize discussions I think. We have to have the opportunity to do something before / when MPI_Finalize gets called.

— Reply to this email directly, view it on GitHub https://github.com/kokkos/kokkos-comm/pull/81#issuecomment-2186888445, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACTXKJSUURADZYJURVUZMWTZJA5YRAVCNFSM6AAAAABJE6KTX6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCOBWHA4DQNBUGU . You are receiving this because you commented.Message ID: @.***>