Skip to content

Add version of ParticleTile using runtime-only 2D array#4404

Open
AlexanderSinn wants to merge 58 commits intoAMReX-Codes:developmentfrom
AlexanderSinn:Add_simpler_version_of_ParticleTile_using_2D_array
Open

Add version of ParticleTile using runtime-only 2D array#4404
AlexanderSinn wants to merge 58 commits intoAMReX-Codes:developmentfrom
AlexanderSinn:Add_simpler_version_of_ParticleTile_using_2D_array

Conversation

@AlexanderSinn
Copy link
Copy Markdown
Member

@AlexanderSinn AlexanderSinn commented Apr 7, 2025

Summary

This PR adds a simpler version of ParticleTile #3570

The new tile only has runtime components and always has a polymorphic arena allocator as described in #4380. To avoid the array of pointer style of the current particle tile, here a 2D array over particle ids and component ids is used. As a simplification both AoS and SoA members have been removed as well as all the push_back functions.

Performance test with HiPACE++:

dev, ParticleContainerPureSoA<11, 1>

TinyProfiler total time across processes [min...avg...max]: 21.44 ... 21.44 ... 21.44
TinyProfiler total time across processes [min...avg...max]: 21.42 ... 21.42 ... 21.42
TinyProfiler total time across processes [min...avg...max]: 21.4 ... 21.4 ... 21.4
TinyProfiler total time across processes [min...avg...max]: 21.43 ... 21.43 ... 21.43
TinyProfiler total time across processes [min...avg...max]: 21.39 ... 21.39 ... 21.39

ParticleContainerPureSoA<0, 0> using only runtime components

TinyProfiler total time across processes [min...avg...max]: 21.93 ... 21.93 ... 21.93
TinyProfiler total time across processes [min...avg...max]: 22.05 ... 22.05 ... 22.05
TinyProfiler total time across processes [min...avg...max]: 21.95 ... 21.95 ... 21.95
TinyProfiler total time across processes [min...avg...max]: 21.96 ... 21.96 ... 21.96
TinyProfiler total time across processes [min...avg...max]: 21.94 ... 21.94 ... 21.94

this PR, ParticleContainerPureSoA2<amrex::Real, int>

TinyProfiler total time across processes [min...avg...max]: 21.4 ... 21.4 ... 21.4
TinyProfiler total time across processes [min...avg...max]: 21.38 ... 21.38 ... 21.38
TinyProfiler total time across processes [min...avg...max]: 21.4 ... 21.4 ... 21.4
TinyProfiler total time across processes [min...avg...max]: 21.44 ... 21.44 ... 21.44
TinyProfiler total time across processes [min...avg...max]: 21.38 ... 21.38 ... 21.38

Additional background

ParticleReal and int are replaced with template types to make implementing the const versions easier and to allow for more flexibility in the future (use double and float in the same application, use double2 or float4, use 64 bit int etc).

Checklist

The proposed changes:

  • 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

@atmyers
Copy link
Copy Markdown
Member

atmyers commented Jun 11, 2025

Hi @AlexanderSinn - is this still something you're working on? Do you think it would be worthwhile to keep pushing on this?

@AlexanderSinn
Copy link
Copy Markdown
Member Author

Yes. There is still a lot left to do. I am waiting for Axel to return and give feedback first

@ax3l ax3l requested review from atmyers and ax3l June 16, 2025 16:37
atmyers pushed a commit that referenced this pull request Aug 13, 2025
## Summary

This PR adds a `SetArena(Arena*)` function to ParticleContainerBase that
allows setting a memory arena that is used for all the particle vectors
if the allocator is PolymorphicArenaAllocator. The function has to be
called before particle tiles are defined.

This functionality is used to fix a bunch of places where a polymorphic
vector would previously not have its arena set properly.

Additionally the `RunOnGpu` logic in `AMReX_WriteBinaryParticleData.H`
is extended to work with a polymorphic allocator.

Uses changes extracted from
#4404.

## Additional background

Previously all components of all particle tiles needed their arena set
individually by the user if PolymorphicArenaAllocator was used. In case
this was done this PR is a braking change due to the
`AMREX_ALWAYS_ASSERT_WITH_MESSAGE(a_arena != nullptr` assert in
`ParticleTile::define()`.

## Checklist

The proposed changes:
- [ ] fix a bug or incorrect behavior in AMReX
- [x] 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
@ax3l
Copy link
Copy Markdown
Member

ax3l commented Nov 10, 2025

@atmyers can you pls prioritize this PR in the coming week(s)? :) 🙏

@AlexanderSinn
Copy link
Copy Markdown
Member Author

This PR depends on #4529 btw

atmyers pushed a commit that referenced this pull request Mar 13, 2026
## Summary

This PR simplifies RedistributeCPU to be independent of particle layout
in preparation for #4404.
For this, push_back is replaced by a resize with a geometric growth
strategy. push_back is error-prone due to having the possibility to
desynchronize the sizes of the individual component vectors if used
incorrectly.

## Additional background

## Checklist

The proposed changes:
- [ ] 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
@AlexanderSinn AlexanderSinn changed the title [WIP] Add simpler version of ParticleTile using 2D array Add version of ParticleTile using runtime-only 2D array Mar 16, 2026
@AlexanderSinn
Copy link
Copy Markdown
Member Author

I think now this is ready for review. It does lack a bit in documentation and is not compatible with everything yet, but the same is true for regular PureSoA.

Comment on lines +93 to +97
[[nodiscard]] auto numParticles () const { return GetParticleTile().numParticles(); }

[[nodiscard]] int numRealParticles () const { return GetParticleTile().numRealParticles(); }
[[nodiscard]] auto numRealParticles () const { return GetParticleTile().numRealParticles(); }

[[nodiscard]] int numNeighborParticles () const { return GetParticleTile().numNeighborParticles(); }
[[nodiscard]] auto numNeighborParticles () const { return GetParticleTile().numNeighborParticles(); }
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm concerned that changing this to an unsigned type will lead to a bunch of mismatched comparisons in downstream code if we adopt this in e.g. WarpX.

};

template <class T>
ArrayView(T* data, std::size_t capacity) -> ArrayView<T>;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this reuse ArrayND?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I switched the index type to amrex::Long now, lets see if that works.

ArrayView is needed for code that expects a PODVector to still work if operator[], data(), dataPtr(), begin() and end() are used. ArrayND<1> does not have all of those functions and could have overflow issues as it uses int as an index type. 

@atmyers
Copy link
Copy Markdown
Member

atmyers commented Mar 25, 2026

Maybe you should also add an assertion to Redistribute that at least AMReX_SPACEDIM runtime real components have been added, since that function does assume they are there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants