itaps-parallel [Fwd: problems with ITAPS interfaces]

[Carl Ollivier-Gooch]

Lori,

Can you forward this to Martin?  I, at least, would like more info from 
him before I start digging.  See below.

Lori A. Diachin wrote:
> Here are the problems that Martin encountered when working with the 
> ITAPS interfaces.  We should be sure that all such issues are cleaned up 
> and resolved before the tutorial...
> 
> Thanks,
> Lori
> 
> ------------------------------------------------------------------------
> 
> Subject:
> problems with ITAPS interfaces
> From:
> Martin Isenburg <isenburg at llnl.gov>
> Date:
> Tue, 03 Jun 2008 00:20:11 -0700
> To:
> "Lori A. Diachin" <diachin2 at llnl.gov>
> 
> To:
> "Lori A. Diachin" <diachin2 at llnl.gov>
> 
> 
> 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.

My memory is that the spec says that an array of zero size (*_allocated 
= 0) gets allocated.  I think the spec is silent about what effect, if 
any, a NULL pointer has; in any event, if the pointer is NULL, then the 
allocated size better be reported as zero!

The spec also says that an array that's allocated too small is an error.

> 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)

I agree that these are all errors.

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

Also agreed.

> 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.

I'm not sure I understand this, in the sense that I'm not sure I see how 
an implementation that correctly does both setIntData() and 
setIntArrData() could do so incompatibly.

In all of these cases, it would be helpful to know which implementations 
exhibit which problems.  (I suspect we'll all just assume that our 
implementations are correct unless told otherwise. :-))  Unit tests for 
behavior with bad array size didn't get written, because at one point 
this was going to be handled for SIDL via Tammy Dalhgren's auto-testing 
of pre- and post-conditions.  While I'm sure Tammy's stuff works great, 
we've moved away from SIDL, so I'll put those unit tests on my to-do 
list.  (Actually, I'll try to figure out a way to rig up a simple code 
generator that will create those tests, if possible, because otherwise 
it'll be an immense pain, and I have no summer student to do that 
particular bit of scut work.... :-()

> 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.

What symptoms does this show (i.e., where and how does it crash)?  The 
swapping code may well assume an ordering for the regions adjacent to a 
face, in which case the swapping code is wrong.  But if an 
implementation is returning face->vertex or region->vertex info out of 
canonical ordering, then that's an implementation problem.  (Personally, 
I suspect it's the former, and that there are other problems preventing 
the swapping code from running with MOAB and FMDB...)

Carl

-- 
------------------------------------------------------------------------
Dr. Carl Ollivier-Gooch, P.Eng.                   Voice: +1-604-822-1854
Associate Professor                                 Fax: +1-604-822-2403
Department of Mechanical Engineering             email: cfog at mech.ubc.ca
University of British Columbia              http://www.mech.ubc.ca/~cfog
Vancouver, BC  V6T 1Z4                  http://tetra.mech.ubc.ca/ANSLab/
------------------------------------------------------------------------