Topic: pmi-simple - update to upstream (MPICH) state

Sandia-OpenSHMEM / SOS

Sandia OpenSHMEM is an implementation of the OpenSHMEM specification over multiple Networking APIs, including Portals 4, the Open Fabric Interface (OFI), and UCX. Please click on the Wiki tab for help with building and using SOS.

Other

63 stars 53 forks source link

Topic: pmi-simple - update to upstream (MPICH) state #1045

Closed davidozog closed 5 months ago

davidozog commented 2 years ago

This requires some slight modifications for SOS:

include "config.h" instead of "mpichconf.h"
define MPI_MAX_PORT_NAMEmanually (w/out including "mpi.h")
carry new pmi-simple requirements from MPICH (mpir_mem.h/mpl_sockaddr.h)
ignore strncasecmp/strnicmp requirement (not used in pmi-simple)

I'm not sure we want to make this change, but it does eliminate several warnings we see in our current version of pmi-simple. If we include it, we may want to defer this for a later SOS release (e.g., v1.5.2).

davidozog commented 2 years ago

Closing for now, tracking with issue #1046, and adding to the v1.5.x Github Milestone

davidozog commented 6 months ago

This version of pmi-simple best aligns with pmodels/MPICH v3.2.1. Beyond that there were more significant changes that may not be practical to incorporate into SOS at the moment.

However, this does eliminate all the compiler warnings from #1046 and passes CI, so I think now is possibly a good time to try it out for the v1.5.3 release.

bcmIntc commented 5 months ago

Question regarding "TODOs" left in the code. Should these be removed/resolved or remove/CR, or left as-is before upstream?

davidozog commented 5 months ago

LGTM. The tests also do not show any more warnings. One question: the 3 new files - are they all verbatim to upstream or do we have some changes?

@wrrobin - the tests are verbatim except for the 4 bullet items listed in the PR description, which are minimal but needed to be compatible with SOS.

Question regarding "TODOs" left in the code. Should these be removed/resolved or remove/CR, or left as-is before upstream?

@bcmIntc - since we keeping these changes "upstream" with respect to MPICH, we should keep as much as possible identical so we don't confuse ourselves or create unnecessary work if/when we update. However, if we fork off then we can remove/address these, but it's something we have avoided in the past for simplicity.