Open vincentlaboure opened 7 years ago
(similarly to subdomain_id, we need to introduce material_id for material properties assignment, depletion_id for transient considerations etc...) but also to keep some data in cache for local operations (e.g. sweeping/local inversion etc...)
In hindsight, we should never have suggested users treat subdomain_id as a raw meaningful value. Storing other ids and data is how you should use subdomain_id - as an index into a container of anything-your-heart-can-imagine. Once you treat subdomain_id as an object of user interest, though, you can no longer change it without surprising the user, and then you can no longer use it to add any other data...
but also to keep some data in cache for local operations (e.g. sweeping/local inversion etc...).
This isn't a bad idea, but if it wasn't for the parallelization question I'd still think it was better addressed via indexing unique_id() into an external container.
That said: Void pointer? Absolutely not. Find the C programmer who first suggested that, and make them set up a fuzz testing suite or something as ironic punishment for unsafe language use.
"UserData" abstract base class pointer, with pure virtual methods for serialization and deserialization, though, could make sense. We'd want it to be a configure-time option, though, not saddle everyone with an extra 8 bytes per element. It would also be libmesh_experimental() for a while, because we're not going to get the API right on the first pass (should we have methods for generating new values when child elements are created by refinement? For updating parent element values when child elements are coarsened away?) and it would be a nightmare to try to maintain backwards compatibility through every design iteration.
Oh, and we need to work out "How do two codes, who know nothing about each other, each attach and query their own UserData subclass objects (which may or may not be of the same subclass) without stepping on each others' toes?", and that may be important enough to get into design iteration 1. Treating subdomain_id as a one-use-case pony is half the reason for adding user_data(), and I don't want to repeat that mistake and end up needing more_user_data() down the road.
So let us add ElemUserData
abstract class and a ElemUserData
pointer in Elem
? The abstract class can provide some utility functions to serialize data, which is actually the reason why this data is hold by libMesh. Otherwise we can add the data on MOOSE or application side. We can also have a virtual member function childUserData()
which by default returns this
and can be used by refining elem. Let us do not touch parent user data by child. Apparently, user will be responsible for making ElemUserData
concrete and constructing it. Yes, we can have a configure flag to enable or disable ElemUserData
.
I feel the stateful material properties in MOOSE are like this user data.
In hindsight, we should never have suggested users treat subdomain_id as a raw meaningful value. Storing other ids and data is how you should use subdomain_id - as an index into a container of anything-your-heart-can-imagine.
Hmm, well I think this user data pointer comes up now because of the complication with DistributedMesh. We have data that may need to be shipped with the Element so the mapping idea doesn't work out so well. I think @YaqiWang and other users would be fine with not having the data attached directly to the element as long as there was a sensible way of migrating it from processor to processor. In MOOSE-based apps, we've long needed the ability to move our material property storage from processor to processor. We are actually closer than you might think. Our material properties are currently already serializable. @friedmud came up with the dataStore<>(), dataLoad<>()
, methods that we look for whenever we create backups for checkpointing.
Another solution to this whole problem would be to have a way of saying, OK, parmetis just repartitioned and libMesh just shipped a bunch of elements around. Now we need to serialize our disconnected data structures and ship those around too. That's the requirement, it doesn't necessarily have to be fulfilled by an extra data pointer on the element.
"UserData" abstract base class pointer, with pure virtual methods for serialization and deserialization, though, could make sense. We'd want it to be a configure-time option, though, not saddle everyone with an extra 8 bytes per element. It would also be libmesh_experimental() for a while, because we're not going to get the API right on the first pass (should we have methods for generating new values when child elements are created by refinement? For updating parent element values when child elements are coarsened away?)
Yes, but I think that 99% of cases will be clone the parent data into each child's data. Likewise coarsening would just be deletion of said data.
and it would be a nightmare to try to maintain backwards compatibility through every design iteration.
Oh, and we need to work out "How do two codes, who know nothing about each other, each attach and query their own UserData subclass objects (which may or may not be of the same subclass) without stepping on each others' toes?"
So this might be a reason to come up with a different solution to moving data around. If we just provided a routine to have on demand arbitrary user serialization for selected elements, I think people would be happy.
Yeah, this would be a great way to handle the stateful material properties + distributed mesh case.
The other alternative I can see, though, would be a global callback function: every time we serialize an element, we call something like serialize_data_for(const Elem &, iterator buffer_out)
on the sending side, and unserialize_data_for(Elem &, const iterator buffer_in)
on the receiving side. That would be slightly less CPU efficient, since you'd have to have an unordered_map at best for data storage, but it would allow us to get rid of the configure-time option, since users who didn't want per-element data would no longer have any memory overhead. And getting rid of the configure-time option would mean better test coverage for everybody.
I think I'm still leaning toward the UserData pointer (on both Elem and Node, perhaps with a separate configure option for each?) but let me know what you both think, because the configure-option-free alternative is tempting.
The other alternative I can see, though, would be a global callback function: every time we serialize an element, we call something like serialize_data_for(const Elem &, iterator buffer_out) on the sending side, and unserialize_data_for(Elem &, const iterator buffer_in) on the receiving side.
That would certainly meet our needs just as well as having the pointer in the dof objects themselves. In either case, we are going to have to create these serialization routines and call them at the right place and handle the communication of that data among processors under the hood. From a developer perspective, I don't see much of a difference in how we'll use the system with these two approaches.
I want to tag @friedmud in here since this is a really important design aspect that we need to get right.
You know... we could actually hide the storage details on this. DofObject::get_user_data() and DofObject::set_user_data(UserData *) don't have to be dumb accessors to a _user_data pointer, they could query a static unordered_map themselves. We could do the static unordered_map implementation first, then add a per-object-pointer option later and see what the performance gain (or loss?) is.
The only detail I don't think we can postpone is the "distinguishing between multiple usages' distinct UserData" detail. We wouldn't want just "get_user_data()", we'd want something like get_user_data<SomeFastIdentifier>()
or get_user_data(const string & some_flexible_identifier)
and I'm not sure which.
I've gone back and forth on this over the years... and still haven't quite made up my mind... but I'll give you my current thoughts:
My initial inclination is that we want this pointer (or pointer-like object, etc.) as part of the element... the reason is that I want O(1) access to the data. I have an application where I really need to access that data really quickly without going through any sort of a map or hash.
However, two things hold me back:
SO: Here's where I currently stand... I think it's better to handle this in application space using unique_id into a map (just pay the price - whatevs)... which is basically what we already do for stateful properties.
BUT: what I would like to see is some libMesh interface for helping an application ship around data it may be storing this way. Some sort of a callback that says serialize_the_data_for_these_elems(std::vector<Elem *>)
and then we give you back the serialized data (just a raw set of bits)... you send it off and then give it back to us unpack_the_data_for_these_elems(raw_bits)
.
libMesh knows where the elements went... and where they came from... so it's in the best position to do this brokering.
This would definitely allow us to properly handle stateful property migration... and we could then grow a system in MOOSE for users to be able to attach other data to elements that aren't necessarily properties that MOOSE would then provide back to them through a nice interface.
I want O(1) access to the data. I have an application where I really need to access that data really quickly without going through any sort of a map or hash.
So no maps and no std::string identifiers, then.
I think the "multiple use" thing is half the risk behind "potential for abuse", and we need to get that straightened out regardless.
The other half of the risk is bugs in serialization or deserialization, and for that we can do the "magic numbers in debug mode" trick that libMesh currently does for packed elements and nodes themselves. That's fairly helpful for tracking down bugs, and it's insanely helpful for detecting bugs without corrupting results. Except, this time we'll be smart and put the magic number trick at the higher level rather than mixed into each particular type's packing. This is actually an argument for making a serialization callback (or virtual function) happen on each object rather than on ranges of objects, so we can detect data sync failure bugs on the exact object where they happen rather than at the end of the range or at the point where mis-sync causes a segfault.
If we do things in application space, serialize and unpack aren't enough; we'll at least also need "clear_the_data_for_these_elems()", not just for memory savings but to try and avoid "dangling" data (like in the bug we're sorting out right now with periodic AMR in MOOSE...).
Even "identifiers" aren't really that great... gotta come up with a way to register unique ones
Yeah. I'm not as troubled by that, though; it'd be easy enough to set up something like a factory pattern that assigns a unique index number from a contiguous range into every class that has its own UserDataIndex static member variable.
Hmm... except we'd definitely want this to be a per-mesh indexing, not a global indexing. So it might be better to avoid nasty C++ global constructor tricks entirely and just require that any code which wants user data call some MeshBase::add_nodal_user_data()
and/or MeshBase::add_elemental_user_data()
before generating or reading the mesh contents. Those could return the data indices. I think I'd prefer to hide data indices from application code altogether, but the only alternatives I see are things like "every different type of user data has to be a different subclass", which would have even bigger disadvantages.
If it is UserData
, it is user's data and user's responsibility to construct it (we do not have to ask users to construct the data at the time Elem
is constructed) and properly use it. As framework, I think that only few things need by the framework:
Elem
is deleted, which requires the destructor of UserData
;serialize
and deserialize
;UserData * child_user_data(unsigned int child_id) const
(can return this->user_data()
by default);UserData * collect_child_user_data() const
(can return this->user_data()
by default);UserData * side_user_data(unsigned int side) const
(can return null by default)I'd rather have the O(1) access than worrying about the abusing of the data which is created by the user in the first place. To us, we will only have few more IDs for elements. Framework needs to give some flexibilities to users and cannot push everything to users. You will push users to make their own framework, for example, create their own copy of libMesh and add few member functions.
If we're using the destructor of UserData, then we can't also have child_user_data()
return this
, or we'll end up freeing dangling pointers when we remote or delete multiple children.
That's a good question to bring up, though. Do we want to use shared_ptr for this case? Or a clone() method?
The desire to associate data with sides and edges is another good point to bring up. I'd only been considering data associated with elements and nodes. My first inclination is to say "if you want edge or face data, use second order elements and assign it to the edge or face nodes", but I'm not sure if that's acceptable to users, especially considering that we have some elements (tets, pyramids, and prisms) where even the second-order versions don't have face nodes. (Not currently, anyway; that's been on my todo list forever, so we can support p>2 shape functions on those elements)
Wouldn't using a shared_ptr lock us into keeping the same data among children elements. Users would probably eventually want to clone data so that it could be changed as you refine. Of course allowing that to happen makes it complicated to deal with coarsening cases. Derek has worked through some of this with the stateful material property system. Right now he simply clones data during refinement, then just selects the the "first closest" data during coarsening and discards the rest. It's hard to do something much better than that from a framework perspective. Sure, I suppose you could create yet another user function that takes in all of the data that must be averaged or aggregated and allow the user to make that decision.
As discussed with @YaqiWang and @permcody, it is highly desired to have the ability to add member variables to
elem
(e.g. through a void pointer that the user can assign to a particularstruct
).Applications are very general and include being able to add various id's to the mesh (similarly to
subdomain_id
, we need to introducematerial_id
for material properties assignment,depletion_id
for transient considerations etc...) but also to keep some data in cache for local operations (e.g. sweeping/local inversion etc...).This change need be directly made in libmesh to fully leverage its parallelization capabilities.