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