espressomd / espresso

The ESPResSo package
https://espressomd.org
GNU General Public License v3.0
226 stars 183 forks source link

Unneeded runtime error collection in the integration loop #1290

Closed RudolfWeeber closed 6 years ago

RudolfWeeber commented 7 years ago

Collection includes an all_reduce which, in effect acts as barrier. Several algorithms call this during the integration loop without checking whether they are on:

There is one non-ifdef-ed check_runtime_errors() somewhere in the middle of the integration loop. Maybe, that's the only one which should stay. It might however be moved to the point at which the announce_resort_particles happens, which also is an effective barrier.

#ifdef NPT
    if (check_runtime_errors())
      break;
#endif

#ifdef VIRTUAL_SITES
    ghost_communicator(&cell_structure.update_ghost_pos_comm);
    update_mol_vel();
    if (check_runtime_errors())
      break;
#endif

#ifdef LB
    if (lattice_switch & LATTICE_LB)
      lattice_boltzmann_update();

    if (check_runtime_errors())
      break;
#endif

#ifdef IMMERSED_BOUNDARY

    IBM_UpdateParticlePositions(local_cells.particles());
// We reset all since otherwise the halo nodes may not be reset
// NB: the normal Espresso reset is also done after applying the forces
//    if (lattice_switch & LATTICE_LB) IBM_ResetLBForces_CPU();
#ifdef LB_GPU
// if (lattice_switch & LATTICE_LB_GPU) IBM_ResetLBForces_GPU();
#endif

    if (check_runtime_errors())
      break;

    // Ghost positions are now out-of-date
    // We should update.
    // Actually we seem to get the same results whether we do this here or not,
    // but it is safer to do it
    ghost_communicator(&cell_structure.update_ghost_pos_comm);

#endif // IMMERSED_BOUNDARY

Also an unconditional ghost comm there.

fweik commented 6 years ago

@RudolfWeeber , was this fixed by #1587?

RudolfWeeber commented 6 years ago

On Thu, Nov 02, 2017 at 04:58:12PM +0000, Florian Weik wrote:

@RudolfWeeber , was this fixed by #1587? Yes, except the one for IMMERSED_BOUNDARIES which I left alone (@sgekle) The runtime error collection there can probably also be removed, but it only becomes relevant when virtual sites are switchable at runtime and IMMERSED_BOUNDARIES can be included in the maxset configuration.