Subject: Re: one red fish? -- bad assumption and a "fix"

From: [email-masked] (Tom Henderson)
Date: Wed, 21 Jul 93 10:30:23 MDT

Hi all,

Very early this morning, it occurred to me that one of the assumptions I made 
in the "sample implementation" of MPI_COMM_MAKE() causes problems with local 
group creation operations, or (if group creation is restricted to be 
collective) with Jim's "group fusion" (MPI_COMM_JOIN()).  The problem comes 
from the following part of the sample implementation:  

> The calling process then sends a message to process 0 in sync_group that 
> contains information describing comm_group.  (If the thread is in process 0, 
> it receives these messages.)  These messages are sent using special context 
> MPI_COMM_MAKE_CONTEXT.  This special context is not accessible to the user.  
> To disambiguate overlapping process groups that have the same "process 0", 
> sync_group is used as message tag.  (Note that I am assuming "INTEGER" handles
> here.  If "group" is a struct then we have to use an implementation-dependent 
> "group ID" field in this struct.  I use this scheme in the example below.)  
> Use of this tag ensures that process 0 will not attempt to receive a message 
> on context MPI_COMM_MAKE_CONTEXT from a process that is not in sync_group 
> until the current call to MPI_COMM_MAKE() completes.  

The assumption here is that groups comm_group and sync_group can each be 
identified by an integer that is unique within sync_group.  If local 
operations have been used to create these groups, then this cannot be 
guaranteed.  If only collective operations have been used to create these 
groups (as in the "basic" level of the "multi-level" proposal) then we're OK 
UNLESS comm_group has been formed by "joining" two groups (Jim's "group 
fusion").  

Here's how group fusion might screw things up even without local operations.  
Start with one group.  Form two groups A and B using MPI_COLL_SUBGROUP().  
Group A forms a "permuted" group A' using 
MPI_COLL_GROUP_PERMUTE(A, new_ranks, A').  Group B forms a "permuted" group B' 
using MPI_COLL_GROUP_PERMUTE(B, new_ranks, B').  Since group A is unaware of 
group B' and group B is unaware of group A', it is possible that the integer 
handle (or system-dependent "group ID") may be the same for groups A' and B'.  
Suppose processes in both groups have assigned the local permuted group to a 
variable named permutedGroup.  If groups A and B then "join" into group A_B, 
what happens when the processes in group A_B call 
MPI_COMM_MAKE(A_B, permutedGroup, newComm, &status)?  BAD THINGS most likely.  

How can this be fixed?  

1)  Send the "flattened" group comm_group in the implementation.  This is 
    probably "slow", but it works, and MPI_COMM_MAKE() is already pretty 
    "slow".  I show this "fix" below...  

2)  Disallow the case of sync_group != comm_group.  This may be unacceptable.  

3)  Make MPI_COMM_JOIN() so sophisticated that it can resolve inconsistent 
    group naming (and lots of other stuff I'm sure) during the "join" 
    operation.  This is so complicated I can't imagine how it would be done.  

4)  Use a "server" or some other scheme to maintain globally unique group 
    identifiers.  Definitely a bad idea...  

This still leaves the issue of local group operations being used to create 
sync_group unresolved.  As I've said before, I'm in favor of restricting these 
local operations to an "expert" level (or eliminating them altogether).  As 
long as sync_group is created by a collective operation, it's "group ID" can 
be unique for all member processes.  

I realize that all of this may be obvious to many of you.  Please be patient 
as I catch up...  :-)

Tom Henderson

----------------- "Implementation" of MPI_COMM_MAKE() -----------------------
-----------------  with "flatten/unflatten" fix.      -----------------------

/* New/changed code is indicated with "$$$" */

/* "Implementation-dependent" part of "group" struct typedef. */
typedef struct group
    {
    ...
    int implementationGroupID;
    ...
    } group;

/* "Global" declaration (visible to all threads in a process). */
pthread_mutex_t mpiCommMakeMutex;
...

