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

Remove more global variables #4783

Merged
merged 7 commits into from
Oct 17, 2023
Merged

Remove more global variables #4783

merged 7 commits into from
Oct 17, 2023

Conversation

jngrad
Copy link
Member

@jngrad jngrad commented Sep 5, 2023

Partial fix for #2628

Description of changes:

  • encapsulate cell structure, box geometry and local box in the System class
  • gather MPI-related global variables in a global struct
  • rewrite position folding/unfolding free functions as member functions of BoxGeometry

I took this opportunity to cleanup and modernize code, by e.g. replacing hard-coded loops by Utils::Vector operations, improving type safety (no mixed signed and unsigned int in arithmetic expression), using more expressive variable names, passing global variables by function argument, adding a collision detection test case, etc.

Make particle coupling code more readable. Simplify ELC kernels.
Remove template parameter from LocalBox class.
@jngrad jngrad force-pushed the cellsystem branch 4 times, most recently from d5e6ccf to 8e10e30 Compare September 5, 2023 13:49
@RudolfWeeber
Copy link
Contributor

Offline discussion: It would be preferrable not to have the BoxGeometyr passed into angel bonds. This is currenlty the case, because they are called with acutal particle positoins rather than with distance vectors.
To separate concerns, minimum image handling should be outsie the bond kernel and only distance vectors should be passed in.

@jngrad jngrad added the automerge Merge with kodiak label Oct 17, 2023
@kodiakhq kodiakhq bot merged commit fdadb98 into espressomd:python Oct 17, 2023
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants