CmPA / iPic3D

Particle-in-Cell code using the implicit moment method
71 stars 57 forks source link

Improve multidimensional arrays implementation #43

Open alecjohnson opened 10 years ago

alecjohnson commented 10 years ago

I want to check in code that changes the implementation of iPic3D arrays.

iPic3D currently uses chained pointers (e.g. double *\ arr3;) rather than array classes. This has a number of deficiencies:

I have therefore implemented array classes according to the following specifications:

I have implemented extensive performance tests of these arrays, and have compared performance of native arrays, chained pointers, and calculated index access for both g++ and icpc (Intel) and for both my laptop and the Xeon processor. With icpc on the Xeon processor, the performance differences can be significant and hard to predict. With g++, performance differences are much smaller. Using the -O3 flag to turn on optimization, I have verified that there is no performance difference resulting from any of the choices of syntax considered in the following paragraphs.

To avoid the need for systematic modifications to the code, I have implemented access of elements via bracket syntax, e.g. intarr[i][j]=5;. If I were writing a new code, I would prefer to use parenthesis syntax, e.g. myarr(i,j)=5;. But requiring such syntax in iPic3D would entail systematically changing about 1000 lines of code, mostly in the fields classes. Given the number of forked versions of iPic3D that need to be merged, it is not desirable to impose such a change at this time. We could use a script to change to such notation in the future; making the switch would be most painless at the end of a merge point in the software development cycle. Given the nature and the pressures of academic work, realizing such a merge point may not be easy.

Ideally, I prefer syntax which distinguishes write access from read access. At a minimum, we should support read-only array access so that we can pass const array references to methods that use array information without modifying it. Therefore, I have implemented get methods, e.g. var = intarr.get(i,j);.

For completeness, I have also implemented set methods, e.g. intarr.set(i,j, myvar);, allowing the user to clearly indicate write-access. I have also implemented a fetch() accessor, which allows both read and write access via:

  myArr.fetch(i,j) = 5;

Of course we could replace fetch with operator(), allowing the usage

  myArr(i,j) = 5;

as in the implementation of Vicenc Beltran and his collaborators, but I preferred being able to search for the string fetch in order to identify potential write-access. I will consider feedback offered in response to this issue and am willing to change fetch to operator() if this is the consensus desideratum.

alecjohnson commented 10 years ago

To maintain the same syntax that we have been using for arrays, I am mimicking boost::multi_array and boost::const_multi_array. In the process, I encountered a bug in the template support for the g++ compiler version 4.0.1 (2005). It is fixed in g++ 4.2.0 (2007). Do we want to support old compilers? That would make it more difficult to maintain the bracket syntax; we would have to give up on const correctness of arrays to do so (which, I admit, we didn't have in the first place).