itaps-parallel technical issues in iMeshP.h

Devine, Karen D kddevin at sandia.gov
Fri Oct 3 12:47:56 CDT 2008


Jason:  Thanks for the good comments; please see my replies below.
All:  I just svn committed to the RPI repository updates based on Ting's and
Jason's recent emails.

Karen


On 10/2/08 3:46 PM, "Jason Kraftcheck" <kraftche at cae.wisc.edu> wrote:

> iMeshP_createPartitionAll
>   - Why MPI_Comm*?  It is an "in" argument.  Pass by value would
>     be better, but if passed by pointer is desired for some reason
>     it should be const.

I'm just used to passing by pointer things that may be structures such as
MPI_Comm.  To be more consistent with MPI, we should pass it by value.  I'll
make the change.

> iMeshP_destroyPartitionAll
>   - Why is 'partition_handle' inout?  What is this function returning
>     a handle to?  Some other partition?

The partition handle should be set to an invalid value when we destroy the
partition.

>   - Why the "All" in the name?  It isn't destroying all partitions,
>     just the passed one.  Presumably in the previous function 'All'
>     means the 'Mesh', but unless where destroying the Mesh in this function
>     it doesn't make sense from a consistency POV either.

We used the convention "All" to indicate that all processes must call the
function synchronously.  In this case, all processes in a partition must
agree to destroy the partition at the same time.  It would be bad for some
processes to think a partition exists while other processes have destroyed
it.

>
> iMeshP_getPartFromPartHandle
> iMeshP_getPartFromPartHandlesArr
> iMeshP_getPartHandleFromPart
> iMeshP_getPartHandlesFromPartsArr
>   - Should these be iMeshP_getPartIdFromPartHandle(sArr)
>                                   ^^
>        and iMeshP_getPartHandleFromPartId(sArr) ?
>                                        ^^

Fine with me; they are probably clearer that way.


> void iMeshP_iPushTagsEnt
>   - If 'entities' is an input, it should be const.

Shall do.  Are all input values supposed to be const?  Or just pointers?


> iMeshP_ghostEntInfo
>   - 'num_ghost_rules" is not documented with other parameters
>   - 'num_ghost_rules" type is inconsistent with 'out' designator

Good catches.





More information about the itaps-parallel mailing list