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

Refactor globals.cpp:bcast_parameter can of worms...globals #4133

Closed
RudolfWeeber opened this issue Mar 4, 2021 · 2 comments · Fixed by #4261
Closed

Refactor globals.cpp:bcast_parameter can of worms...globals #4133

RudolfWeeber opened this issue Mar 4, 2021 · 2 comments · Fixed by #4261

Comments

@RudolfWeeber
Copy link
Contributor

This relies on gloals.cpp::fields which maps flags to memory localtion of globals.
Ghey are then sent via mpi without serialization.
Afterwards some logic in events.cpp is used to do follow-up changes that aris from the parameter broadcast.

All of this needs to be replaced:

  • For each of the objects (box_l, periodicity, skin, ...) an mpi callback pair needs tobe created that does the data transfer and handles the follow up logic on the receiving end.

I stumbled up on this, because I wanted to make m_length and m_periodic in BoxGeometry private (as they should be). This would have allowed to pre-cauclate half_box_l and box_l_inv for more efficient get_mi_vector in bond-heavy and Lees-Edwards systems.

@KaiSzuttor
Copy link
Member

In general, we want to get rid of all of those globals in the long term, right?

@RudolfWeeber
Copy link
Contributor Author

RudolfWeeber commented Mar 5, 2021 via email

@kodiakhq kodiakhq bot closed this as completed in #4261 May 30, 2021
kodiakhq bot added a commit that referenced this issue May 30, 2021
Fixes #4133

- created MPI-Callbacks for each variable in the old `global.hpp:field`
- removed `globals.hpp` file
- removed the `kT=None, gamma=None` default logic in thermostats.pyx
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants