[MOAB-dev] MBInterface API changes for meshsets

[Brandon Smith]

Hi Everyone,

Is the MESHSET_SET vs. MESHSET_ORDERED analogous to MBRanges vs. std::vectors? Ranges (and MESHSET_SETS) are ordered by handle and unique. Vectors (and MESHSET_ORDEREDs) are not ordered by handle and preserve duplicates. 

I would get rid of the MESHSET_SET flag and only use MESHSET_ORDERED and MESHSET_TRACKING. If either of these are not specified, the default behavior would be unordered, not tracking. In my short MOAB career I've thought the MESHSET_SET flag seems unnecessary.

Brandon 


----- Original Message ----
From: Jason Kraftcheck <kraftche at cae.wisc.edu>
To: Tim Tautges <tautges at mcs.anl.gov>
Cc: Developer correspondence for MOAB <moab-dev at lists.mcs.anl.gov>
Sent: Tue, October 27, 2009 3:59:47 PM
Subject: [MOAB-dev] MBInterface API changes for meshsets

MOAB currently defines the following enum in MBTypes.h:

  /** Meshset options: properties for meshset creation.
   *  Values are bit flags that may be combined with a bitwise OR (|)
   */
  enum MBEntitySetProperty {
    MESHSET_TRACK_OWNER = 0x1, /**< create entity to meshset adjacencies */
    MESHSET_SET         = 0x2, /**< set contents are unique */
    MESHSET_ORDERED     = 0x4  /**< order of set contents is preserved */
  };

and has the following three functions in MBInterface that interact with this
enum:

  MBErrorCode create_meshset(const unsigned int options,
                             MBEntityHandle &ms_handle,
                             int start_id = 0) = 0;

  MBErrorCode get_meshset_options(const MBEntityHandle ms_handle,
                                  unsigned int& options) const = 0;

  MBErrorCode set_meshset_options(const MBEntityHandle ms_handle,
                                  const unsigned int options) = 0;

This API is somewhat confusing for two reasons:
  a) We pass a bitwise-OR of the MBEntitySetProperty values an int,
     but the latter two are mutually exclusive in the implementation.
  b) The get/set paradigm for the options is not very convenient (and
     potentially error prone) because one needs to get the previous
     options and flip bits to make sure that some property (e.g.
     tracking) isn't accidentally disabled.  For example, doing:
      set_meshset_options( my_set, MESHSET_SET )
     may will remove the MESHSET_TRACK_OWNER property, if it is set.

I think that the API would be easier to understand and less error prone if
we instead had:
  enum MBEntitySetStorage { MESHSET_SET, MESHSET_ORDERED };

  MBErrorCode create_meshset( MBEntitySetStorage type,
                              MBEntityHandle& ms_handle,
                              bool tracking = false,
                              int start_id = 0 ) = 0;

  MBErrorCode get_meshset_options( MBEntityHandle ms_handle,
                                   MBEntitySetStorage& storage,
                                   bool& is_tracking ) = 0;

  MBErrorCode set_meshset_tracking( MBEntityHandle ms_handle,
                                    bool tracking ) = 0;

  MBErrorCode set_meshset_storage( MBEntityHandle ms_handle,
                                   MBEntitySetStorage type ) = 0;

Any opinions?

- jason