/*****************************************************************************
** function MPI_COMM_MAKE()
**
** Note that I am "assuming" an error status passed as an OUT argument...  
** Error handling is oversimplified.  I use only point-to-point communication 
** routines.  This version has a "fix".  Group comm_group is now sent 
** "flattened" instead of by "group ID" in the "request" msg.  
**
*****************************************************************************/
void MPI_COMM_MAKE(group sync_group, group comm_group, comm comm_new, 
                   int *status)
    {
    pthread_mutex_t mpiCommMakeMutex;                  /* "shared" variable */
/* $$$ */
    int i, j, myRank, syncGroupSize, syncGroupID, flatGroupSize, flatMsgSize;
    int newContext, ackMsg, errFlag, dummy1, dummy2;
    receiveHandle rcvStatus;
/* $$$ */
    char flatCommGroup[MAX_FLAT_GROUP_SIZE], flatMsg[MAX_FLAT_GROUP_SIZE];

    /* Lock "shared" variable (blocks until lock succeeds). */
    pthread_mutex_lock(&mpiCommMakeMutex);

/* $$$ */
    /* Get implementation-dependent group IDs for sync_group. */
    syncGroupID = sync_group.implementationGroupID;

/* $$$ */
    /* Get flattened commGroup. */
    MPI_GROUP_FLATTEN(comm_group, MAX_FLAT_GROUP_SIZE, flatCommGroup, 
                      &flatGroupSize);

    /* Code for process 0 in sync_group and other processes in sync_group. */
    MPI_GROUP_RANK(sync_group, &myRank);
    if (myRank == 0)
        {
        /* Receive "request" message from each process in sync_group. */
/* $$$ */
        /* This message should contain "flattened" comm_group. */
        MPI_GROUP_SIZE(sync_group, &syncGroupSize);
        errFlag = 0;
        for (i=1; i<syncGroupSize; i++)
            {
/* $$$ */
            MPI_RECVC(flatMsg, flatGroupSize, MPI_BYTE, MPI_ANY_SOURCE, 
                      syncGroupID, MPI_COMM_MAKE_CONTEXT, rcvStatus);
            /* Set error flag if wrong comm_group sent by a process. */
            MPI_RETURN_STATUS(rcvStatus, &flatMsgSize, &dummy1, &dummy2);
            if (flatMsgSize != flatGroupSize)
                errFlag = 1;
            else
                {
                /* Compare flattened groups. */
                /* This might be done better by unflattening and using an */
                /* internal implementation-dependent group comparison */
                /* routine.  This would be required if "group ID" is */
                /* imbedded in the flattened information. */
                for (j=0; j<flatGroupSize; j++)
                    if (flatMsg[j] != flatCommGroup[j])
                        errFlag = 1;
                }
            }
        /* Send "acknowledge" message or "error" message. */
        if (errFlag == 0)
            {
            /* "Acknowledge" message (continue, everything's OK...). */
            ackMsg = MPI_OK;
            for (i=1; i<syncGroupSize; i++)
                {
                MPI_SENDC(&ackMsg, 1, MPI_INT, i, syncGroupID, 
                          MPI_COMM_MAKE_CONTEXT);
                }
            }
        else
            {
            /* Handle error (wrong comm_group sent by a process). */
            ackMsg = MPI_ERROR_BAD_COMM_GROUP;
            for (i=1; i<syncGroupSize; i++)
                {
                MPI_SENDC(&ackMsg, 1, MPI_INT, i, syncGroupID, 
                          MPI_COMM_MAKE_CONTEXT);
                }
            *status = MPI_ERROR_BAD_COMM_GROUP;
            return;
            }
        /* Get new context and send to other processes. */
        MPI_CONTEXTS_RESERVE(1, &newContext);
        for (i=1; i<syncGroupSize; i++)
            {
            MPI_SENDC(&newContext, 1, MPI_INT, i, syncGroupID, 
                      MPI_COMM_MAKE_CONTEXT);
            }
        }  /* end of myRank==0 if */
    else
        {
/* $$$ */
        /* Send "request" message containing "flattened" comm_group to */
        /* process 0 in sync_group. */
        MPI_SENDC(flatCommGroup, flatGroupSize, MPI_BYTE, 0, syncGroupID, 
                  MPI_COMM_MAKE_CONTEXT);
        /* Receive "acknowledge" message from process 0. */
        MPI_RECVC(&ackMsg, 1, MPI_INT, 0, syncGroupID, MPI_COMM_MAKE_CONTEXT, 
                  rcvStatus);
        /* Check for error... */
        if (ackMsg != MPI_OK)
            {
            *status = ackMsg;
            return;
            }
        /* Receive new context from process 0. */
        MPI_RECVC(&newContext, 1, MPI_INT, 0, syncGroupID, 
                  MPI_COMM_MAKE_CONTEXT, rcvStatus);
        }  /* end of myRank==0 else */

    /* All processes in sync_group bind newContext to comm_group to create */
    /* new communicator. */
/* $$$ */
    /* It would be a good idea to resolve any "group ID" mismatches in */
    /* comm_group before this call so comm_new is constructed in exactly */
    /* the same way at every process... */
    MPI_COMM_BIND(comm_group, newContext, comm_new);

    /* Release lock. */
    pthread_mutex_unlock(&mpiCommMakeMutex);

    /* Set error status to "OK" and return. */
    *status = MPI_OK;
    return;
    }  /* end of function MPI_COMM_MAKE() */