iRel.h interface update [was iRel proposal: removing non-one-to-one relation functions]

[seol at scorec.rpi.edu]

>From what we discussed in the bootcamp, the way of removing the relation (use case: removing classification) is still
missing. Mark B. proposed to add iRel_removeXXXXRelation.

> * (RPI) Change iRel_newRel -> iRel_create (likewise for dtor -> destroy)
>   Not much sense in doing this until iMesh/iGeom do it.

If I am not mistaken, this issue has been ratified (conclusion: YES)

> * (RPI) Missing iRel_get(Ent|Set)SetArrRelation
>   I'm arguing against all functions like this anyway, so I left them out
>   until we decide on that to do there.

To the best of my remembrance, we agreed to add iRel_getEntSetArr and iRel_getSetSetArr in the bootcamp. And then you
suggested to remove all "non" one-to-one get/set functions as the following:

  iRel_set|getEntEntArrRelation
  iRel_set|getSetEntArrRelation
  iRel_set|getEntSetArrRelation
  iRel_set|getSetSetArrRelation

Not so sure about removing all one-to-many functions until the effective classificaiton support issue is completely
resolved. (See below)

> * (RPI) Missing switch_order from iRel_setEntEntRelation and friends
>   This argument isn't necessary to make these functions work. If people
>   really see a use for them, we can add them, but it wasn't obvious to
>   me that this was the way to go. This would need discussion, I think.

Among 24 iRel_set/get functions, the eight functions don't have switch_order argument to determine the "relation side".
Incompleteness could be interpreted in either of two: "relation side" was accidentally missed from the eight functions
or the necessity of "relation side" is not clear yet. If neither is the case, let's discuss.

In addition to the above, I propose some wordsmithing idea:
1. change "int *ierr" to "int *err" for consistency with other interface

2. (for all interfaces) having "int *err" at the end of arg list
    I checked iMesh and iRel. Most of iMesh/iRel functions do have "int *err" at the end of arg list, but some (not
many) don't. Do we have any conventions for the order of args?

3. We defined the enumeration type "RelationType". To represent "relation type", we use "integer" type instead of
"RelationType". Here I see two options:

   #1. change "int ent_or_set" to "RelationType type"
   #2. remove the enumeration type "RelationType" since it's no use

4. adding "EntArr" to "Relation Type" and removing iRel_getEntSetIter & iRel_getEntArrSetIterArr

   CURRENT Relation Type: Ent, Set, Both (Ent&Set)
   NEW: Ent, Set, Both (Ent&Set), EntArr

  rationale: The current spec of iRel requires "set" as the only method to represent "multiple data" such as
"entities". This doesn't effectively meet the "non" set-dominant implementation. for example, the relation to
represent the classification in FMDB and Simmetrix is one-to-(NON-SET)many relation, in other words, entity to
entities. In order to stay within Ent/Set/Both Relation Type rule and support the need for one-to-(NON-SET)many
relation, we put "iRel_getEntSetIter" and "iRel_getEntArrSetIterArr" WITHOUT matching set functions.

Even though it helps for iterating over (getting) one-to-(non-set)many relation without having "true set", it still
requires the implementation to build "fake set" to be acting as "set-dominant" for some functions such as
iRel_getEntSet, needing unnecessary data conversion from entities (implementation level) to set (iRel level) to meet
the iRel spec which is in favor of set-dominant implementation. But not all implentations use "set" as a way of
represent a group of data.

As we do have both EntitySet and EntArr in iMesh to support both entity array (FMDB, GRUMMP) and set-favored
implementation (MOAB), we suggest to add "EntArr" to Relation Type as a way of representing "one-to-(NON-SET)many"
relations. This avails non set-dominant impementation to support "iRel" effectively without interfering set-dominant
implementation and not against neither of "interchangeability" and "interoperability".

       iRel_createRelation (irel, igeom, iRel_ENT, iRel_IGEOM_IFACE,
                            imesh, iRel_ENTARR, iRel_IMESH_IFACE, &pair, &ierr);
       /* setting classification/reverse classification */
       iRel_setEntEntRelation
       iRel_setEntEntArrRelation
       iRel_setEntSetRelation // return error

       /* getting classification/reverse classification */
       iRel_getEntEntRelation
       iRel_getEntEntArrRelation
       iRel_getEntSetRelation // return error

With the relation created with Relation Type "EntArr", any set|getRelation functions with "Set" will get error.

By adding "EntArr" to Relation Type, the following functions will be removed.
  iRel_getEntSetIter
  iRel_getEntArrSetIterArr
  iRel_set|getSetEntArr
  iRel_setEntSetArr (no matching get function at the moment)
  iRel_setSetSetArr (no matching get function at the moment)

Seegyoung

