Skip to content
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

cleaning and fixes for PureSoA ParticleTile #3327

Conversation

AlexanderSinn
Copy link
Member

@AlexanderSinn AlexanderSinn commented May 24, 2023

Summary

  • fix return by value/reference mismatch of id() and cpu() with decltype(auto)
  • remove dangerous functions data and dataPtr form ArrayOfStructs (they should have returned ParticleType* but returned the position array of the first particle)
  • update packParticleData and unpackParticleData for PureSoA
  • change getSuperParticle and add setSuperParticle for PureSoA. Remaining issue: super particle has redundant/unused fields for positions, id and cpu
  • update ConstParticleTileData for PureSoA
  • don’t call getParticleTileData in ParticleTile::id() etc. as this could be very slow if used in a for loop
  • use if constexpr instead if SFINAE in a number of places
  • change PODVector constructor so that getParticleTileData() won't allocate memory if no runtime components are used

Additional background

Follow-up to #2878.

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

@ax3l
Copy link
Member

ax3l commented May 24, 2023

Epic changes all along. Thanks a lot! 🚀 ✨


[[nodiscard]] bool empty () const { return m_data.empty(); }

[[nodiscard]] const RealType* data () const { return &(m_data[0].m_pos[0]); }
Copy link
Member

Choose a reason for hiding this comment

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

I agree that these are weird, but unfortunately they are needed for backwards compatibility with codes that still pass particle structs into Fortran subroutines.

Copy link
Member

Choose a reason for hiding this comment

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

Should we potentially make the void* pointers to reflect what they do?

(Can be a separate PR.)

Copy link
Member

Choose a reason for hiding this comment

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

Ah I see, changed to ParticleType* is also good.

@atmyers atmyers merged commit b533128 into AMReX-Codes:development May 24, 2023
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