-
Notifications
You must be signed in to change notification settings - Fork 20
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update Particle Container to Pure SoA #348
Conversation
38c7f86
to
c4268f4
Compare
cf12d2c
to
83f96ff
Compare
pxout = px + dpx; | ||
|
||
p.pos(RealAoS::y) = y; | ||
// yout = y; |
Check notice
Code scanning / CodeQL
Commented-out code Note
pxout = px + dpx; | ||
|
||
p.pos(RealAoS::y) = y; | ||
// yout = y; |
Check notice
Code scanning / CodeQL
Commented-out code Note
a469076
to
ba1d781
Compare
…oA (#3296) The implementation of `ParticleTile::empty()` was wrong for pure SoA, which led to problems in ImpactX: ECP-WarpX/impactx#348 The proposed changes: - [x] 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
99fb419
to
03d6b6c
Compare
@cemitch99 when you have time next week, can you please review the chances I did to individual elements? |
7d89ec1
to
51a7f03
Compare
This comment was marked as resolved.
This comment was marked as resolved.
fe812bd
to
7b85a26
Compare
This pattern of changes to individual elements looks correct, and I like the improved symmetry between the treatment of position and momentum variables. The main risk introduced is possible mixing of the initial values and final values, so I will need to check every element line-by-line before review. |
7b85a26
to
4772210
Compare
This comment was marked as resolved.
This comment was marked as resolved.
d158624
to
0e8b676
Compare
This comment was marked as resolved.
This comment was marked as resolved.
6968ba3
to
e954247
Compare
Transition particle containers to pure SoA layouts.
e954247
to
1829d1f
Compare
Congrats to all who helped @Thierry992, @atmyers, @AlexanderSinn et al. 🎉 👏 |
I repeated the benchmarks on GPU for scrutiny on Perlmutter A100 SMX 80 GB GPUs using the new AMReX AMReX-Codes/amrex#3763 There is now a bit more overhead added due to the excessive syncs, but individual elements should be more precisely measured. Benchmarks on NVIDIA A100-SXM 80GB GPU, SP
soa_gpu_sp.txt, old_gpu_sp.txt
Benchmarks on NVIDIA A100-SXM 80GB GPU, DP
soa_gpu_dp.txt, old_gpu_dp.txt
|
Transition particle containers to pure SoA layouts.
Redistribute
fix in AMReXParticleIDWrapper::make_invalid()
AMReX-Codes/amrex#3735Benchmarks on NVIDIA A100-SXM 80GB GPU, SP
soa_gpu_sp.txt, dev_gpu_sp.txt
impactx::Push::Drift
: 2.33x fasterimpactx::Push::Quad
: 2.85x fasterimpactX::collect_lost_particles
: 2.77x fasterImpactX::add_particles
: 5.4% fasterBenchmarks on NVIDIA A100-SXM 80GB GPU, DP
soa_gpu_dp.txt, dev_gpu_dp.txt
impactx::Push::Drift
: 1.85x fasterimpactx::Push::Quad
: 1.42x fasterimpactX::collect_lost_particles
: 3.68x fasterImpactX::add_particles
: 6.6% fasterBenchmarks on CPU, SP
Laptop: 12th Gen Intel(R) Core(TM) i9-12900H, performance core w/ performance power mode on
soa_cpu_sp.txt, dev_cpu_sp.txt
impactx::Push::Drift
: 2.17x fasterimpactx::Push::Quad
: 4.8% fasterimpactX::collect_lost_particles
: 2.48x fasterImpactX::add_particles
: 1.7% fasterBenchmarks on CPU, DP
Laptop: 12th Gen Intel(R) Core(TM) i9-12900H, performance core w/ performance power mode on
soa_cpu_dp.txt, dev_cpu_dp.txt
impactx::Push::Drift
: 1.76x fasterimpactx::Push::Quad
: same perf.impactX::collect_lost_particles
: 3.24x fasterImpactX::add_particles
: 2.2% faster