openPMD / openPMD-api

:floppy_disk: C++ & Python API for Scientific I/O
https://openpmd-api.readthedocs.io
GNU Lesser General Public License v3.0
142 stars 51 forks source link

Setters/Getters of Attributable #255

Open ax3l opened 6 years ago

ax3l commented 6 years ago

It's cool that a Container itself is attributable, but why are attributes not simply implemented as Container themselves as well?

I think similar to h5py's AttributeManager, using our container both for data sets and attributes would make it very natural to interact with them. Or am I missing something?

C0nsultant commented 6 years ago

I frankly do not understand your question or the motivation behind it.

DISCLAIMER - Attributes were one of the first features. Please excuse design flaws and antipatterns. Let me elaborate a bit on the choices concerning attributes: There's three major types of abstract objects in this API:

  1. Datasets (i.e. RecordComponent)
  2. Groups with arbitrary child-nodes (i.e. Mesh), all implementing the Container interface
  3. Groups with well-defined child-nodes (i.e. Iteration)

All of these can be attributed with arbitrary key-value pairs (with Attributable::setAttribute()). Some of them additionally carry pairs that are restricted by openPMD (e.g. Series::setOpenPMD()).

To have management of attributes centralized, the Attributable mixin was created. Essentially it boils down to a map that stores key-value pairs with std::string keys and Attribute values. The attributes may be of any Datatype mentioned in openPMD and supported by major backends.

At the moment I can not see how Container fits into the frame. It is a mixin for the core logic needed in arbitrary Groups (2.). Just like Attributable it boils down to a key-value store, but the repsonsibilities are different. It should handle like a map. Attributable shouldn't. Thinking about it... Are you asking why Container is not used internally in Attributable? Currently, that would be a chicken-and-egg problem, but could possibly be solved.

ax3l commented 6 years ago

The thing that I mean is: why do we need a set/get for attributes?

Could we just do a [...].attributes["newkey"] = T and T = [...].attributes["getkey"] as well? Isn't this nice and similar to what the container interface does?

C0nsultant commented 6 years ago

why do we need a set/get for attributes?

We allow attributes of arbitrary datatype, C++ is a strongly typed language, and we have to enforce datatypes at certain times.

We could certainly hack in something along the lines of [...].attributes["newkey"] = T, but how would you ensure the correct type of T if it is dictated by openPMD? For eaxmple, how would you ensure that in

Series s = /*...*/;
s.attributes["openPMDextension"] = 4;

openPMDextension would be stored as uint32_t?

Certainly the syntax would be nice. I could not agree with you more. I've even though about implementing something like T = [...].attributes["getkey"] for (floatX) in openPMD (by abusing operator float()/operator double()/operator long double() to do the implicit type casting).

I may simply not be aware of the techniques... If you show me how to achieve this in C++, I'm happy to do it.

ax3l commented 6 years ago

I see, I am clearly writing too many python bindings... :-D