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.

Sign up for GitHub

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

Jump to bottom

Encapsulate long-range actors #4749

Merged
merged 5 commits into from
Jul 13, 2023
Merged

Encapsulate long-range actors #4749

merged 5 commits into from
Jul 13, 2023

Conversation

jngrad
Copy link
Member

@jngrad jngrad commented Jul 6, 2023 •
edited
Loading

Description of changes:

  • make electrostatics a member of the System class
  • make magnetostatics a member of the System class
  • remove 5 global variables (partial fix for Remove global variables #2628)
  • API change: new syntax for long-range actors
system.electrostatics.solver = espressomd.electrostatics.P3M(...)
system.electrostatics.extension = espressomd.electrostatic_extensions.ICC(...)
system.magnetostatics.solver = espressomd.magnetostatics.DipolarDirectSumCpu(...)

system.electrostatics.clear()
system.magnetostatics.clear()

jngrad added 2 commits July 4, 2023 17:59
@jngrad
core: Encapsulate electrostatics globals
03f0ac0 
The electrostatics solver is now a member of the System class.
@jngrad
core: Encapsulate magnetostatics globals
e4d68bc 
The magnetostatics solver is now a member of the System class.
@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

@jngrad jngrad force-pushed the actors branch 2 times, most recently from 2673c5c to 48483d6 Compare July 8, 2023 17:15
jngrad added 2 commits July 10, 2023 18:52
@jngrad
core: Use pointer-to-implementation
6c293f7 
Separation of concerns: hide implementation details of long-range
methods from the global System class.
@jngrad
script_interface: Add automatic feature checks
61ea4e4
@jngrad jngrad marked this pull request as ready for review July 10, 2023 18:46
@jngrad jngrad added this to the ESPResSo 4.3.0 milestone Jul 10, 2023
@jngrad jngrad requested a review from RudolfWeeber July 10, 2023 18:47
@RudolfWeeber
Copy link
Contributor

LGTM. It might be helpful to explicitly mention the pointer-to-impl pattern and the visitor pattern at a few strategic places, so developers who have not seen this patterns know what to google for.

@jngrad jngrad added the automerge Merge with kodiak label Jul 13, 2023
@kodiakhq kodiakhq bot merged commit 3fd0a82 into espressomd:python Jul 13, 2023
@jngrad jngrad deleted the actors branch July 13, 2023 14:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RudolfWeeber RudolfWeeber RudolfWeeber approved these changes

Assignees
No one assigned
Labels
ApiChange automerge Merge with kodiak CodeQuality Core ScriptInterface
Projects
None yet
Milestone
ESPResSo 4.3.0
Development

Successfully merging this pull request may close these issues.
2 participants
@jngrad @RudolfWeeber