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

Enhancements to mpi::datatype #49

Merged
merged 5 commits into from
Nov 4, 2018

Conversation

AndrewGaspar
Copy link
Contributor

  • Adds new types: UncommittedUserDatatype + UncommittedDatatypeRef
    • same constructors as UserDatatype and DatatypeRef
    • UncommittedUserDatatype::commit consumes the old value to produce a UserDatatype
    • UserDatatype::structured and UncommittedUserDatatype::structured constructors takes a slice that can be passed directly to underlying MPI_Type_create_struct call without copy
    • new as_ref() functions for UserDatatype and UncommittedUserDatatype
  • Adds UncommittedDatatype trait
    • Implemented for all Datatype types
    • Supports capability for duplicating the datatype
  • From implemented for Datatype types to allow downcasting of heterogeneous types when being mixed into a single contiguous array. Utility demonstrated by bool::equivalent_datatype().into() in complex_datatype.rs.
  • New generic trait: FromRaw
    • Provides fixed way for constructing an mpi::Foo from a MPI_Foo
    • Implemented for all the Datatype types
  • New generic trait: MatchesRaw
    • Marker trait that indicates it's representationally safe to treat a slice of Foo as a slice of Foo::Raw
    • Implemented for all of the Datatype types
  • Adds a complex_datatype.rs test that constructs a UserDatatype from multiple UncommittedUserDatatype values

- Adds dup() for all implementations of Datatype trait
- Adds as_ref() to get a DatatypeRef from any Datatype
- Adds example of dup()
- Adds new types: UncommittedUserDatatype + UncommittedDatatypeRef
  - same constructors as UserDatatype and DatatypeRef
  - UncommittedUserDatatype::commit consumes the old value to produce a
    UserDatatype
  - structured constructor takes a slice that can be passed directly to
    underlying MPI_Type_create_struct call without copy
- Adds UncommittedDatatype trait
  - Implemented for UncommittedUserDatatype, UncommittedDatatypeRef,
    UserDatatype, and DatatypeRef
  - Supports capability for duplicating the datatype
- New generic trait: FromRaw
  - Provides fixed way for constructing an mpi::Foo from a MPI_Foo
  - Implemented for all the Datatype types
- New generic trait: MatchesRaw
  - Marker trait that indicates it's representationally safe to treat
    a slice of Foo as a slice of Foo::Raw
  - Implemented for all of the Datatype types
- Adds a complex_datatype test that constructs a UserDatatype from
  multiple UncommittedUserDatatype values
-
@bsteinb
Copy link
Collaborator

bsteinb commented Oct 14, 2018

Cool! Could I ask you for two additions?

First, I think it would be good to at least mention the ...Ref and Uncommitted... types in the module level documentation to be aware of them and see how they fit into the family of types that keeps growing.

Second, I was going to ask you to add an assertion to UserDatatype's impl of FromRaw to make sure the raw value actually references a committed data type, but then I noticed there does not seem to be a way to find out whether a type has been committed. So, okay... It does say in the standard that calling MPI_Type_commit on a committed type is a no-op, so maybe we should just unconditionnaly do that to be safe?

@AndrewGaspar
Copy link
Contributor Author

I'm not 100% sure the suggestion to promote the datatype to committed is a good idea. It seems like the rest of the FromRaw implementations are very straightforward and the requirement on the caller is clear: the caller is in charge of enforcing the invariants of the type that are constructing. By, in a way, silently promoting the datatype to "committed", I feel like we could potentially be masking a user bug of not previously committing the datatype.

On the other hand, I think that's the only potential negative side effect. I'm just uncomfortable with it because it seems side-effecty in a way that may be unexpected. Thoughts?

@bsteinb
Copy link
Collaborator

bsteinb commented Nov 4, 2018

All right, since from_raw is marked unsafe, this is allowed to require some care on the side of API consumers.

@bsteinb bsteinb merged commit 807b5ad into rsmpi:master Nov 4, 2018
@AndrewGaspar AndrewGaspar deleted the rsmpi-datatype-builder branch December 2, 2018 22:05
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.

None yet

2 participants