search

devosoft / Empirical

A library of tools for scientific software development, with emphasis on also being able to build web interfaces using Emscripten.
Other
86 stars 38 forks source link

emp::array uses dynamically allocated memory under the hood #408

Open mmore500 opened 3 years ago

mmore500 commented 3 years ago

Describe the bug A clear and concise description of what the bug is.

emp::array uses std::vector as a backend. For single process jobs, this works fine. For multiprocessing jobs, this creates an issue when passing arrays in messages between processes. Messages will contain pointers to data that allocated on one process that another processes doesn't have.

To my knowledge, there isn't a fundamental reason emp::array is implemented using std::vector. Looking into a quick patch, I found that working with std::vector is more convenient because its iterator is a proper class and therefore you can inherit from it. std::array's iterator is /not/ a class -- it's just a pointer so you can't inherit from it. But with a little elbow grease we should be able to switch out emp::array's backend to be std::array instead. Perhaps @mercere99 can weigh in on this assessment?

mercere99 commented 3 years ago

The comments made clear that I used std::vector as the base intentionally (for debug mode only), but I have no recollection why. Have you tried swapping in std::array to see what goes wrong? From your comment above, it sounds like I was more easily able to derive from vector's iterator in order to identify when an iterator have become invalid. Conveniently, the lack of size changes makes this a much rarer situation in arrays.

I have no issues with you making a fix! And if you come up with a reason why it really needs to be vector, please update the docs to indicate. ;-)

mmore500 commented 3 years ago

I did notice those comments :detective:, I'll take a shot at it!