Apply code review suggestions

Co-authored-by: Mariano Brito <[email protected]>

doc/sphinx/advanced_methods.rst

@@ -54,7 +54,7 @@ This protocol is set up with :class:`~espressomd.collision_detection.BindCenters
 The parameters are as follows:
 
 * ``distance`` is the distance between two particles at which the binding is triggered.
- This cutoff distance, ``1.5`` in the example above, is typically chosen slightly larger
+ This cutoff distance, ``0.1`` in the example above, is typically chosen slightly larger
  than the particle diameter. It is also a good choice for the equilibrium length of the bond.
 * ``bond_centers`` is the bonded interaction to be created between the particles
  (an instance of :class:`~espressomd.interactions.HarmonicBond` in the example above).
@@ -82,7 +82,7 @@ is added between the two colliding particles and the virtual particles.
 In the latter case, the virtual particles are the centers of the angle potentials
 (particle 2 in the description of the angle potential, see :ref:`Bond-angle interactions`).
 Due to the rigid connection between each of the colliding particles and their
-respective virtual sites, a sliding at the contact point is no longer possible.
+respective virtual sites, sliding at the contact point is no longer possible.
 See :ref:`Rigid arrangements of particles` for details. This protocol affects all particle types.
 
 In addition to the bond between the virtual sites, a bond between the colliding
@@ -105,7 +105,7 @@ This protocol is set up with :class:`~espressomd.collision_detection.BindAtPoint
  bond_centers=bond_centers,
  bond_vs=bond_vs,
  part_type_vs=1,
- vs_placement=0.2)
+ vs_placement=0.5)
 
 The parameters ``distance`` and ``bond_centers`` have the same meaning
 as in the :ref:`Bind centers` protocol. The remaining parameters are as follows:
@@ -135,6 +135,7 @@ Glue to surface
 ~~~~~~~~~~~~~~~
 
 Attach small particles to the surface of a large particle.
+The bond can be made irreversible.
 
 Several small particles can be bound to a large particle but not vice versa.
 The small particles can change type after collision to become *inert*.

src/core/collision_detection/BindAtPointOfCollision.hpp

@@ -36,18 +36,19 @@ namespace CollisionDetection {
 
 class BindAtPointOfCollision {
 public:
+ /// Distance at which particle are bound
  double distance;
- // Square of distance at which particle are bound
+ /// Square of distance at which particle are bound
  double distance_sq;
-
  /// bond type used between centers of colliding particles
  int bond_centers;
  /// bond type used between virtual sites
  int bond_vs;
- /** Placement of virtual sites for MODE_VS.
- * 0=on same particle as related to,
- * 1=on collision partner,
- * 0.5=in the middle between
+ /**
+ * @brief Barycenter of the virtual site.
+ * 0=on same particle as related to,
+ * 1=on collision partner,
+ * 0.5=in the middle between the two particles
  */
  double vs_placement;
  /// particle type for virtual sites created on collision

src/core/collision_detection/BindCenters.hpp

@@ -35,10 +35,10 @@ namespace CollisionDetection {
 
 class BindCenters {
 public:
+ /// Distance at which particle are bound
  double distance;
- // Square of distance at which particle are bound
+ /// Square of distance at which particle are bound
  double distance_sq;
-
  /// bond type used between centers of colliding particles
  int bond_centers;
 

src/core/collision_detection/GlueToSurface.hpp

@@ -36,22 +36,21 @@ namespace CollisionDetection {
 
 class GlueToSurface {
 public:
+ /// Distance at which particle are bound
  double distance;
- // Square of distance at which particle are bound
+ /// Square of distance at which particle are bound
  double distance_sq;
  /// bond type used between centers of colliding particles
  int bond_centers;
  /// bond type used between virtual sites
  int bond_vs;
  /// particle type for virtual sites created on collision
  int part_type_vs;
- /// For mode "glue to surface": The distance from the particle which is to be
- /// glued to the new virtual site
+ /// Distance from the particle which is to be glued to the new virtual site
  double dist_glued_part_to_vs;
- /// For mode "glue to surface": The particle type being glued
+ /// The particle type being glued (the small particle)
  int part_type_to_be_glued;
- /// For mode "glue to surface": The particle type to which the virtual site is
- /// attached
+ /// The particle type to which virtuals site are attached (the large particle)
  int part_type_to_attach_vs_to;
  /// Particle type to which the newly glued particle is converted
  int part_type_after_glueing;
@@ -74,10 +73,10 @@ class GlueToSurface {
 
  /** @brief Check additional criteria for the glue_to_surface collision mode */
  bool glue_to_surface_criterion(Particle const &p1, Particle const &p2) const {
- return (((p1.type() == part_type_to_be_glued) and
- (p2.type() == part_type_to_attach_vs_to)) or
- ((p2.type() == part_type_to_be_glued) and
- (p1.type() == part_type_to_attach_vs_to)));
+ return ((p1.type() == part_type_to_be_glued and
+ p2.type() == part_type_to_attach_vs_to) or
+ (p2.type() == part_type_to_be_glued and
+ p1.type() == part_type_to_attach_vs_to));
  }
 
  void handle_collisions(System::System &system,

src/core/collision_detection/Off.hpp

@@ -29,7 +29,6 @@
 #include "nonbonded_interactions/nonbonded_interaction_data.hpp"
 #include "system/System.hpp"
 
-#include <utility>
 #include <vector>
 
 namespace CollisionDetection {
@@ -43,6 +42,7 @@ class Off {
  void initialize(System::System &) {}
 
  void handle_collisions(System::System &, std::vector<CollisionPair> &) {}
+
  bool detect_collision(Particle const &, Particle const &,
  double const) const {
  return false;

src/core/collision_detection/common.hpp

@@ -35,10 +35,8 @@ inline auto detect_collision_common(Particle const &p1, Particle const &p2,
  return false;
 
  // Check, if there's already a bond between the particles
- if (pair_bond_exists_on(p1.bonds(), p2.id(), bond_centers))
- return false;
-
- if (pair_bond_exists_on(p2.bonds(), p1.id(), bond_centers))
+ if (pair_bond_exists_on(p1.bonds(), p2.id(), bond_centers) or
+ pair_bond_exists_on(p2.bonds(), p1.id(), bond_centers))
  return false;
 
  /* If we're still here, there is no previous bond between the particles,

src/script_interface/collision_detection/Off.hpp

@@ -39,6 +39,7 @@ class Off : public Protocol {
  Off()
  : m_protocol{std::make_shared<::CollisionDetection::ActiveProtocol>(
  ::CollisionDetection::Off())} {}
+
  std::shared_ptr<::CollisionDetection::ActiveProtocol> protocol() override {
  return m_protocol;
  }

testsuite/python/save_checkpoint.py

@@ -327,7 +327,7 @@
 if espressomd.has_features("COLLISION_DETECTION"):
  if espressomd.has_features("VIRTUAL_SITES_RELATIVE"):
  protocol = espressomd.collision_detection.BindAtPointOfCollision(
- distance=0.11, bond_centers=harmonic_bond,
+ distance=0.12, bond_centers=harmonic_bond,
  bond_vs=strong_harmonic_bond, part_type_vs=2, vs_placement=1. / 3.)
  else:
  protocol = espressomd.collision_detection.BindCenters(

testsuite/python/test_checkpoint.py

@@ -928,17 +928,19 @@ def test_comfixed(self):
  @utx.skipIfMissingFeatures('COLLISION_DETECTION')
  def test_collision_detection(self):
  protocol = system.collision_detection.protocol
- self.assertAlmostEqual(protocol.distance, 0.11, delta=1E-9)
- self.assertEqual(protocol.bond_centers, system.bonded_inter[0])
  if espressomd.has_features("VIRTUAL_SITES_RELATIVE"):
  self.assertIsInstance(
  protocol, espressomd.collision_detection.BindAtPointOfCollision)
+ self.assertAlmostEqual(protocol.distance, 0.12, delta=1E-9)
+ self.assertEqual(protocol.bond_centers, system.bonded_inter[0])
  self.assertEqual(protocol.bond_vs, system.bonded_inter[1])
  self.assertEqual(protocol.part_type_vs, 2)
  self.assertAlmostEqual(protocol.vs_placement, 1. / 3., delta=1e-6)
  else:
  self.assertIsInstance(
  protocol, espressomd.collision_detection.BindCenters)
+ self.assertAlmostEqual(protocol.distance, 0.11, delta=1E-9)
+ self.assertEqual(protocol.bond_centers, system.bonded_inter[0])
 
  @utx.skipIfMissingFeatures('EXCLUSIONS')
  def test_exclusions(self):