Skip to content

Commit

Permalink
PureSoA: Disable AoS Access (#3290)
Browse files Browse the repository at this point in the history
## Summary

When using a PureSoA particle container, trying to access the AoS
components will now result in a compile-time error.

Previously it still compiled, and I suspect it even allocated memory for
AoS.
New error message:
```
/hipace/src/Hipace.cpp:1499:39: error: ‘struct amrex::ThisParticleTileHasNoAoS’ has no member named ‘begin’
 1499 |         const auto& pos_structs = aos.begin() + nreal;
      |                                       ^~~~~
```
Using this I found more places that needed to be updated to account for
PureSoA.


PureSoA still has the following issues:
- [x] I believe it’s not possible to use the binning functions (in
`DenseBins.H`) as they require a particle pointer.
- [ ] The `ParticleTileData` struct that is used to access SoA data can
be very large, over 300 bytes in some cases. I recommend changing the
array-of-pointers to a full 2D array. This way ParticleTileData could be
as small as 24 bytes (real pointer, int pointer, stride).


## Additional background

Follow-up to #2878.

## Checklist

The proposed changes:
- [x] fix a bug or incorrect behavior in AMReX
- [ ] add new capabilities to AMReX
- [ ] changes answers in the test suite to more than roundoff level
- [ ] are likely to significantly affect the results of downstream AMReX
users
- [ ] include documentation in the code and/or rst files, if appropriate

---------

Co-authored-by: Axel Huebl <axel.huebl@plasma.ninja>
  • Loading branch information
AlexanderSinn and ax3l authored Sep 22, 2023
1 parent b2052f2 commit 2e99628
Show file tree
Hide file tree
Showing 6 changed files with 32 additions and 21 deletions.
16 changes: 8 additions & 8 deletions Src/Extern/SENSEI/AMReX_ParticleDataAdaptorI.H
Original file line number Diff line number Diff line change
Expand Up @@ -581,14 +581,14 @@ int ParticleDataAdaptor<ParticleType, NArrayReal, NArrayInt>::GetMeshMetadata(
for (const auto& kv : pmap)
{
// loop over particles in ParticleTile
const auto& aos = kv.second.GetArrayOfStructs();
const auto &parts = aos();
auto& particle_tile = kv.second;
auto ptd = particle_tile.getConstParticleTileData();

// visit only the "real" particles, skip the "neighbor" particles.
long long numReal = aos.numRealParticles();
long long numReal = particle_tile.numRealParticles();
for (long long i = 0; i < numReal; ++i)
{
const auto &part = parts[i];
const auto &part = ptd[i];
if (part.id() > 0)
{
#if (AMREX_SPACEDIM == 1)
Expand Down Expand Up @@ -869,7 +869,7 @@ int ParticleDataAdaptor<ParticleType, NArrayReal, NArrayInt>::AddParticlesSOARea
for (MyParIter pti(*this->m_particles, level); pti.isValid(); ++pti)
{
auto& particle_attributes = pti.GetStructOfArrays();
auto& aos = pti.GetArrayOfStructs();
auto ptd = pti.GetParticleTile().getParticleTileData();

auto numReal = pti.numParticles();
// shuffle from the AMReX component order
Expand All @@ -881,7 +881,7 @@ int ParticleDataAdaptor<ParticleType, NArrayReal, NArrayInt>::AddParticlesSOARea

for (long long i = 0; i < numReal; ++i)
{
const auto &part = aos[i];
const auto &part = ptd[i];
if (part.id() > 0)
{
pData[i*nComps + j] = realData[i];
Expand Down Expand Up @@ -944,7 +944,7 @@ int ParticleDataAdaptor<ParticleType, NArrayReal, NArrayInt>::AddParticlesSOAInt
for (MyParIter pti(*this->m_particles, level); pti.isValid(); ++pti)
{
auto& particle_attributes = pti.GetStructOfArrays();
auto& aos = pti.GetArrayOfStructs();
auto ptd = pti.GetParticleTile().getParticleTileData();

auto numReal = pti.numParticles();
// shuffle from the AMReX component order
Expand All @@ -953,7 +953,7 @@ int ParticleDataAdaptor<ParticleType, NArrayReal, NArrayInt>::AddParticlesSOAInt

for (long long i = 0; i < numReal; ++i)
{
const auto &part = aos[i];
const auto &part = ptd[i];
if (part.id() > 0)
{
pData[i] = intData[i];
Expand Down
18 changes: 10 additions & 8 deletions Src/Particle/AMReX_ParticleContainerI.H
Original file line number Diff line number Diff line change
Expand Up @@ -1653,8 +1653,8 @@ ParticleContainer_impl<ParticleType, NArrayReal, NArrayInt, Allocator, CellAssig
if (npart != 0) {
Long last = npart - 1;
Long pindex = 0;
auto ptd = particle_tile->getParticleTileData();
while (pindex <= last) {
auto ptd = particle_tile->getParticleTileData();
ParticleType p(ptd,pindex);

if ((remove_negative == false) && (p.id() < 0)) {
Expand All @@ -1669,7 +1669,7 @@ ParticleContainer_impl<ParticleType, NArrayReal, NArrayInt, Allocator, CellAssig
for (int comp = 0; comp < NumIntComps(); comp++) {
soa.GetIntData(comp)[pindex] = soa.GetIntData(comp)[last];
}
correctCellVectors(last, pindex, grid, aos[pindex]);
correctCellVectors(last, pindex, grid, ptd[pindex]);
--last;
continue;
}
Expand All @@ -1685,7 +1685,7 @@ ParticleContainer_impl<ParticleType, NArrayReal, NArrayInt, Allocator, CellAssig
for (int comp = 0; comp < NumIntComps(); comp++) {
soa.GetIntData(comp)[pindex] = soa.GetIntData(comp)[last];
}
correctCellVectors(last, pindex, grid, aos[pindex]);
correctCellVectors(last, pindex, grid, ptd[pindex]);
--last;
continue;
}
Expand Down Expand Up @@ -1739,7 +1739,7 @@ ParticleContainer_impl<ParticleType, NArrayReal, NArrayInt, Allocator, CellAssig
for (int comp = 0; comp < NumIntComps(); comp++) {
soa.GetIntData(comp)[pindex] = soa.GetIntData(comp)[last];
}
correctCellVectors(last, pindex, grid, aos[pindex]);
correctCellVectors(last, pindex, grid, ptd[pindex]);
--last;
continue;
}
Expand Down Expand Up @@ -2206,13 +2206,15 @@ RedistributeMPI (std::map<int, Vector<char> >& not_ours,
const auto& src_tile = kv.second;

auto& dst_tile = GetParticles(host_lev)[std::make_pair(grid,tile)];
auto old_size = dst_tile.GetArrayOfStructs().size();
auto old_size = dst_tile.size();
auto new_size = old_size + src_tile.size();
dst_tile.resize(new_size);

Gpu::copyAsync(Gpu::hostToDevice,
src_tile.begin(), src_tile.end(),
dst_tile.GetArrayOfStructs().begin() + old_size);
if constexpr(! ParticleType::is_soa_particle) {
Gpu::copyAsync(Gpu::hostToDevice,
src_tile.begin(), src_tile.end(),
dst_tile.GetArrayOfStructs().begin() + old_size);
}

for (int i = 0; i < NumRealComps(); ++i) {
Gpu::copyAsync(Gpu::hostToDevice,
Expand Down
4 changes: 2 additions & 2 deletions Src/Particle/AMReX_ParticleInit.H
Original file line number Diff line number Diff line change
Expand Up @@ -1143,7 +1143,7 @@ InitRandom (Long icount,
const auto& src_tile = kv.second;

auto& dst_tile = GetParticles(host_lev)[std::make_pair(grid,tile)];
auto old_size = dst_tile.GetArrayOfStructs().size();
auto old_size = dst_tile.size();
auto new_size = old_size;
if constexpr(!ParticleType::is_soa_particle)
{
Expand Down Expand Up @@ -1286,7 +1286,7 @@ InitRandom (Long icount,
const auto& src_tile = kv.second;

auto& dst_tile = GetParticles(host_lev)[std::make_pair(grid,tile)];
auto old_size = dst_tile.GetArrayOfStructs().size();
auto old_size = dst_tile.size();
auto new_size = old_size;
if constexpr(!ParticleType::is_soa_particle)
{
Expand Down
11 changes: 10 additions & 1 deletion Src/Particle/AMReX_ParticleTile.H
Original file line number Diff line number Diff line change
Expand Up @@ -634,6 +634,12 @@ struct ConstParticleTileData
}
};

struct ThisParticleTileHasNoParticleVector {};

struct ThisParticleTileHasNoAoS {
using ParticleVector = ThisParticleTileHasNoParticleVector;
};

template <typename T_ParticleType, int NArrayReal, int NArrayInt,
template<class> class Allocator=DefaultAllocator>
struct ParticleTile
Expand All @@ -651,7 +657,10 @@ struct ParticleTile

using SuperParticleType = Particle<NStructReal + NArrayReal, NStructInt + NArrayInt>;

using AoS = ArrayOfStructs<ParticleType, Allocator>;
using AoS = typename std::conditional<
ParticleType::is_soa_particle,
ThisParticleTileHasNoAoS,
ArrayOfStructs<ParticleType, Allocator>>::type;
//using ParticleVector = typename AoS::ParticleVector;

using SoA = StructOfArrays<NArrayReal, NArrayInt, Allocator>;
Expand Down
2 changes: 1 addition & 1 deletion Tests/Particles/RedistributeSOA/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ class TestParticleContainer
}

auto& particle_tile = DefineAndReturnParticleTile(lev, mfi.index(), mfi.LocalTileIndex());
auto old_size = particle_tile.GetArrayOfStructs().size();
auto old_size = particle_tile.size();
auto new_size = old_size + host_real[0].size();
particle_tile.resize(new_size);

Expand Down
2 changes: 1 addition & 1 deletion Tests/Particles/SENSEI_Insitu_SOA/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ class TestParticleContainer
}

auto& particle_tile = DefineAndReturnParticleTile(lev, mfi.index(), mfi.LocalTileIndex());
auto old_size = particle_tile.GetArrayOfStructs().size();
auto old_size = particle_tile.size();
auto new_size = old_size + host_real[0].size();
particle_tile.resize(new_size);

Expand Down

0 comments on commit 2e99628

Please sign in to comment.