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

Integrator maintenance #4026

Merged
merged 16 commits into from
Dec 11, 2020
Merged

Integrator maintenance #4026

merged 16 commits into from
Dec 11, 2020

Conversation

jngrad
Copy link
Member

@jngrad jngrad commented Dec 4, 2020 •
edited
Loading

Description of changes:

  • bugfix: block combination NpT + P3M GPU (no P3M energies = undefined virial)
  • separation of concerns: significantly reduce indirect inclusion of integrate.hpp and npt.hpp
  • ifdef guard everything related to NpT
  • remove several FIELD_NPTISO_* enum values and handle NpT communication in npt.cpp instead of global.cpp
  • replace handle_errors()-based exception mechanism by throw std::runtime_error()
  • move NpT and steepest descent initializing logic from integrate.cpp and pressure.cpp to the relevant files npt.cpp and steepest_descent.cpp
  • make NpT and steepest descent initializing logic failsafe: setting either integrator with invalid parameters will throw a runtime error but won't alter the state of the system

jngrad added 10 commits December 4, 2020 00:03
@jngrad
tests: Cleanup NpT testcase
79f0ccf
@jngrad
cython: Use standard exception mechanism
411875a 
Replace the `runtimeErrorMsg()` logger by `throw std::runtime_error`.
This avoids the situation where the error code is no checked, e.g.
in `int integrate_set_sd()`, which was declared in the Cython code
as `void integrate_set_sd()` without generating a compiler warning.
Add python tests to check the new exception mechanism.
@jngrad
core: Feature guard NpT global variables
548b7e2
@jngrad
core: Move integrators init to relevant files
0c7e9c8 
Move the struct initialization logic and exception mechanism to the
relevant source files.
@jngrad
core: Remove FIELD_NPTISO_PISTON communication
e043305
@jngrad
core: Remove unused FIELD_NPTISO_* enum values
bd98613
@jngrad
core: Fix doxygen
0500410
@jngrad
core: Make NpT and Steepest Descent init failsafe
8977e50 
Setting the NpT and Steepest Descent integrators with incorrect
parameters will raise a RuntimeError but won't leave the system
in an undefined state (the old integrator remains untouched).
@jngrad
core: Simplify NpT, narrow integrate.hpp includes
54f43c2
@jngrad
core: Prevent combination NpT + P3M GPU
4d5621e 
P3M GPU doesn't calculate energies, therefore the NpT virial
cannot be calculated. This combination is now tested.
@jngrad jngrad added this to the Espresso 4.2 milestone Dec 4, 2020
@jngrad jngrad requested a review from RudolfWeeber December 4, 2020 16:22
jngrad added 5 commits December 4, 2020 19:37
@jngrad
tests: Fix regression
e519200 
The test is extremely sensitive to initial conditions and will fail
on 3 MPI ranks without this truncated value for the LJ cutoff. NpT
gives different trajectories depending on the number of MPI ranks.
@jngrad
core: Remove redundant feature guards
deb9b04
@jngrad
core: Narrow NpT includes
1cc0703 
Separation of concerns: npt.hpp is now directly and indirectly
included in 9 files instead of 23 files.
@jngrad
core: Simplify conditionals
0064e7d
@jngrad
core: Use boost::mpi instead of MPI primitives
6b2f51c
@RudolfWeeber RudolfWeeber added the automerge Merge with kodiak label Dec 11, 2020
@kodiakhq kodiakhq bot merged commit 2bd9e66 into espressomd:python Dec 11, 2020
@jngrad jngrad deleted the integrator-globals branch December 14, 2020 14:18
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
automerge Merge with kodiak BugFix CodeQuality Core Testcase
Projects
None yet
Milestone
Espresso 4.2.0
Development

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