Changed Cells, CellLookup, MCells, and MCellsLookup to std::vector(...)

[chrisatang]

Changed the Cells, CellLookup, MCells, and MCellsLookup from dynamic calloc(...) to std::vector(...), which is a start to resolving any possible memory issues the current code might have.

[matt-gretton-dann]

Thank you for your PR, we are currently very busy and so responses will take a while.

[matt-gretton-dann]

You are trying to initialise these at program start up time when P.NC is 0 - so this will fail - I think need to have something like:

std::vector<cell> Cells;

// Whereever the calloc was
Cells.resize(P.NC);

etc.

[matt-gretton-dann]

std::vector::at() will throw an exception on a bad access. Not a bad thing per se - however, the code isn't exception aware and I think there will be problems with exceptions crossing thread boundaries.

There are two solutions to this:

The easy one: Just use [] access everywhere. (Why do you use it in some places and not others?).

The harder one, put try/catch blocks everywhere that is appropriate.

[cjgdev]

Just to expand on the uncaught exceptions issue flagged here, the OpenMP specification mandates that an exception thrown within the omp parallel for region must cause the thread to resume within the same region (ie. no uncaught exceptions within omp parallel for loops).

I second the suggestion to prefer the use of operator[], rather than wrapping inner loops in try catch blocks, since I don't think there's much that can be done about an invalid index in either case, plus a segmentation fault raised by an invalid index is more obviously wrong.

[cjgdev]

Suggested change

	zfwrite_big((void*)&Cells.front(), sizeof(cell), (size_t)P.NC, dat);
	zfwrite_big((void*)Cells.data(), sizeof(cell), (size_t)P.NC, dat);

Calling front results in undefined behaviour in the case where Cells is empty. C++11 introduced std::vector<T,Allocator>::data, which will simply return a null pointer in the case of an empty vector. We should use that here.

[cjgdev]

Just to expand on the uncaught exceptions issue flagged here, the OpenMP specification mandates that an exception thrown within the omp parallel for region must cause the thread to resume within the same region (ie. no uncaught exceptions within omp parallel for loops).

I second the suggestion to prefer the use of operator[], rather than wrapping inner loops in try catch blocks, since I don't think there's much that can be done about an invalid index in either case, plus a segmentation fault raised by an invalid index is more obviously wrong.