[PyMOAB] Can't set data on mesh tag

[Guilherme Caminha]

I believe that the Range class is not accepting the root_set entity. Not sure why. For that reason, the tag_set_data method from PyMOAB can't figure out how to insert data for the root_set. Example:

>>> from pymoab import core, types
>>> mb = core.Core()
>>> tag = mb.tag_get_handle("test", 1, types.MB_TYPE_HANDLE, types.MB_TAG_MESH, True)
>>> a = mb.create_meshset()
>>> mb.tag_set_data(tag, 0, a)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "core.pyx", line 738, in pymoab.core.Core.tag_set_data (pymoab/core.cpp:6897)
AssertionError: Incorrect data length

>>> from pymoab import rng
>>> r = rng.Range(0)
>>> len(r)
0
>>> r.insert(0)
>>> len(r)
0

[Iulian Grindeanu]

root set is (internally) the zero set , 0

It is a special set, and it contains everything in the local instance.

It cannot be part of a range, as individual entities start at 1 (1 is a vertex; 0x1000....0001 is the first edge, etc)

[Guilherme Caminha]

I see. PyMOAB is internally converting any iterable or single entity you pass to the tag_set_data method into a Range and then using this range to call the MOAB implementation of tag_set_data. Probably it needs some special treatment for the root set.

[Guilherme Caminha]

This probably applies to other methods that might receive the root set as well.

[Iulian Grindeanu]

if pymoab is converting the root entity to a range, that is a problem with the implementation; it is a bug;

[Iulian Grindeanu]

MB_TAG_MESH type is a special tag storage type, that makes sense only in the "global" sense. It will always set a value to the "root" set, just because it should have the same interface; It is a global variable, in a sense; it is just "hooked" to the root set.

def tag_set_data(self, Tag tag, entity_handles, data, exceptions = ()) when the tag type is MB_TAG_MESH, should check if the "entity_handles" is the root set (0), and then set the data.

[Guilherme Caminha]

Makes sense to me. However, one might be interested in setting some value to the root set even if the tag type is not MB_TAG_MESH? There are other methods that would need a fix too, for example, tag_delete_data.

[Patrick Shriwise]

@iulian07 I did not know the root set couldn't be part of a Range. This is definitely a bug. In this case, PyMOAB should be using the vector-based function signatures for tag_set_data and other various methods. My apologies for the oversight.

I can submit a fix for this soon.

[Iulian Grindeanu]

Thanks @pshriwise and @Estanho for finding the issue;

I think it should be separated; as long as the tag is of type MB_TAG_MESH, it can take only root set as "entity", and the "vector" should have size 1; It should never allow to set another type of tag (storage type) on a root set;

tag_set_data() api has 2 variants in moab, one with range, one with vector of entities; it should have had a third variant, just for the root set (so it would take no "entity"). But the decision was made probably more than 10 years ago to use the variant with vector, and of size 1;

In moab Core, both tag_set_data methods immediately delegate to derived tag classes (TagInfo is abstract class)

also, I noticed that the tag_set_data doc is wrong
def tag_set_data(self, Tag tag, entity_handles, data, exceptions = ()): """ Create an elments of type, entity_type, using vertex EntityHandles in connectivity.

[Patrick Shriwise]

There are some methods in PyMOAB such as get_entities_by_type or get_entities_by_type_and_tag which currently return Ranges. For the Range-based methods in MOAB, these methods cannot return the root_set, right?

In PyMOAB it was convenient to the return result of these queries as Ranges due to the convenience functions that come along with Ranges, but should I be returning these as numpy arrays so that the root_set can be included?

[Iulian Grindeanu]

I think the user should know not to expect root_set from ranges. (or it should be made clear from docs); root set cannot be returned when you asked get_entities_by_type. It is not of "MBENTITYSET" type.

root set should never be returned in search queries; MB_TAG_MESH type should be set only on the root set, so probably we should just document this better. Whenever we have a MB_TAG_MESH, we know it is just set on the root set; no other tag should be set on the root set;

the code from @Estanho should work, though, I am not sure what is the best way to fix it

[Guilherme Caminha]

If the root set cannot be tagged by any other tag type, and therefore cannot be returned by other methods, then it's just this special case and using Range as default should be fine.

[Iulian Grindeanu]

Actually, there is a bigger problem with converting to ranges; when you use arrays of entities (numpy arrays?), they could come in unordered, and by converting to range, they are becoming basically ordered; effectively the setting of tags can be wrong.

The same when you are retrieving tags

[Guilherme Caminha]

I see, although I've been using PyMOAB a bit and haven't seen this happening yet.

[Patrick Shriwise]

Glad to hear that returning Ranges isn't going to be an issue. I think it will a lot in memory savings an in convenience for querying entity types, etc.

@Estanho most of the time EntityHandles are going to naturally appear in order, but I think @iulian07 is right - they could be unordered.

A fix for this issue is in progress. It creates numpy arrays for the EntityHandles passed to the tag_get/set/delete_data methods and should address both of these problems. I'll take a look at the tag_get/set_by_ptr methods as well to make sure we won't have problems there either.

[Guilherme Caminha]

Ok, thanks!

[Vijay M]

As @iulian07 points out, the conversion of unordered to ordered insertions with ranges can be a problem. @pshriwise we need to look more carefully at the implementation to see whether this is violated currently. Adding a unit test for this will also be useful.

Regarding the root set, the user should treat this as a special case. So if we want to expose a specific method for this in the API, I'm fine with it.

[Patrick Shriwise]

I'll review the methods in pymoab.core to make sure that Ranges aren't used where users will expect data or entities to come back in the same order as an array of EntityHandles that they provide.