> I've committed a new version of iRel.h taking into account the recent
> discussion (*not* including my non-one-to-one proposal). Be warned,
> there are a lot of changes by line, since I fixed the indentation on the
> comments to be consistent with iMesh.h and iGeom.h as well as documented
> in/out parameters. I'd upload those in separate revisions, but SVN
> doesn't play nicely with merging histories between repos, and it'd be a
> lot of work to do manually.
>
> Here's a rundown of what's changed (names in parens indicate who made
> the suggestion):
>
> * (Porter, J) Replace iRel_LAST_ERROR with iRel_getDescription and
>   iRel_getErrorType for consistency with iMesh/iGeom
> * (RPI) __iRel_LASSO_HPP__ is now _ITAPS_iRel
> * (RPI) Includes iBase.h instead of iGeom.h/iMesh.h
> * (RPI) Documented argument types (in, out, inout)
> * (Porter, J; RPI) Cleaned up comment formatting, fixed out-of-date or
>   inaccurate comments (remove is_set comments from infer functions)
> * (Porter, J) Uses of "Relation" to refer to a PairHandle are now "Pair"
>
> Stuff I didn't do (and why):
>
> * (Porter, J) Define iBase_EntityIterator in iBase.h
>   This would have effects on iMesh and iGeom, so I left it aside for
>   now.
> * (RPI) How to work with iBase_EntityIterator
>   Nothing to do currently. Right now you'd just use the iMesh or iGeom
>   functions as appropriate. This needs some discussion if it's going to
>   change, otherwise it could just be documented in iBase.h, where
>   iBase_EntityIterator belongs.
>
> I think that's everything.
>
> - Jim
>
>
> On Tue, 2010-10-26 at 13:50 -0400, Mark Shephard wrote:
>> As was stated previously, we are still waiting for Tim to provide the
>> up-dated iRel.h documentation to reflect the things discussed at the
>> bootcamp that are needed by implementations other than MOAB. These items
>> have been agreed to (twice now) but still not included in iRel.h. Until
>> those items are settled it does not make sense to consider other changes
>> that may influence the things we are waiting for.
>>
>> Tim - When with the updated iRel.h file be provided? If you can not get
>> to it, there are others that could likely edit the iRel.h to reflect the
>> things that were agreed to at the boot camp.
>>
>> Mark
>>
>>
>> On 10/26/10 1:39 PM, James Porter wrote:
>> > I haven't seen any responses to this. I'm hoping this is because no one
>> > has a problem with it. I'll give it a few more days for comments, but if
>> > no one pipes up, I'll make the proposed changes.
>> >
>> > - Jim
>> >
>> > On Mon, 2010-10-04 at 14:55 -0500, James Porter wrote:
>> >> Currently in iRel, there are a handful of functions to get/set relations
>> >> that aren't one-to-one (e.g. iRel_getEntEntArrRelation). It's my
>> >> understanding that these are only meant to work in, say, Entity<->Set
>> >> relations, and that the entity array returned is the contents of the set
>> >> related to the entity on the left side. That is, it's equivalent to
>> >> calling iRel_getEntSetRelation and then i(Mesh|Geom)_getEntities on the
>> >> set you get back.
>> >>
>> >> I propose that these functions be removed. Here's why:
>> >>
>> >> Note 1: most of this is based on my discussion with Tim Tautges and our
>> >> implementation, Lasso, so hopefully I'm not just misinterpreting the
>> >> spec.
>> >>
>> >> Note 2: I use "left-" and "right-" prefixes to denote which side of the
>> >> relation an entity or set is on.
>> >>
>> >> 1) Problems with setting relations
>> >>
>> >> The biggest problem with these functions is in setting relations. What
>> >> you're actually doing is updating the contents of the related set to
>> >> contain those new elements. That is, if left-ent A is related to
>> >> right-set B, setting a relation between A and [C,D,...,Z] (where C-Z are
>> >> right-ents) actually inserts C-Z into B. That's certainly not what I'd
>> >> expect initially; I would expect iRel to only affect stuff in its own
>> >> area (e.g. relation tags set on each interface). It gets worse because
>> >> it's not clear what you should do when the set B doesn't exist (throw an
>> >> error? create a new set? Lasso does the former, currently).
>> >>
>> >> This also presents further problems because there's no corresponding
>> >> iRel_setEntSetArrRelation function, even though a set could be contained
>> >> in a set just as easily as an entity.
>> >>
>> >> 2) Confusion about iRel's capabilities
>> >>
>> >> iRel really creates one-to-one (possibly asymmetric) relations, but
>> >> these functions seem to imply that one-to-many or even many-to-many
>> >> relationships are possible. This is not, as far as I can tell, the
>> >> intention.
>> >>
>> >> 3) Not that useful
>> >>
>> >> In most cases, these functions only save a single line of code. If
>> >> you're trying to be truly generic (i.e. you don't know what the set-side
>> >> interface is), it's a little more complicated, but not much.
>> >>
>> >> 4) Hurts clarity of Python interfaces
>> >>
>> >> This is a mostly selfish reason, since I'm the developers of the ITAPS
>> >> Python interfaces, but if all relation functions are one-to-one, I'm
>> >> able to simplify the Python iRel interface's functions to "getRelation"
>> >> and "setRelation", since I know a priori the type of element that should
>> >> be returned when you call getRelation.
>> >>
>> >> - Jim
>> >>
>> >
>> >
>>
>
>
>