-
Notifications
You must be signed in to change notification settings - Fork 13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Optionally support user data of arbitrary size. #60
Comments
Just out of curiosity - why uint8_t* ? Not that I think it matters really as nothing is ever converted - but it introduces an extra unnecessary cast, doesn't it? |
Well @ebknudsen, cast from and to what? :-) The idea is basically to allow advanced users to put N custom bytes on each particle, leaving it up to them what they want to use them for. That could be some integers, some floating point numbers, or some combination or whatever. Of course, the burden will be on such advanced users to perform proper (de)serialisation of the data with all that entails of funny and tricky portability issues (e.g. endianess). Or perhaps you meant why Or did I misunderstand your question? |
Nope - the second understanding is what I meant. |
Well, it is a matter of taste I guess, and probably there would not be much difference in practice. I also don't really think there will be any "extra cast" in one solution as opposed to another (either in code or binary, although perhaps at some intermediate level of compiler representation). Unless of course you actually want a bunch of But most importantly to me, a |
Btw., your question made me recall that we already have "binary blob" support, although per-file not per-particle:
So for consistency one might argue for |
Just to continue arguing :-), I may be missing something but I don't see how a cast to anything specific (like uint8_t*) is going to change the fact that whatever bits you put in the field is going to have to be interpreted by the user. Any sequence of n x 8 bits can always be converted into 8-bit integers, no matter what it is really supposed to be. |
Hombres @ebknudsen and @tkittel, my 0.02$: "Just choose something and document that well." :-D |
@willend: yes, I will of course do just that :-) @ebknudsen, since you are not completely convinced allow me a few more arguments. Perhaps first let us ask another way: What would be gained by using a void pointer which contains absolutely no type information? To me, basically And yes, a native fortran MCPL reader would certainly be a possibility, although perhaps a better example would be the native Python reader which we already have. Here we will be reading such data using Of course, the backdrop of this discussion is that presently in real life, char's, void*, uint8_t, etc. all points to groups with 8 bits, and we have byte-wise access. So at the end of the day users can use and abuse these uint8 arrays in whatever way they want. But the important point for me is that we think about the "contract" and support provided by a project like MCPL. For that it will be simplest to have a well defined contract about what we provide ("arrays of uint8, use them as you wish but don't complain to us if you mess up"). 😄 |
Well - I can think of one thing that would (in my mind) be gained from using a void* (as opposed to a uint8_t* ) - not very significant but nonetheless. A void* is an clear indicator that it is up to the user to define what is being put here, and that the format simply stores a set of bits (or indeed bytes), no questions asked. A cast pointer suggests a preference in some way, even though there is none. An equally valid passage in the docs would be that the userdata pointer is simply a pointer - do what you wish with it, and that would also be an equally valid contract between user and developer. All that said - the fact that c++-lore discourages the use of void* is to me the closing argument. Consider me convinced :-) |
Yeah... I certainly see your point about the semantics of a void* pointer in C, and if we were purely discussing a C-API I probably would agree with you. It is just that I am thinking more about the on-disk format, documentation, and C++/python/... code. Anyway, by the time I will actually get around to working on this issue, the current-best-practices etc. might already have changed 😁 |
Currently custom user-data is always in the form of a 32bit unsigned integer:
However, one potential user just had a use-case needing to store 2 floating point numbers which would clearly not fit. It would make sense to add a new version of the MCPL format where user flag data could be of arbitrary (but fixed) size. I.e. N bytes, made available as a pointer:
uint8_t* userdata;
.The text was updated successfully, but these errors were encountered: