Allow to calc energy contrib for single bond #5040
Conversation
| assert(1 == boost::mpi::all_reduce(comm, static_cast<int>(!!result), | ||
| std::plus<>()) && | ||
| "Incorrect number of return values"); | ||
| if (boost::mpi::all_reduce(comm, static_cast<int>(!!result), | ||
| std::plus<>())!=1) { | ||
| throw std::runtime_error("Internal error. Incorrect number of return values when collecting values from MPI ranks"); | ||
| } |
There was a problem hiding this comment.
Are you really sure about this change? We use assertions in almost all MPI reductions so that we get code coverage for the assertion. By turning it into an exception, we get zero code coverage for the body of the conditional, because it's impossible for the testsuite to enter it when there is no bug.
I'm always developing code with -D CMAKE_BUILD_TYPE=RelWithAssert builds, which is almost as fast as Release builds except for a few momentum-conservation tests, and has the advantage of enabling all assertions. By selectively turning a few critical assertions into exceptions, we are decreasing incentives for other ESPResSo developers to adopt RelWithAssert.
There was a problem hiding this comment.
Looks like my answer via email ditn't work. I reverted the change in reuduce_optoinal()
via
system.analysis.particle_bond_energy(particle,bond)This also
particle.bondsUtils::MPI::reduce_optional()to an exception. This should never happen to end users in any case, but is easier on developers who trigger it...