Ascent: SoA Particle Support#3350
Conversation
|
@atmyers @c-wetterer-nelson @cyrush @andrewcombs let's coordinate how we finalize this - this is the last element blocking the AMReX SoA transition for WarpX on our agenda 🥁 |
RE: the catalyst 2 support PR. Yesterday was Andrew's last day so I will take over. The last piece there is a GitHub action script to add catalyst build testing to the CI. We could do that in a separate PR if you're comfortable with that. I am hoping to find time next week to make that happen. Otherwise, we are ready to review that PR. |
| { | ||
| int num_particles = ptile.GetArrayOfStructs().size(); | ||
| int struct_size = sizeof(Particle<NStructReal, NStructInt>); | ||
| int struct_size = ParticleType::is_soa_particle ? 0 : sizeof(ParticleType); |
There was a problem hiding this comment.
struct_size is used to represent element striding in the Conduit tree.
For the SOA case - I expect we want native striding (for example, 8 bytes for float64), so I think we need to update the Particle Container to Conduit wrappers to handle these cases.
There was a problem hiding this comment.
Thank you Cyrus - where is that wrapper located? I thought it was this file...
| 0, | ||
| struct_size); | ||
| #endif | ||
| } |
There was a problem hiding this comment.
Yes, this is the style of logic we need.
If AOS, we stride by the struct size. If SOA, we stride by the native stride (set_external w/o the stride argument accomplishes this)
| // struct real fields, the first set are always the particle positions | ||
| // which we wrap above | ||
| for (int i = 0; i < NStructReal; i++) | ||
| if constexpr(!ParticleType::is_soa_particle) |
There was a problem hiding this comment.
we need to build the fields for the SOA case, not sure how to access from AMReX but it should be a an else case.
|
Restarting CI. From local tests, I think CI does not catch that his PR still has a compile error: #3639 |
|
Based on |
Add support for pure SoA layouted particle containers for Ascent.
Co-authored-by: Andrew Myers <[email protected]>
4992cfe to
b178ad9
Compare
|
I think our |
## Summary For some reason, `AMReX_ASCENT` does not trigger `AMReX_CONDUIT` to be set, too. cc @cyrush ## Additional background X-ref: AMReX-Codes#3350
## Summary Add support for pure SoA layouted particle containers for Ascent. ## Additional background Follow-up to AMReX-Codes#2878. ## 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 --------- Co-authored-by: Andrew Myers <[email protected]>
Summary
Add support for pure SoA layouted particle containers for Ascent.
Additional background
Follow-up to #2878.
Checklist
The proposed changes: