On Jul 16, 2012, at 9:51 PM, Jed Brown wrote:
> On Mon, Jul 16, 2012 at 6:53 PM, Barry Smith <bsmith at mcs.anl.gov> wrote:
> Yes, that is absolutely a hack and does not belong there. But you are
> totally miss understanding what I am saying: that hack is NEW. For 15 years
> PETSc did NOT need a hack to work with MPIUNI (which has a single
> communicator), thus I conclude that MPUNI is fine and something is wrong with
> the PETSc thread comm stuff if it requires that hack. That is, why the heck
> does petscthreadcomm depend on MPI_COMM_SELF != MPI_COMM_WORLD while for 20
> years NOTHING ELSE IN PETSc (which is a dang lot more complicated than
> petscthreadcomm) does not depend on MPI_COMM_SELF != MPI_COMM_WORLD???
>
> As far as I know, the other calls to MPI_Attr_put occur the first time the
> comm is used with an object rather than being pre-initialized. In this case,
> PETSC_COMM_SELF and PETSC_COMM_WORLD are pre-endowed with the threadcomm.
> Should the threadcomm be placed in some global variable and obtained (and
> referenced) the first time it is used with a given communicator? If we store
> the canonical one on a comm, then it must be on COMM_WORLD and COMM_SELF.
/* PETSC_COMM_SELF = PETSC_COMM_WORLD for MPIUNI */
#if !defined(PETSC_HAVE_MPIUNI)
ierr = PetscCommDuplicate(PETSC_COMM_WORLD,&icomm,PETSC_NULL);CHKERRQ(ierr);
ierr = MPI_Attr_put(icomm,Petsc_ThreadComm_keyval,(void*)tcomm);CHKERRQ(ierr);
tcomm->refct++; /* Share the threadcomm with PETSC_COMM_SELF */
#endif
ierr = PetscCommDuplicate(PETSC_COMM_SELF,&icomm,PETSC_NULL);CHKERRQ(ierr);
ierr = MPI_Attr_put(icomm,Petsc_ThreadComm_keyval,(void*)tcomm);CHKERRQ(ierr);
I would not do it this way. Instead I would write a general routine that
attached a threadcomm to a MPI_Comm; this routine would get the
threadcomm_keyval and if it did NOT find it then would be put the attribute,
otherwise it would know one was already there. Say it is called
PetscThreadCommAttach(MPI_Comm, threadcomm); then in this routine you would
just write
PetscThreadCommAttach(PETSC_COMM_WORLD, tcomm);
PetscThreadCommAttach(PETSC_COMM_SELF,tcomm); /* won't attr it again
for MPIUni because it is already there */
>
>
> In other words fix petscthreadcomm model; don't mess with a perfectly good
> mpiuni.
>
> MPI has them as separate communicators. MPIUNI breaks that model for, as far
> as I can tell, no good reason.
1 Philosophical reason) The natural limitation of MPI to one process is to
have a single communicator; having two different ones that are "identical" is
unnatural
2 Practical reason) MPIUNI would be more complicated to handle two sets of
attributes etc. Plus if you do this you really need to support MPI_Comm_dup as
creating yet another "different" communicator making the code even more
complicated since now you need to manage duping and freeing comms, becomes more
like a real MPI implement. Not worth it.
3 It finds bugs in the design of new components like PetscThreadComm that make
assumptions and puts an attribute without first checking if it is already there
:-).
Barry
