IPPL Indexes support strides. However, a lot of the codebase just assumes stride 1.
This happens all over the place. Even within the class itself. For example:
In /src/Index/Index.hpp:
|
KOKKOS_INLINE_FUNCTION bool Index::touches(const Index& a) const { |
|
return (min() <= a.max()) && (max() >= a.min()); |
|
} |
|
|
|
KOKKOS_INLINE_FUNCTION bool Index::contains(const Index& a) const { |
|
return (min() <= a.min()) && (max() >= a.max()); |
|
} |
For example,
Index(0, 10, 2) represents
{0, 2, 4, 6, 8, 10} and
Index(1, 9, 2) represents
{1, 3, 5, 7, 9}. Their
min()/
max() ranges overlap
[0,10] vs
[1,9], but they share zero elements.
touches() returns
true and
contains() returns
true, both incorrectly.
|
KOKKOS_INLINE_FUNCTION Index Index::grow(int ncells) const { |
|
Index index; |
|
|
|
index.first_m = this->first_m - ncells; |
|
index.length_m = this->length_m + 2 * ncells; |
|
index.stride_m = this->stride_m; |
|
|
|
return index; |
|
} |
This should multiply
ncells by stride before shifting
first_m.
In src/Field/BcTypes.hpp:
|
int offset, offsetRecv, matchtag; |
|
if (face & 1) { |
|
// upper face |
|
offset = -domain[d].length(); |
|
offsetRecv = nghost; |
|
matchtag = comm.preceding_tag(mpi::tag::BC_PARALLEL_PERIODIC); |
|
} else { |
|
// lower face |
|
offset = domain[d].length(); |
|
offsetRecv = -nghost; |
|
matchtag = comm.following_tag(mpi::tag::BC_PARALLEL_PERIODIC); |
|
} |
There are many places where calculations break because of the assumption that
lenght() == size(). Obviously, using a strided index, this does not hold anymore.
|
for (size_t j = 0; j < Dim; ++j) { |
|
range.lo[j] = overlap[j].first() - nd[j].first() + nghost; |
|
range.hi[j] = overlap[j].last() - nd[j].first() + nghost + 1; |
|
} |
Calculations like this should become this:
https://github.com/NickvonWit/ippl/blob/a8c423af24d4f3e2dc15edad512ba38ad0c044ed/src/Field/BcTypes.hpp#L257-L262
for (size_t j = 0; j < Dim; ++j) {
const int stride = nd[j].stride();
range.lo[j] = (overlap[j].first() - nd[j].first()) / stride + nghost;
range.hi[j] = (overlap[j].last() - nd[j].first()) / stride + nghost + 1;
}
There are many other parts of the codebase that have this issue.
IPPL Indexes support strides. However, a lot of the codebase just assumes stride 1.
This happens all over the place. Even within the class itself. For example:
In
/src/Index/Index.hpp:ippl/src/Index/Index.hpp
Lines 154 to 160 in c2ec8aa
For example,
Index(0, 10, 2)represents{0, 2, 4, 6, 8, 10}and Index(1, 9, 2)represents{1, 3, 5, 7, 9}. Theirmin()/max()ranges overlap[0,10]vs[1,9], but they share zero elements. touches()returns trueand contains()returnstrue, both incorrectly.ippl/src/Index/Index.hpp
Lines 261 to 269 in c2ec8aa
This should multiply
ncellsby stride before shiftingfirst_m.In
src/Field/BcTypes.hpp:ippl/src/Field/BcTypes.hpp
Lines 219 to 230 in c2ec8aa
There are many places where calculations break because of the assumption that
lenght() == size(). Obviously, using a strided index, this does not hold anymore.ippl/src/Field/BcTypes.hpp
Lines 255 to 258 in c2ec8aa
Calculations like this should become this:
https://github.com/NickvonWit/ippl/blob/a8c423af24d4f3e2dc15edad512ba38ad0c044ed/src/Field/BcTypes.hpp#L257-L262
There are many other parts of the codebase that have this issue.