[MOAB-announce] Fwd: [MOAB-dev] r4877 - in MOAB/trunk: itaps/imesh src src/io src/parallel test test/h5file test/obb tools/dagmc

[Tim Tautges]

FYI to those using MOAB, this is changed in the trunk version of MOAB and will be in the next release.

- tim

-------- Original Message --------
Subject: [MOAB-dev] r4877 - in MOAB/trunk: itaps/imesh src src/io	src/parallel test test/h5file test/obb tools/dagmc
Date: Wed, 25 May 2011 11:26:49 -0500 (CDT)
From: kraftche at cae.wisc.edu
Reply-To: moab-dev at mcs.anl.gov
To: moab-dev at mcs.anl.gov

Author: kraftche
Date: 2011-05-25 11:26:49 -0500 (Wed, 25 May 2011)
New Revision: 4877

Modified:
    MOAB/trunk/itaps/imesh/iMesh_MOAB.cpp
    MOAB/trunk/src/AdaptiveKDTree.cpp
    MOAB/trunk/src/Core.cpp
    MOAB/trunk/src/io/ReadCCMIO.cpp
    MOAB/trunk/src/io/ReadCGM.cpp
    MOAB/trunk/src/io/ReadHDF5.cpp
    MOAB/trunk/src/io/Tqdcfr.cpp
    MOAB/trunk/src/io/WriteCCMIO.cpp
    MOAB/trunk/src/io/WriteHDF5.cpp
    MOAB/trunk/src/io/WriteNCDF.cpp
    MOAB/trunk/src/io/WriteSLAC.cpp
    MOAB/trunk/src/io/WriteTemplate.cpp
    MOAB/trunk/src/parallel/ParallelComm.cpp
    MOAB/trunk/test/TagTest.cpp
    MOAB/trunk/test/h5file/h5file_test.cpp
    MOAB/trunk/test/h5file/h5portable.cpp
    MOAB/trunk/test/h5file/h5varlen.cpp
    MOAB/trunk/test/obb/obb_test.cpp
    MOAB/trunk/test/obb/obb_time.cpp
    MOAB/trunk/test/obb/obb_tree_tool.cpp
    MOAB/trunk/tools/dagmc/DagMC.cpp
Log:
o Change all MOAB code to pass zero-valued handle rather than NULL entity
   handle array to get/set tag data on the mesh.
o Print warning when NULL handle array is passed to tag get/set functions.
o Add #ifdef'd out logic for future transition to disallowing NULL handle
   array in tag get/set functions

Detailed description:

MOAB originally interpreted a NULL handle array and a zero handle count
to mean that tag get/set functions should operate on the global/mesh value
of the tag.  For example:
   int meshval;
   moab->tag_get_data( tag, 0, 0, &meshval );

For compatibility with ITAPS I changed MOAB to also accept a zero-valued
handle to get the mesh value.  For example:
   int meshval;
   EntityHandle root = 0;
   moab->tag_get_data( tag, &root, 1, &meshval );

The latter is better than the former for several reasons:
1) It is consistent with other API functions that use a zero-valued
    handle to indicate the mesh (e.g. get_entities_by_type)
2) It allows retrieval of the mesh value with other values in a single
    call.
3) It is more efficient because it eliminates a special case in the
    primary code path for getting/setting tag data.  (The special case
    is moved into an error handling path for invalid handles for dense
    tags and is eliminated entirely for sparse and bit tags.)
4) It avoids accidental segfaults if the caller asks for no entities.
    For example, the code below will currently segfault if the entity
    set happens to be empty:
      std::vector<EntityHandle> handles;
      moab->get_entities_by_handle( set, handles );
      std::vector<int> data(handles.size() * ints_per_tag);
      moab->tag_get_data( tag, &handles[0], handles.size(), &data[0] );
    because MOAB tries to write the global/mesh value into the unallocated
    'data' vector.

So we want to transition to accepting only the latter method of operating
on the global/mesh tag value.  This checkin is the first of a three phase
process to implement this transition:
1) Print warning when tag functions are passed a NULL array, but otherwise
    continue to return the global/mesh value of the tag.
2) Print error and abort when tag functions are passed a NULL array.  This
    is an intermediate stage intended to trap the last few uses of the old
    method.
3) Remove check for NULL handle array.  This final state will allow the
    example in 4) above to correctly return nothing.

This checkin contains all three of the above phases, where the behavior
is selected via #defines near the beginning of Core.cpp.



Modified: MOAB/trunk/itaps/imesh/iMesh_MOAB.cpp
===================================================================
--- MOAB/trunk/itaps/imesh/iMesh_MOAB.cpp	2011-05-25 05:40:11 UTC (rev 4876)
+++ MOAB/trunk/itaps/imesh/iMesh_MOAB.cpp	2011-05-25 16:26:49 UTC (rev 4877)
@@ -1637,7 +1637,8 @@
        }
          // check if tag value is set on mesh
        const void* data_ptr;
-      result = MOABI->tag_get_data( this_tag, 0, 0, &data_ptr );
+      EntityHandle root = 0;
+      result = MOABI->tag_get_data( this_tag, &root, 1, &data_ptr );
        if (MB_SUCCESS == result)
          ERROR(iBase_TAG_IN_USE, "iMesh_destroyTag: forced=false and mesh"
                " is still assigned this tag.");
@@ -1775,15 +1776,8 @@
    {
      ErrorCode result;

-    if (entity_set_handle == 0)
-        // set the tag data on this entity set
-      result = MOABI->tag_set_data(TAG_HANDLE(tag_handle),
-                                 NULL, 0, tag_value);
-    else {
-      EntityHandle set = ENTITY_HANDLE(entity_set_handle);
-      result = MOABI->tag_set_data(TAG_HANDLE(tag_handle), &set, 1, tag_value);
-    }
-
+    EntityHandle set = ENTITY_HANDLE(entity_set_handle);
+    result = MOABI->tag_set_data(TAG_HANDLE(tag_handle), &set, 1, tag_value);
      CHKERR(result, "iMesh_setEntSetData: error");

      RETURN(iBase_SUCCESS);
@@ -1851,12 +1845,8 @@

      ALLOC_CHECK_TAG_ARRAY(tag_value, tag_size);

-    if (eh == 0)
-      result = MOABI->tag_get_data(tag, NULL, 0,
-                                   *static_cast<void**>(tag_value));
-    else
-      result = MOABI->tag_get_data(tag, &eh, 1,
-                                   *static_cast<void**>(tag_value));
+    result = MOABI->tag_get_data(tag, &eh, 1,
+                                 *static_cast<void**>(tag_value));

      CHKERR(result, "iMesh_getEntSetData didn't succeed.");
      KEEP_ARRAY(tag_value);
@@ -1937,11 +1927,6 @@
                             /*in*/ iBase_EntitySetHandle entity_set_handle,
                             /*in*/ const iBase_TagHandle tag_handle, int *err)
    {