itaps-parallel [Fwd: problems with ITAPS interfaces]

[Tim Tautges]

Hi gang,
   Sorry I didn't get to this until now, here are my responses.  Martin, 
sorry for the double copy if you're already on the itaps-parallel list...


Lori A. Diachin wrote:

> 
> hello lori,
> 
> here a summary of the ITAPS problems i came across.
> 
> Overall the biggest issue is the exact "expectations" on the value of 
> passed in arguments that are of InOut or Out type. Let us look at the 
> example of iMesh_getIntArrData().
> 
> void iMesh_getIntArrData(iMesh_Instance instance,
>                          /*in*/ const iBase_EntityHandle* entity_handles1,
>                          /*in*/ const int num_entities,
>                          /*in*/ const iBase_TagHandle tag_handle1,
>                          /*inout*/ int** tag_values,
>                          /*inout*/ int* tag_values_allocated,
>                          /*out*/ int* tag_values_size,
>                          /*out*/ int *err)
> 
> The biggest issue is the correct handling of memory allocation. What 
> exactly is expected? If  one (or both?) of tag_values_allocated and 
> tag_values is zero the function is to allocate the required memory using 
> malloc(), if tag_values_allocated  is less than what is needed an error 
> is to be returned.
> 
> I saw implementations errors that
> 
> * assumed enough memory was allocated
> * that reset a passed in tag_values_allocated to be identical to a 
> smaller tag_values_size
> * always allocated new memory (even if sufficient memory was passed in)
> 

The proper behavior, IMO, is:

- should *not* assume enough memory is allocated
- if allocated but too small, should return with 
iBase_MEMORY_ALLOCATION_FAILED or  iBase_INVALID_ARGUMENT (Carl, you 
probably know which it's supposed to be - MOAB returns the former)

One thing I'm not sure we have agreement on is whether BOTH the pointer 
and the allocated size have to be zero, or EITHER can be zero, to 
indicate memory should be allocated.  I use EITHER in MOAB.  In any 
case, if you're passing in BOTH and getting the errant behavior above, 
I'd call that a compliance failure.

> Also ... functions should *not* expect err that is passed in to be 
> properly initialized.
> 

Agreed.  These are marked as out-type parameters.

> Another issue is the interchangeable use of setIntData(), getIntData(), 
> setIntArrData()  getIntArrData().  While setIntData() and getIntData() 
> set or get a single integer tag value ... they should be made compatible 
> with setIntArrData() and getIntArrData(). these functions can either set 
> or get (a) multiple integers for one tag, (b) set or get single integers 
> for multiple tags, and (c) set or get multiple integers for multiple 
> tags. for case (b) the implementation must assure that those single 
> integers per tag may interchangeably be set and gotten with setIntData() 
> and getIntData(). the deciding instance for how many integers there are 
> per tag should be what is defined in the function that creates the tags.

This is an interface issue.  Right now, the functions get the values for 
a given tag for either one or multiple entities.  Both get a tag size 
corresponding to that set when the tag was created.  So, a 
multiple-valued tag will return multiple values from getIntArrData, and 
calling getIntData for that tag should result in an error.  Is that the 
problem, that we don't describe that clearly enough in the docs?

- tim

> 
> Finally i noticed that Carls's swapping code seems to assume a 
> particular orientation when querying the vertices of faces and/or 
> tetrahedra with iMesh_getEntArrAdj() in the ITAPS_Swap::Swap3D::swap 
> function that makes it impossible to run his code with a different 
> (other than GRUMP) iMesh implementation.
> 
> that's all i can thing of for now.
> 
> martin
> 
> 

-- 
================================================================
"You will keep in perfect peace him whose mind is
   steadfast, because he trusts in you."               Isaiah 26:3

              Tim Tautges            Argonne National Laboratory
          (tautges at mcs.anl.gov)      (telecommuting from UW-Madison)
          phone: (608) 263-8485      1500 Engineering Dr.
            fax: (608) 263-4499      Madison, WI 53706