-
Notifications
You must be signed in to change notification settings - Fork 5
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
std::bad_alloc in WriteSOCatalog #71
Comments
I went back to this issue yesterday and today. This is caused by a copy operation of many PIDs from a array of vectors into a single vector, which is then used for writing the resulting data into disk. The same applies for an array of types: Lines 1506 to 1525 in 64de17b
It just so happens that the original array of vectors holds quite a big number of IDs (in our case
With This has been a known problem for a while though: Lines 1213 to 1214 in 64de17b
I'll relabel this as an improvement rather than a bug. |
In the original code, the original each of the vectors in the array-of-vectors was emptied after their contents were copied (but it wasn't shrunk via A non-invasive, possible improvement for this situation is then to resize the 1D array multiple times as we iterate over the original array-of-vectors, and as we effectively release the memory held by each of those individual vectors by calling |
Two other ideas about this that can be quickly implemented:
|
Spherical overdensity information (Particle IDs and types) are collected by two different routines: GetInclusiveMasses and GetSOMasses. Both routines use the following technique: first, for "ngroup" groups, a C array-of-vectors (AOV) of "ngroup + 1" elements was allocated via "new" (which was eventually delete[]'ed). Vectors for each group are filled independently as groups are processed; the loop over groups is 1-indexed, and indexing into these AOVs happens using the same iteration variable, meaning that their 0 element is skipped. Finally, these AOVs are passed to WriteSOCatalog for writing. WritingSOCatalog is aware of the 1-based indexing, and additionally it flattens the AOVs into a single vector (thus duplicating their memory requirement) to finally write the data into the output HDF5 file. This commit originally aimed to reduce the memory overhead of the final writing of data into the HDF5 file (see #71). The main change required to achieve this is to perform the flattening of data at separate times, such that particles IDs and types are not flattened at the same time, but one after the other, with memory from the first flattening operation being released before the second flattening happens, thus reducing the maximum memory requirement of the application. This goal was achieved. However, while performing these changes two things became clear: firstly, that using a vector-of-vectors (VOV) was a better interface than an AOV (due to automatic memory management), and secondly that the 1-based indexing of the original AOVs introduced much complexity in the code. The scope of these changes was then broadened to cover these two extra changes, and therefore this commit considerably grew in size. In particular the 0-indexing of the VOVs allowed us to more easily use more std algorithms that clarify the intent in certain places of the code. There are other minor changes that have also been included in this commit, mostly to reduce variable scopes, reduce code duplication, and such. Assertions have also been sprinkled here and there to add further assurance that the code is working as expected. As an unintended side-effect, this commit also fixed the wrongly-calculated Offset dataset, which was off by one index in the original values. This problem was reported in #108, and seems to have always been there.
The first bullet point mentioned in the last comment has now been merged into the Although this is progress, a better solution is of course to avoid having to copy data in the first place, which should be possible if we collected PIDs and types into a single vector in the first place. This could be investigated in the future. |
VR is crashing in
WriteSOCatalog
with the following message:This feels like a bug introduced with #57, but it will require a bit of investigation.
The text was updated successfully, but these errors were encountered: