Extend Force sub-counters
authorMark Abraham <mark.j.abraham@gmail.com>
Mon, 16 Feb 2015 18:50:30 +0000 (19:50 +0100)
committerMark Abraham <mark.j.abraham@gmail.com>
Thu, 19 Mar 2015 11:08:20 +0000 (12:08 +0100)
Need more data for understanding performance variation

Implemented subcounter "restart" and used it for accumulating
position-restraints time with FEP to the position-restraints
subcounter.

Noted TODOs for some future extensions not currently possible.

Also added logfile output from GMX_CYCLE_BARRIER where people
analyzing the performance will see it.

Refs #1686

Change-Id: I9d60d0a683f56549879bb739269e9466c96572c4

src/gromacs/listed-forces/listed-forces.cpp
src/gromacs/listed-forces/listed-forces.h
src/gromacs/listed-forces/position-restraints.cpp
src/gromacs/listed-forces/position-restraints.h
src/gromacs/mdlib/force.cpp
src/gromacs/mdlib/sim_util.cpp
src/gromacs/timing/wallcycle.c
src/gromacs/timing/wallcycle.h

index 4cb4e90ac098f720da30eb699aabfc7eddaccd44..f95db0b9c515605199007ac2d7aeb076e32d0853 100644 (file)
@@ -294,6 +294,10 @@ calc_one_bond(int thread,
     {
         if (ftype == F_CMAP)
         {
+            /* TODO The execution time for CMAP dihedrals might be
+               nice to account to its own subtimer, but first
+               wallcycle needs to be extended to support calling from
+               multiple threads. */
             v = cmap_dihs(nbn, iatoms+nb0,
                           idef->iparams, &idef->cmap_grid,
                           x, f, fshift,
@@ -361,6 +365,9 @@ calc_one_bond(int thread,
     }
     else
     {
+        /* TODO The execution time for pairs might be nice to account
+           to its own subtimer, but first wallcycle needs to be
+           extended to support calling from multiple threads. */
         v = do_pairs(ftype, nbn, iatoms+nb0, idef->iparams, x, f, fshift,
                      pbc, g, lambda, dvdl, md, fr, grpp, global_atom_index);
     }
@@ -385,6 +392,7 @@ ftype_is_bonded_potential(int ftype)
 }
 
 void calc_listed(const gmx_multisim_t *ms,
+                 gmx_wallcycle        *wcycle,
                  const t_idef *idef,
                  const rvec x[], history_t *hist,
                  rvec f[], t_forcerec *fr,
@@ -428,39 +436,55 @@ void calc_listed(const gmx_multisim_t *ms,
     }
 #endif
 
-    if (idef->il[F_POSRES].nr > 0)
-    {
-        posres_wrapper(nrnb, idef, pbc_full, x, enerd, lambda, fr);
-    }
+    if ((idef->il[F_POSRES].nr > 0) ||
+        (idef->il[F_FBPOSRES].nr > 0) ||
+        (idef->il[F_ORIRES].nr > 0) ||
+        (idef->il[F_DISRES].nr > 0))
+    {
+        /* TODO Use of restraints triggers further function calls
+           inside the loop over calc_one_bond(), but those are too
+           awkward to account to this subtimer properly in the present
+           code. We don't test / care much about performance with
+           restraints, anyway. */
+        wallcycle_sub_start(wcycle, ewcsRESTRAINTS);
+
+        if (idef->il[F_POSRES].nr > 0)
+        {
+            posres_wrapper(nrnb, idef, pbc_full, x, enerd, lambda, fr);
+        }
 
-    if (idef->il[F_FBPOSRES].nr > 0)
-    {
-        fbposres_wrapper(nrnb, idef, pbc_full, x, enerd, fr);
-    }
+        if (idef->il[F_FBPOSRES].nr > 0)
+        {
+            fbposres_wrapper(nrnb, idef, pbc_full, x, enerd, fr);
+        }
 
-    /* Do pre force calculation stuff which might require communication */
-    if (idef->il[F_ORIRES].nr)
-    {
-        enerd->term[F_ORIRESDEV] =
-            calc_orires_dev(ms, idef->il[F_ORIRES].nr,
-                            idef->il[F_ORIRES].iatoms,
-                            idef->iparams, md, x,
-                            pbc_null, fcd, hist);
-    }
-    if (idef->il[F_DISRES].nr)
-    {
-        calc_disres_R_6(idef->il[F_DISRES].nr,
-                        idef->il[F_DISRES].iatoms,
-                        idef->iparams, x, pbc_null,
-                        fcd, hist);
-#ifdef GMX_MPI
-        if (fcd->disres.nsystems > 1)
+        /* Do pre force calculation stuff which might require communication */
+        if (idef->il[F_ORIRES].nr > 0)
         {
-            gmx_sum_sim(2*fcd->disres.nres, fcd->disres.Rt_6, ms);
+            enerd->term[F_ORIRESDEV] =
+                calc_orires_dev(ms, idef->il[F_ORIRES].nr,
+                                idef->il[F_ORIRES].iatoms,
+                                idef->iparams, md, x,
+                                pbc_null, fcd, hist);
         }
+        if (idef->il[F_DISRES].nr)
+        {
+            calc_disres_R_6(idef->il[F_DISRES].nr,
+                            idef->il[F_DISRES].iatoms,
+                            idef->iparams, x, pbc_null,
+                            fcd, hist);
+#ifdef GMX_MPI
+            if (fcd->disres.nsystems > 1)
+            {
+                gmx_sum_sim(2*fcd->disres.nres, fcd->disres.Rt_6, ms);
+            }
 #endif
+        }
+
+        wallcycle_sub_stop(wcycle, ewcsRESTRAINTS);
     }
 
+    wallcycle_sub_start(wcycle, ewcsLISTED);
 #pragma omp parallel for num_threads(fr->nthreads) schedule(static)
     for (thread = 0; thread < fr->nthreads; thread++)
     {
@@ -504,15 +528,21 @@ void calc_listed(const gmx_multisim_t *ms,
             }
         }
     }
+    wallcycle_sub_stop(wcycle, ewcsLISTED);
+
     if (fr->nthreads > 1)
     {
+        wallcycle_sub_start(wcycle, ewcsLISTED_BUF_OPS);
         reduce_thread_forces(fr->natoms_force, f, fr->fshift,
                              enerd->term, &enerd->grpp, dvdl,
                              fr->nthreads, fr->f_t,
                              fr->red_nblock, 1<<fr->red_ashift,
                              bCalcEnerVir,
                              force_flags & GMX_FORCE_DHDL);
+        wallcycle_sub_stop(wcycle, ewcsLISTED_BUF_OPS);
     }
+
+    /* Remaining code does not have enough flops to bother counting */
     if (force_flags & GMX_FORCE_DHDL)
     {
         for (i = 0; i < efptNR; i++)
@@ -623,14 +653,13 @@ do_force_listed(gmx_wallcycle        *wcycle,
         return;
     }
 
-    wallcycle_sub_start(wcycle, ewcsLISTED);
-
     if ((idef->il[F_POSRES].nr > 0) ||
         (idef->il[F_FBPOSRES].nr > 0))
     {
+        /* Not enough flops to bother counting */
         set_pbc(&pbc_full, fr->ePBC, box);
     }
-    calc_listed(ms, idef, x, hist, f, fr, pbc, &pbc_full,
+    calc_listed(ms, wcycle, idef, x, hist, f, fr, pbc, &pbc_full,
                 graph, enerd, nrnb, lambda, md, fcd,
                 global_atom_index, flags);
 
@@ -639,10 +668,11 @@ do_force_listed(gmx_wallcycle        *wcycle,
      */
     if (fepvals->n_lambda > 0 && (flags & GMX_FORCE_DHDL))
     {
-        posres_wrapper_lambda(fepvals, idef, &pbc_full, x, enerd, lambda, fr);
+        posres_wrapper_lambda(wcycle, fepvals, idef, &pbc_full, x, enerd, lambda, fr);
 
         if (idef->ilsort != ilsortNO_FE)
         {
+            wallcycle_sub_start(wcycle, ewcsLISTED_FEP);
             if (idef->ilsort != ilsortFE_SORTED)
             {
                 gmx_incons("The bonded interactions are not sorted for free energy");
@@ -661,9 +691,8 @@ do_force_listed(gmx_wallcycle        *wcycle,
                 sum_epot(&(enerd->foreign_grpp), enerd->foreign_term);
                 enerd->enerpart_lambda[i] += enerd->foreign_term[F_EPOT];
             }
+            wallcycle_sub_stop(wcycle, ewcsLISTED_FEP);
         }
     }
     debug_gmx();
-
-    wallcycle_sub_stop(wcycle, ewcsLISTED);
 }
index 6bf953af6fb4dba8f09fee309e3683261b323a74..968e3745568fb6ab85e22469d6c39d32b6ad77d9 100644 (file)
@@ -86,6 +86,7 @@ ftype_is_bonded_potential(int ftype);
  * Note that pbc_full is used only for position restraints, and is
  * not initialized if there are none. */
 void calc_listed(const gmx_multisim_t *ms,
+                 struct gmx_wallcycle *wcycle,
                  const t_idef *idef,
                  const rvec x[], history_t *hist,
                  rvec f[], t_forcerec *fr,
index 7378b205fe9d2e2522dc9243e194cd50c5f35339..3a37dbdfa4a023b84a08f8effedb1d80ecb3c3b9 100644 (file)
@@ -1,7 +1,7 @@
 /*
  * This file is part of the GROMACS molecular simulation package.
  *
- * Copyright (c) 2014, by the GROMACS development team, led by
+ * Copyright (c) 2014,2015, by the GROMACS development team, led by
  * Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl,
  * and including many others, as listed in the AUTHORS file in the
  * top-level source directory and at http://www.gromacs.org.
 #include "gromacs/legacyheaders/nrnb.h"
 #include "gromacs/math/vec.h"
 #include "gromacs/pbcutil/pbc.h"
+#include "gromacs/timing/wallcycle.h"
 #include "gromacs/topology/idef.h"
 #include "gromacs/utility/basedefinitions.h"
 
+struct gmx_wallcycle;
+
 namespace
 {
 
@@ -402,13 +405,14 @@ posres_wrapper(t_nrnb             *nrnb,
 }
 
 void
-posres_wrapper_lambda(const t_lambda     *fepvals,
-                      const t_idef       *idef,
-                      const struct t_pbc *pbc,
-                      const rvec          x[],
-                      gmx_enerdata_t     *enerd,
-                      real               *lambda,
-                      t_forcerec         *fr)
+posres_wrapper_lambda(struct gmx_wallcycle *wcycle,
+                      const t_lambda       *fepvals,
+                      const t_idef         *idef,
+                      const struct t_pbc   *pbc,
+                      const rvec            x[],
+                      gmx_enerdata_t       *enerd,
+                      real                 *lambda,
+                      t_forcerec           *fr)
 {
     real  v;
     int   i;
@@ -418,6 +422,7 @@ posres_wrapper_lambda(const t_lambda     *fepvals,
         return;
     }
 
+    wallcycle_sub_start_nocount(wcycle, ewcsRESTRAINTS);
     for (i = 0; i < enerd->n_lambda; i++)
     {
         real dvdl_dum = 0, lambda_dum;
@@ -430,6 +435,7 @@ posres_wrapper_lambda(const t_lambda     *fepvals,
                             fr->rc_scaling, fr->ePBC, fr->posres_com, fr->posres_comB);
         enerd->enerpart_lambda[i] += v;
     }
+    wallcycle_sub_stop(wcycle, ewcsRESTRAINTS);
 }
 
 /*! \brief Helper function that wraps calls to fbposres for
index fea4890ca909a360f7d8fe7b7259cfca8aed3c0c..6d79b2ae7bceb9447c9d5c867207a42c0bf57951 100644 (file)
@@ -57,6 +57,7 @@ extern "C" {
 #endif
 
 struct t_pbc;
+struct gmx_wallcycle;
 
 /*! \brief Helper function that wraps calls to posres */
 void
@@ -71,13 +72,14 @@ posres_wrapper(t_nrnb             *nrnb,
 /*! \brief Helper function that wraps calls to posres for free-energy
     pertubation */
 void
-posres_wrapper_lambda(const t_lambda     *fepvals,
-                      const t_idef       *idef,
-                      const struct t_pbc *pbc,
-                      const rvec          x[],
-                      gmx_enerdata_t     *enerd,
-                      real               *lambda,
-                      t_forcerec         *fr);
+posres_wrapper_lambda(struct gmx_wallcycle *wcycle,
+                      const t_lambda       *fepvals,
+                      const t_idef         *idef,
+                      const struct t_pbc   *pbc,
+                      const rvec            x[],
+                      gmx_enerdata_t       *enerd,
+                      real                 *lambda,
+                      t_forcerec           *fr);
 
 /*! \brief Helper function that wraps calls to fbposres for
     free-energy perturbation */
index e9e80dd12aada29d9089b40ef7b8b0bb70138b5e..d11bf98797cb09c62decffaa535145f606e9740d 100644 (file)
@@ -3,7 +3,7 @@
  *
  * Copyright (c) 1991-2000, University of Groningen, The Netherlands.
  * Copyright (c) 2001-2004, The GROMACS development team.
- * Copyright (c) 2013,2014, by the GROMACS development team, led by
+ * Copyright (c) 2013,2014,2015, by the GROMACS development team, led by
  * Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl,
  * and including many others, as listed in the AUTHORS file in the
  * top-level source directory and at http://www.gromacs.org.
@@ -347,8 +347,14 @@ void do_force_lowlevel(t_forcerec *fr,      t_inputrec *ir,
     /* Shift the coordinates. Must be done before listed forces and PPPM,
      * but is also necessary for SHAKE and update, therefore it can NOT
      * go when no listed forces have to be evaluated.
+     *
+     * The shifting and PBC code is deliberately not timed, since with
+     * the Verlet scheme it only takes non-zero time with triclinic
+     * boxes, and even then the time is around a factor of 100 less
+     * than the next smallest counter.
      */
 
+
     /* Here sometimes we would not need to shift with NBFonly,
      * but we do so anyhow for consistency of the returned coordinates.
      */
@@ -369,6 +375,9 @@ void do_force_lowlevel(t_forcerec *fr,      t_inputrec *ir,
         ((flags & GMX_FORCE_LISTED)
          || EEL_RF(fr->eeltype) || EEL_FULL(fr->eeltype) || EVDW_PME(fr->vdwtype)))
     {
+        /* TODO There are no electrostatics methods that require this
+           transformation, when using the Verlet scheme, so update the
+           above conditional. */
         /* Since all atoms are in the rectangular or triclinic unit-cell,
          * only single box vector shifts (2 in x) are required.
          */
@@ -497,6 +506,8 @@ void do_force_lowlevel(t_forcerec *fr,      t_inputrec *ir,
 
             if (EEL_PME_EWALD(fr->eeltype) && fr->n_tpi == 0)
             {
+                /* This is not in a subcounter because it takes a
+                   negligible and constant-sized amount of time */
                 Vcorr_q += ewald_charge_correction(cr, fr, lambda[efptCOUL], box,
                                                    &dvdl_long_range_correction_q,
                                                    fr->vir_el_recip);
index 38d043e28ff3f57135adca0543b34d3d8e7a1a54..b589455c64470774844c55f11b2699933d49683e 100644 (file)
@@ -1111,7 +1111,7 @@ void do_force_cutsVERLET(FILE *fplog, t_commrec *cr,
     }
 
     /* Start the force cycle counter.
-     * This counter is stopped in do_forcelow_level.
+     * This counter is stopped after do_force_lowlevel.
      * No parallel communication should occur while this counter is running,
      * since that will interfere with the dynamic load balancing.
      */
@@ -1218,7 +1218,7 @@ void do_force_cutsVERLET(FILE *fplog, t_commrec *cr,
         }
 
         /* Add all the non-bonded force to the normal force array.
-         * This can be split into a local a non-local part when overlapping
+         * This can be split into a local and a non-local part when overlapping
          * communication with calculation with domain decomposition.
          */
         cycles_force += wallcycle_stop(wcycle, ewcFORCE);
@@ -1233,6 +1233,8 @@ void do_force_cutsVERLET(FILE *fplog, t_commrec *cr,
         if ((flags & GMX_FORCE_VIRIAL) &&
             nbv->grp[aloc].nbl_lists.nnbl > 1)
         {
+            /* This is not in a subcounter because it takes a
+               negligible and constant-sized amount of time */
             nbnxn_atomdata_add_nbat_fshift_to_fshift(nbv->grp[aloc].nbat,
                                                      fr->fshift);
         }
@@ -1756,7 +1758,7 @@ void do_force_cutsGROUP(FILE *fplog, t_commrec *cr,
     }
 
     /* Start the force cycle counter.
-     * This counter is stopped in do_forcelow_level.
+     * This counter is stopped after do_force_lowlevel.
      * No parallel communication should occur while this counter is running,
      * since that will interfere with the dynamic load balancing.
      */
index f11272ba4a1dff42dcb411b6506dcff1ebc5c9d4..81f58b7aeccb2c5ecb375981ac6c7f0ece9ebf6a 100644 (file)
@@ -113,8 +113,14 @@ static const char *wcsn[ewcsNR] =
     "DD redist.", "DD NS grid + sort", "DD setup comm.",
     "DD make top.", "DD make constr.", "DD top. other",
     "NS grid local", "NS grid non-loc.", "NS search local", "NS search non-loc.",
-    "Bonded F", "Nonbonded F", "Ewald F correction",
-    "NB X buffer ops.", "NB F buffer ops."
+    "Bonded F",
+    "Bonded-FEP F",
+    "Restraints F",
+    "Listed buffer ops.",
+    "Nonbonded F",
+    "Ewald F correction",
+    "NB X buffer ops.",
+    "NB F buffer ops.",
 };
 
 gmx_bool wallcycle_have_counter(void)
@@ -867,6 +873,13 @@ void wallcycle_print(FILE *fplog, int nnodes, int npme, double realtime,
         }
     }
 
+    if (wc->wc_barrier)
+    {
+        md_print_warn(NULL, fplog,
+                      "MPI_Barrier was called before each cycle start/stop\n"
+                      "call, so timings are not those of real runs.\n");
+    }
+
     if (wc->wcc[ewcNB_XF_BUF_OPS].n > 0 &&
         (cyc_sum[ewcDOMDEC] > tot*0.1 ||
          cyc_sum[ewcNS] > tot*0.1))
@@ -930,6 +943,17 @@ void wallcycle_sub_start(gmx_wallcycle_t wc, int ewcs)
     }
 }
 
+void wallcycle_sub_start_nocount(gmx_wallcycle_t wc, int ewcs)
+{
+    if (wc == NULL)
+    {
+        return;
+    }
+
+    wallcycle_sub_start(wc, ewcs);
+    wc->wcsc[ewcs].n--;
+}
+
 void wallcycle_sub_stop(gmx_wallcycle_t wc, int ewcs)
 {
     if (wc != NULL)
@@ -944,6 +968,9 @@ void wallcycle_sub_stop(gmx_wallcycle_t wc, int ewcs)
 void wallcycle_sub_start(gmx_wallcycle_t gmx_unused wc, int gmx_unused ewcs)
 {
 }
+void wallcycle_sub_start_nocount(gmx_wallcycle_t gmx_unused wc, int gmx_unused ewcs)
+{
+}
 void wallcycle_sub_stop(gmx_wallcycle_t gmx_unused wc, int gmx_unused ewcs)
 {
 }
index 69c92cde722ddb801b16c4bf8114515d8792bf46..d0295b67dd8f6288a933f6bcd0d94ce0513bd3c9 100644 (file)
@@ -3,7 +3,7 @@
  *
  * Copyright (c) 1991-2000, University of Groningen, The Netherlands.
  * Copyright (c) 2001-2008, The GROMACS development team.
- * Copyright (c) 2013,2014, by the GROMACS development team, led by
+ * Copyright (c) 2013,2014,2015, by the GROMACS development team, led by
  * Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl,
  * and including many others, as listed in the AUTHORS file in the
  * top-level source directory and at http://www.gromacs.org.
@@ -37,6 +37,9 @@
 #ifndef GMX_TIMING_WALLCYCLE_H
 #define GMX_TIMING_WALLCYCLE_H
 
+/* NOTE: None of the routines here are safe to call within an OpenMP
+ * region */
+
 #include <stdio.h>
 
 #include "gromacs/legacyheaders/types/commrec_fwd.h"
@@ -66,8 +69,14 @@ enum {
     ewcsDD_MAKETOP, ewcsDD_MAKECONSTR, ewcsDD_TOPOTHER,
     ewcsNBS_GRID_LOCAL, ewcsNBS_GRID_NONLOCAL,
     ewcsNBS_SEARCH_LOCAL, ewcsNBS_SEARCH_NONLOCAL,
-    ewcsLISTED, ewcsNONBONDED, ewcsEWALD_CORRECTION,
-    ewcsNB_X_BUF_OPS, ewcsNB_F_BUF_OPS,
+    ewcsLISTED,
+    ewcsLISTED_FEP,
+    ewcsRESTRAINTS,
+    ewcsLISTED_BUF_OPS,
+    ewcsNONBONDED,
+    ewcsEWALD_CORRECTION,
+    ewcsNB_X_BUF_OPS,
+    ewcsNB_F_BUF_OPS,
     ewcsNR
 };
 
@@ -108,6 +117,9 @@ void wcycle_set_reset_counters(gmx_wallcycle_t wc, gmx_int64_t reset_counters);
 void wallcycle_sub_start(gmx_wallcycle_t wc, int ewcs);
 /* Set the start sub cycle count for ewcs */
 
+void wallcycle_sub_start_nocount(gmx_wallcycle_t wc, int ewcs);
+/* Set the start sub cycle count for ewcs without increasing the call count */
+
 void wallcycle_sub_stop(gmx_wallcycle_t wc, int ewcs);
 /* Stop the sub cycle count for ewcs */