Convert remaining callers of do_force to C++
authorMark Abraham <mark.j.abraham@gmail.com>
Thu, 4 Sep 2014 21:39:15 +0000 (23:39 +0200)
committerMark Abraham <mark.j.abraham@gmail.com>
Thu, 27 Nov 2014 18:26:30 +0000 (19:26 +0100)
Major callers are converted in other patches.

Removed unused variables, used std::min/max, named constants, put
maximum length on sscanf floats.

Introduced an instance of bMaster. Initialized gstat to and later
checked for NULL before use. These help static analysis reason
correctly. Previous code functioned correctly in practice, but correct
analysis would require cross-function analysis that cr, when passed as
non-const, is not actually modified. Fixed a warning about stepsize
not being used in L-BFGS, and added more comments to this algorithm
while editing the code to help us understand what it does.
The predict_shells() function has been modified to use the
gmx_mtop_atomnr_to_atom() call for case 2; the previous version would
use an uninitialized pointer.

Fixes #1593.

Change-Id: I54f238cedc78aadb0dad080ea3b28f001dce8d94

src/gromacs/mdlib/minimize.cpp [moved from src/gromacs/mdlib/minimize.c with 92% similarity]
src/gromacs/mdlib/shellfc.cpp [moved from src/gromacs/mdlib/shellfc.c with 97% similarity]
src/gromacs/mdlib/tpi.cpp [moved from src/gromacs/mdlib/tpi.c with 99% similarity]

similarity index 92%
rename from src/gromacs/mdlib/minimize.c
rename to src/gromacs/mdlib/minimize.cpp
index 8c7f8181b606ad1fd82dffba68ccb4a7c4525677..bf9a6bd9109d4b758481d70f631d8e50ab5a66c1 100644 (file)
@@ -42,6 +42,8 @@
 #include <string.h>
 #include <time.h>
 
+#include <algorithm>
+
 #include "gromacs/domdec/domdec.h"
 #include "gromacs/ewald/pme.h"
 #include "gromacs/fileio/confio.h"
@@ -203,7 +205,7 @@ static void get_f_norm_max(t_commrec *cr,
                            real *fnorm, real *fmax, int *a_fmax)
 {
     double fnorm2, *sum;
-    real   fmax2, fmax2_0, fam;
+    real   fmax2, fam;
     int    la_max, a_max, start, end, i, m, gf;
 
     /* This routine finds the largest force and returns it.
@@ -212,7 +214,6 @@ static void get_f_norm_max(t_commrec *cr,
     fnorm2 = 0;
     fmax2  = 0;
     la_max = -1;
-    gf     = 0;
     start  = 0;
     end    = mdatoms->homenr;
     if (mdatoms->cFREEZE)
@@ -427,6 +428,10 @@ void init_em(FILE *fplog, const char *title,
     {
         *gstat = global_stat_init(ir);
     }
+    else
+    {
+        *gstat = NULL;
+    }
 
     *outf = init_mdoutf(fplog, nfile, fnm, 0, cr, ir, top_global, NULL, wcycle);
 
@@ -588,6 +593,7 @@ static void do_em_step(t_commrec *cr, t_inputrec *ir, t_mdatoms *md,
     x1 = s1->x;
     x2 = s2->x;
 
+    // cppcheck-suppress unreadVariable
     nthreads = gmx_omp_nthreads_get(emntUpdate);
 #pragma omp parallel num_threads(nthreads)
     {
@@ -971,8 +977,8 @@ double do_cg(FILE *fplog, t_commrec *cr,
     rvec             *f;
     gmx_global_stat_t gstat;
     t_graph          *graph;
-    rvec             *f_global, *p, *sf, *sfm;
-    double            gpa, gpb, gpc, tmp, sum[2], minstep;
+    rvec             *f_global, *p, *sf;
+    double            gpa, gpb, gpc, tmp, minstep;
     real              fnormn;
     real              stepsize;
     real              a, b, c, beta = 0.0;
@@ -986,7 +992,6 @@ double do_cg(FILE *fplog, t_commrec *cr,
     int               number_steps, neval = 0, nstcg = inputrec->nstcgsteep;
     gmx_mdoutf_t      outf;
     int               i, m, gf, step, nminstep;
-    real              terminate = 0;
 
     step = 0;
 
@@ -1022,7 +1027,7 @@ double do_cg(FILE *fplog, t_commrec *cr,
      */
     evaluate_energy(fplog, cr,
                     top_global, s_min, top,
-                    inputrec, nrnb, wcycle, gstat,
+                    inputrec, nrnb, wcycle, gstat ? gstat : NULL,
                     vsite, constr, fcd, graph, mdatoms, fr,
                     mu_tot, enerd, vir, pres, -1, TRUE);
     where();
@@ -1045,16 +1050,17 @@ double do_cg(FILE *fplog, t_commrec *cr,
 
     if (MASTER(cr))
     {
+        double sqrtNumAtoms = sqrt(static_cast<double>(state_global->natoms));
         fprintf(stderr, "   F-max             = %12.5e on atom %d\n",
                 s_min->fmax, s_min->a_fmax+1);
         fprintf(stderr, "   F-Norm            = %12.5e\n",
-                s_min->fnorm/sqrt(state_global->natoms));
+                s_min->fnorm/sqrtNumAtoms);
         fprintf(stderr, "\n");
         /* and copy to the log file too... */
         fprintf(fplog, "   F-max             = %12.5e on atom %d\n",
                 s_min->fmax, s_min->a_fmax+1);
         fprintf(fplog, "   F-Norm            = %12.5e\n",
-                s_min->fnorm/sqrt(state_global->natoms));
+                s_min->fnorm/sqrtNumAtoms);
         fprintf(fplog, "\n");
     }
     /* Start the loop over CG steps.
@@ -1199,7 +1205,7 @@ double do_cg(FILE *fplog, t_commrec *cr,
         /* Calculate energy for the trial step */
         evaluate_energy(fplog, cr,
                         top_global, s_c, top,
-                        inputrec, nrnb, wcycle, gstat,
+                        inputrec, nrnb, wcycle, gstat ? gstat : NULL,
                         vsite, constr, fcd, graph, mdatoms, fr,
                         mu_tot, enerd, vir, pres, -1, FALSE);
 
@@ -1259,7 +1265,7 @@ double do_cg(FILE *fplog, t_commrec *cr,
          * the line minimum. We try to interpolate based on the derivative at the endpoints,
          * and only continue until we find a lower value. In most cases this means 1-2 iterations.
          *
-         * I also have a safeguard for potentially really patological functions so we never
+         * I also have a safeguard for potentially really pathological functions so we never
          * take more than 20 steps before we give up ...
          *
          * If we already found a lower value we just skip this step and continue to the update.
@@ -1307,7 +1313,7 @@ double do_cg(FILE *fplog, t_commrec *cr,
                 /* Calculate energy for the trial step */
                 evaluate_energy(fplog, cr,
                                 top_global, s_b, top,
-                                inputrec, nrnb, wcycle, gstat,
+                                inputrec, nrnb, wcycle, gstat ? gstat : NULL,
                                 vsite, constr, fcd, graph, mdatoms, fr,
                                 mu_tot, enerd, vir, pres, -1, FALSE);
 
@@ -1395,7 +1401,6 @@ double do_cg(FILE *fplog, t_commrec *cr,
                 }
                 swap_em_state(s_b, s_c);
                 gpb = gpc;
-                b   = c;
             }
             else
             {
@@ -1406,7 +1411,6 @@ double do_cg(FILE *fplog, t_commrec *cr,
                 }
                 swap_em_state(s_b, s_a);
                 gpb = gpa;
-                b   = a;
             }
 
         }
@@ -1419,7 +1423,6 @@ double do_cg(FILE *fplog, t_commrec *cr,
             }
             swap_em_state(s_b, s_c);
             gpb = gpc;
-            b   = c;
         }
 
         /* new search direction */
@@ -1455,8 +1458,9 @@ double do_cg(FILE *fplog, t_commrec *cr,
         {
             if (bVerbose)
             {
+                double sqrtNumAtoms = sqrt(static_cast<double>(state_global->natoms));
                 fprintf(stderr, "\rStep %d, Epot=%12.6e, Fnorm=%9.3e, Fmax=%9.3e (atom %d)\n",
-                        step, s_min->epot, s_min->fnorm/sqrt(state_global->natoms),
+                        step, s_min->epot, s_min->fnorm/sqrtNumAtoms,
                         s_min->fmax, s_min->a_fmax+1);
             }
             /* Store the new (lower) energies */
@@ -1549,10 +1553,11 @@ double do_cg(FILE *fplog, t_commrec *cr,
                   top_global, inputrec, step,
                   s_min, state_global, f_global);
 
-    fnormn = s_min->fnorm/sqrt(state_global->natoms);
 
     if (MASTER(cr))
     {
+        double sqrtNumAtoms = sqrt(static_cast<double>(state_global->natoms));
+        fnormn = s_min->fnorm/sqrtNumAtoms;
         print_converged(stderr, CG, inputrec->em_tol, step, converged, number_steps,
                         s_min->epot, s_min->fmax, s_min->a_fmax, fnormn);
         print_converged(fplog, CG, inputrec->em_tol, step, converged, number_steps,
@@ -1600,14 +1605,14 @@ double do_lbfgs(FILE *fplog, t_commrec *cr,
     t_graph           *graph;
     rvec              *f_global;
     int                ncorr, nmaxcorr, point, cp, neval, nminstep;
-    double             stepsize, gpa, gpb, gpc, tmp, minstep;
+    double             stepsize, step_taken, gpa, gpb, gpc, tmp, minstep;
     real              *rho, *alpha, *ff, *xx, *p, *s, *lastx, *lastf, **dx, **dg;
     real              *xa, *xb, *xc, *fa, *fb, *fc, *xtmp, *ftmp;
     real               a, b, c, maxdelta, delta;
     real               diag, Epot0, Epot, EpotA, EpotB, EpotC;
     real               dgdx, dgdg, sq, yr, beta;
     t_mdebin          *mdebin;
-    gmx_bool           converged, first;
+    gmx_bool           converged;
     rvec               mu_tot;
     real               fnorm, fmax;
     gmx_bool           do_log, do_ene, do_x, do_f, foundlower, *frozen;
@@ -1616,8 +1621,6 @@ double do_lbfgs(FILE *fplog, t_commrec *cr,
     gmx_mdoutf_t       outf;
     int                i, k, m, n, nfmax, gf, step;
     int                mdof_flags;
-    /* not used */
-    real               terminate;
 
     if (PAR(cr))
     {
@@ -1730,7 +1733,7 @@ double do_lbfgs(FILE *fplog, t_commrec *cr,
     ems.f   = f;
     evaluate_energy(fplog, cr,
                     top_global, &ems, top,
-                    inputrec, nrnb, wcycle, gstat,
+                    inputrec, nrnb, wcycle, gstat ? gstat : NULL,
                     vsite, constr, fcd, graph, mdatoms, fr,
                     mu_tot, enerd, vir, pres, -1, TRUE);
     where();
@@ -1763,18 +1766,22 @@ double do_lbfgs(FILE *fplog, t_commrec *cr,
 
     if (MASTER(cr))
     {
+        double sqrtNumAtoms = sqrt(static_cast<double>(state->natoms));
         fprintf(stderr, "Using %d BFGS correction steps.\n\n", nmaxcorr);
         fprintf(stderr, "   F-max             = %12.5e on atom %d\n", fmax, nfmax+1);
-        fprintf(stderr, "   F-Norm            = %12.5e\n", fnorm/sqrt(state->natoms));
+        fprintf(stderr, "   F-Norm            = %12.5e\n", fnorm/sqrtNumAtoms);
         fprintf(stderr, "\n");
         /* and copy to the log file too... */
         fprintf(fplog, "Using %d BFGS correction steps.\n\n", nmaxcorr);
         fprintf(fplog, "   F-max             = %12.5e on atom %d\n", fmax, nfmax+1);
-        fprintf(fplog, "   F-Norm            = %12.5e\n", fnorm/sqrt(state->natoms));
+        fprintf(fplog, "   F-Norm            = %12.5e\n", fnorm/sqrtNumAtoms);
         fprintf(fplog, "\n");
     }
 
+    // Point is an index to the memory of search directions, where 0 is the first one.
     point = 0;
+
+    // Set initial search direction to the force (-gradient), or 0 for frozen particles.
     for (i = 0; i < n; i++)
     {
         if (!frozen[i])
@@ -1787,6 +1794,10 @@ double do_lbfgs(FILE *fplog, t_commrec *cr,
         }
     }
 
+    // Stepsize will be modified during the search, and actually it is not critical
+    // (the main efficiency in the algorithm comes from changing directions), but
+    // we still need an initial value, so estimate it as the inverse of the norm
+    // so we take small steps where the potential fluctuates a lot.
     stepsize  = 1.0/fnorm;
 
     /* Start the loop over BFGS steps.
@@ -1826,16 +1837,16 @@ double do_lbfgs(FILE *fplog, t_commrec *cr,
 
         /* Do the linesearching in the direction dx[point][0..(n-1)] */
 
-        /* pointer to current direction - point=0 first time here */
+        /* make s a pointer to current search direction - point=0 first time we get here */
         s = dx[point];
 
-        /* calculate line gradient */
+        // calculate line gradient in position A
         for (gpa = 0, i = 0; i < n; i++)
         {
             gpa -= s[i]*ff[i];
         }
 
-        /* Calculate minimum allowed stepsize, before the average (norm)
+        /* Calculate minimum allowed stepsize along the line, before the average (norm)
          * relative change in coordinate is smaller than precision
          */
         for (minstep = 0, i = 0; i < n; i++)
@@ -1856,7 +1867,7 @@ double do_lbfgs(FILE *fplog, t_commrec *cr,
             break;
         }
 
-        /* Store old forces and coordinates */
+        // Before taking any steps along the line, store the old position
         for (i = 0; i < n; i++)
         {
             lastx[i] = xx[i];
@@ -1864,15 +1875,14 @@ double do_lbfgs(FILE *fplog, t_commrec *cr,
         }
         Epot0 = Epot;
 
-        first = TRUE;
-
         for (i = 0; i < n; i++)
         {
             xa[i] = xx[i];
         }
 
         /* Take a step downhill.
-         * In theory, we should minimize the function along this direction.
+         * In theory, we should find the actual minimum of the function in this
+         * direction, somewhere along the line.
          * That is quite possible, but it turns out to take 5-10 function evaluations
          * for each line. However, we dont really need to find the exact minimum -
          * it is much better to start a new BFGS step in a modified direction as soon
@@ -1880,25 +1890,36 @@ double do_lbfgs(FILE *fplog, t_commrec *cr,
          *
          * In practice, we just try to take a single step.
          * If it worked (i.e. lowered the energy), we increase the stepsize but
-         * the continue straight to the next BFGS step without trying to find any minimum.
+         * continue straight to the next BFGS step without trying to find any minimum,
+         * i.e. we change the search direction too. If the line was smooth, it is
+         * likely we are in a smooth region, and then it makes sense to take longer
+         * steps in the modified search direction too.
+         *
          * If it didn't work (higher energy), there must be a minimum somewhere between
-         * the old position and the new one.
+         * the old position and the new one. Then we need to start by finding a lower
+         * value before we change search direction. Since the energy was apparently
+         * quite rough, we need to decrease the step size.
          *
          * Due to the finite numerical accuracy, it turns out that it is a good idea
-         * to even accept a SMALL increase in energy, if the derivative is still downhill.
+         * to accept a SMALL increase in energy, if the derivative is still downhill.
          * This leads to lower final energies in the tests I've done. / Erik
          */
-        foundlower = FALSE;
+
+        // State "A" is the first position along the line.
+        // reference position along line is initially zero
         EpotA      = Epot0;
         a          = 0.0;
-        c          = a + stepsize; /* reference position along line is zero */
 
-        /* Check stepsize first. We do not allow displacements
-         * larger than emstep.
-         */
+        // Check stepsize first. We do not allow displacements
+        // larger than emstep.
+        //
         do
         {
+            // Pick a new position C by adding stepsize to A.
             c        = a + stepsize;
+
+            // Calculate what the largest change in any individual coordinate
+            // would be (translation along line * gradient along line)
             maxdelta = 0;
             for (i = 0; i < n; i++)
             {
@@ -1908,6 +1929,7 @@ double do_lbfgs(FILE *fplog, t_commrec *cr,
                     maxdelta = delta;
                 }
             }
+            // If any displacement is larger than the stepsize limit, reduce the step
             if (maxdelta > inputrec->em_stepsize)
             {
                 stepsize *= 0.1;
@@ -1915,24 +1937,24 @@ double do_lbfgs(FILE *fplog, t_commrec *cr,
         }
         while (maxdelta > inputrec->em_stepsize);
 
-        /* Take a trial step */
+        // Take a trial step and move the coordinate array xc[] to position C
         for (i = 0; i < n; i++)
         {
             xc[i] = lastx[i] + c*s[i];
         }
 
         neval++;
-        /* Calculate energy for the trial step */
+        // Calculate energy for the trial step in position C
         ems.s.x = (rvec *)xc;
         ems.f   = (rvec *)fc;
         evaluate_energy(fplog, cr,
                         top_global, &ems, top,
-                        inputrec, nrnb, wcycle, gstat,
+                        inputrec, nrnb, wcycle, gstat ? gstat : NULL,
                         vsite, constr, fcd, graph, mdatoms, fr,
                         mu_tot, enerd, vir, pres, step, FALSE);
         EpotC = ems.epot;
 
-        /* Calc derivative along line */
+        // Calc line gradient in position C
         for (gpc = 0, i = 0; i < n; i++)
         {
             gpc -= s[i]*fc[i]; /* f is negative gradient, thus the sign */
@@ -1943,59 +1965,53 @@ double do_lbfgs(FILE *fplog, t_commrec *cr,
             gmx_sumd(1, &gpc, cr);
         }
 
-        /* This is the max amount of increase in energy we tolerate */
+        // This is the max amount of increase in energy we tolerate.
+        // By allowing VERY small changes (close to numerical precision) we
+        // frequently find even better (lower) final energies.
         tmp = sqrt(GMX_REAL_EPS)*fabs(EpotA);
 
-        /* Accept the step if the energy is lower, or if it is not significantly higher
-         * and the line derivative is still negative.
-         */
+        // Accept the step if the energy is lower in the new position C (compared to A),
+        // or if it is not significantly higher and the line derivative is still negative.
         if (EpotC < EpotA || (gpc < 0 && EpotC < (EpotA+tmp)))
         {
+            // Great, we found a better energy. We no longer try to alter the
+            // stepsize, but simply accept this new better position. The we select a new
+            // search direction instead, which will be much more efficient than continuing
+            // to take smaller steps along a line. Set fnorm based on the new C position,
+            // which will be used to update the stepsize to 1/fnorm further down.
             foundlower = TRUE;
-            /* Great, we found a better energy. Increase step for next iteration
-             * if we are still going down, decrease it otherwise
-             */
-            if (gpc < 0)
-            {
-                stepsize *= 1.618034; /* The golden section */
-            }
-            else
-            {
-                stepsize *= 0.618034; /* 1/golden section */
-            }
+            fnorm      = ems.fnorm;
         }
         else
         {
-            /* New energy is the same or higher. We will have to do some work
-             * to find a smaller value in the interval. Take smaller step next time!
-             */
+            // If we got here, the energy is NOT lower in point C, i.e. it will be the same
+            // or higher than in point A. In this case it is pointless to move to point C,
+            // so we will have to do more iterations along the same line to find a smaller
+            // value in the interval [A=0.0,C].
+            // Here, A is still 0.0, but that will change when we do a search in the interval
+            // [0.0,C] below. That search we will do by interpolation or bisection rather
+            // than with the stepsize, so no need to modify it. For the next search direction
+            // it will be reset to 1/fnorm anyway.
             foundlower = FALSE;
-            stepsize  *= 0.618034;
         }
 
-        /* OK, if we didn't find a lower value we will have to locate one now - there must
-         * be one in the interval [a=0,c].
-         * The same thing is valid here, though: Don't spend dozens of iterations to find
-         * the line minimum. We try to interpolate based on the derivative at the endpoints,
-         * and only continue until we find a lower value. In most cases this means 1-2 iterations.
-         *
-         * I also have a safeguard for potentially really patological functions so we never
-         * take more than 20 steps before we give up ...
-         *
-         * If we already found a lower value we just skip this step and continue to the update.
-         */
-
         if (!foundlower)
         {
-
+            // OK, if we didn't find a lower value we will have to locate one now - there must
+            // be one in the interval [a,c].
+            // The same thing is valid here, though: Don't spend dozens of iterations to find
+            // the line minimum. We try to interpolate based on the derivative at the endpoints,
+            // and only continue until we find a lower value. In most cases this means 1-2 iterations.
+            // I also have a safeguard for potentially really pathological functions so we never
+            // take more than 20 steps before we give up.
+            // If we already found a lower value we just skip this step and continue to the update.
             nminstep = 0;
             do
             {
-                /* Select a new trial point.
-                 * If the derivatives at points a & c have different sign we interpolate to zero,
-                 * otherwise just do a bisection.
-                 */
-
+                // Select a new trial point B in the interval [A,C].
+                // If the derivatives at points a & c have different sign we interpolate to zero,
+                // otherwise just do a bisection since there might be multiple minima/maxima
+                // inside the interval.
                 if (gpa < 0 && gpc > 0)
                 {
                     b = a + gpa*(a-c)/(gpc-gpa);
@@ -2013,25 +2029,25 @@ double do_lbfgs(FILE *fplog, t_commrec *cr,
                     b = 0.5*(a+c);
                 }
 
-                /* Take a trial step */
+                // Take a trial step to point B
                 for (i = 0; i < n; i++)
                 {
                     xb[i] = lastx[i] + b*s[i];
                 }
 
                 neval++;
-                /* Calculate energy for the trial step */
+                // Calculate energy for the trial step in point B
                 ems.s.x = (rvec *)xb;
                 ems.f   = (rvec *)fb;
                 evaluate_energy(fplog, cr,
                                 top_global, &ems, top,
-                                inputrec, nrnb, wcycle, gstat,
+                                inputrec, nrnb, wcycle, gstat ? gstat : NULL,
                                 vsite, constr, fcd, graph, mdatoms, fr,
                                 mu_tot, enerd, vir, pres, step, FALSE);
                 EpotB = ems.epot;
-
                 fnorm = ems.fnorm;
 
+                // Calculate gradient in point B
                 for (gpb = 0, i = 0; i < n; i++)
                 {
                     gpb -= s[i]*fb[i]; /* f is negative gradient, thus the sign */
@@ -2043,7 +2059,8 @@ double do_lbfgs(FILE *fplog, t_commrec *cr,
                     gmx_sumd(1, &gpb, cr);
                 }
 
-                /* Keep one of the intervals based on the value of the derivative at the new point */
+                // Keep one of the intervals [A,B] or [B,C] based on the value of the derivative
+                // at the new point B, and rename the endpoints of this new interval A and C.
                 if (gpb > 0)
                 {
                     /* Replace c endpoint with b */
@@ -2120,7 +2137,7 @@ double do_lbfgs(FILE *fplog, t_commrec *cr,
                     xx[i] = xc[i];
                     ff[i] = fc[i];
                 }
-                stepsize = c;
+                step_taken = c;
             }
             else
             {
@@ -2131,7 +2148,7 @@ double do_lbfgs(FILE *fplog, t_commrec *cr,
                     xx[i] = xa[i];
                     ff[i] = fa[i];
                 }
-                stepsize = a;
+                step_taken = a;
             }
 
         }
@@ -2145,7 +2162,7 @@ double do_lbfgs(FILE *fplog, t_commrec *cr,
                 xx[i] = xc[i];
                 ff[i] = fc[i];
             }
-            stepsize = c;
+            step_taken = c;
         }
 
         /* Update the memory information, and calculate a new
@@ -2161,7 +2178,7 @@ double do_lbfgs(FILE *fplog, t_commrec *cr,
         for (i = 0; i < n; i++)
         {
             dg[point][i]  = lastf[i]-ff[i];
-            dx[point][i] *= stepsize;
+            dx[point][i] *= step_taken;
         }
 
         dgdg = 0;
@@ -2254,8 +2271,6 @@ double do_lbfgs(FILE *fplog, t_commrec *cr,
             }
         }
 
-        stepsize = 1.0;
-
         /* Test whether the convergence criterion is met */
         get_f_norm_max(cr, &(inputrec->opts), mdatoms, f, &fnorm, &fmax, &nfmax);
 
@@ -2264,8 +2279,9 @@ double do_lbfgs(FILE *fplog, t_commrec *cr,
         {
             if (bVerbose)
             {
+                double sqrtNumAtoms = sqrt(static_cast<double>(state->natoms));
                 fprintf(stderr, "\rStep %d, Epot=%12.6e, Fnorm=%9.3e, Fmax=%9.3e (atom %d)\n",
-                        step, Epot, fnorm/sqrt(state->natoms), fmax, nfmax+1);
+                        step, Epot, fnorm/sqrtNumAtoms, fmax, nfmax+1);
             }
             /* Store the new (lower) energies */
             upd_mdebin(mdebin, FALSE, FALSE, (double)step,
@@ -2288,10 +2304,12 @@ double do_lbfgs(FILE *fplog, t_commrec *cr,
             IMD_send_positions(inputrec->imd);
         }
 
+        // Reset stepsize in we are doing more iterations
+        stepsize = 1.0/fnorm;
+
         /* Stop when the maximum force lies below tolerance.
          * If we have reached machine precision, converged is already set to true.
          */
-
         converged = converged || (fmax < inputrec->em_tol);
 
     } /* End of the loop */
@@ -2349,10 +2367,11 @@ double do_lbfgs(FILE *fplog, t_commrec *cr,
 
     if (MASTER(cr))
     {
+        double sqrtNumAtoms = sqrt(static_cast<double>(state->natoms));
         print_converged(stderr, LBFGS, inputrec->em_tol, step, converged,
-                        number_steps, Epot, fmax, nfmax, fnorm/sqrt(state->natoms));
+                        number_steps, Epot, fmax, nfmax, fnorm/sqrtNumAtoms);
         print_converged(fplog, LBFGS, inputrec->em_tol, step, converged,
-                        number_steps, Epot, fmax, nfmax, fnorm/sqrt(state->natoms));
+                        number_steps, Epot, fmax, nfmax, fnorm/sqrtNumAtoms);
 
         fprintf(fplog, "\nPerformed %d energy evaluations in total.\n", neval);
     }
@@ -2395,7 +2414,7 @@ double do_steep(FILE *fplog, t_commrec *cr,
     rvec             *f;
     gmx_global_stat_t gstat;
     t_graph          *graph;
-    real              stepsize, constepsize;
+    real              stepsize;
     real              ustep, fnormn;
     gmx_mdoutf_t      outf;
     t_mdebin         *mdebin;
@@ -2405,8 +2424,6 @@ double do_steep(FILE *fplog, t_commrec *cr,
     int               nsteps;
     int               count          = 0;
     int               steps_accepted = 0;
-    /* not used */
-    real              terminate = 0;
 
     s_min = init_em_state();
     s_try = init_em_state();
@@ -2462,7 +2479,7 @@ double do_steep(FILE *fplog, t_commrec *cr,
 
         evaluate_energy(fplog, cr,
                         top_global, s_try, top,
-                        inputrec, nrnb, wcycle, gstat,
+                        inputrec, nrnb, wcycle, gstat ? gstat : NULL,
                         vsite, constr, fcd, graph, mdatoms, fr,
                         mu_tot, enerd, vir, pres, count, count == 0);
 
@@ -2586,10 +2603,11 @@ double do_steep(FILE *fplog, t_commrec *cr,
                   top_global, inputrec, count,
                   s_min, state_global, f_global);
 
-    fnormn = s_min->fnorm/sqrt(state_global->natoms);
-
     if (MASTER(cr))
     {
+        double sqrtNumAtoms = sqrt(static_cast<double>(state_global->natoms));
+        fnormn = s_min->fnorm/sqrtNumAtoms;
+
         print_converged(stderr, SD, inputrec->em_tol, count, bDone, nsteps,
                         s_min->epot, s_min->fmax, s_min->a_fmax, fnormn);
         print_converged(fplog, SD, inputrec->em_tol, count, bDone, nsteps,
@@ -2638,8 +2656,6 @@ double do_nm(FILE *fplog, t_commrec *cr,
     rvec                *f;
     gmx_global_stat_t    gstat;
     t_graph             *graph;
-    real                 t, t0, lambda, lam0;
-    gmx_bool             bNS;
     tensor               vir, pres;
     rvec                 mu_tot;
     rvec                *fneg, *dfdx;
@@ -2653,7 +2669,7 @@ double do_nm(FILE *fplog, t_commrec *cr,
     int        i, j, k, row, col;
     real       der_range = 10.0*sqrt(GMX_REAL_EPS);
     real       x_min;
-    real       fnorm, fmax;
+    bool       bIsMaster = MASTER(cr);
 
     if (constr != NULL)
     {
@@ -2674,7 +2690,7 @@ double do_nm(FILE *fplog, t_commrec *cr,
     snew(dfdx, natoms);
 
 #ifndef GMX_DOUBLE
-    if (MASTER(cr))
+    if (bIsMaster)
     {
         fprintf(stderr,
                 "NOTE: This version of Gromacs has been compiled in single precision,\n"
@@ -2706,7 +2722,7 @@ double do_nm(FILE *fplog, t_commrec *cr,
         bSparse = TRUE;
     }
 
-    if (MASTER(cr))
+    if (bIsMaster)
     {
         sz = DIM*top_global->natoms;
 
@@ -2723,12 +2739,6 @@ double do_nm(FILE *fplog, t_commrec *cr,
         }
     }
 
-    /* Initial values */
-    t0           = inputrec->init_t;
-    lam0         = inputrec->fepvals->init_lambda;
-    t            = t0;
-    lambda       = lam0;
-
     init_nrnb(nrnb);
 
     where();
@@ -2739,7 +2749,7 @@ double do_nm(FILE *fplog, t_commrec *cr,
     /* fudge nr of steps to nr of atoms */
     inputrec->nsteps = natoms*2;
 
-    if (MASTER(cr))
+    if (bIsMaster)
     {
         fprintf(stderr, "starting normal mode calculation '%s'\n%d steps.\n\n",
                 *(top_global->name), (int)inputrec->nsteps);
@@ -2751,7 +2761,7 @@ double do_nm(FILE *fplog, t_commrec *cr,
     cr->nnodes = 1;
     evaluate_energy(fplog, cr,
                     top_global, state_work, top,
-                    inputrec, nrnb, wcycle, gstat,
+                    inputrec, nrnb, wcycle, gstat ? gstat : NULL,
                     vsite, constr, fcd, graph, mdatoms, fr,
                     mu_tot, enerd, vir, pres, -1, TRUE);
     cr->nnodes = nnodes;
@@ -2792,7 +2802,7 @@ double do_nm(FILE *fplog, t_commrec *cr,
             cr->nnodes = 1;
             evaluate_energy(fplog, cr,
                             top_global, state_work, top,
-                            inputrec, nrnb, wcycle, gstat,
+                            inputrec, nrnb, wcycle, gstat ? gstat : NULL,
                             vsite, constr, fcd, graph, mdatoms, fr,
                             mu_tot, enerd, vir, pres, atom*2, FALSE);
 
@@ -2805,7 +2815,7 @@ double do_nm(FILE *fplog, t_commrec *cr,
 
             evaluate_energy(fplog, cr,
                             top_global, state_work, top,
-                            inputrec, nrnb, wcycle, gstat,
+                            inputrec, nrnb, wcycle, gstat ? gstat : NULL,
                             vsite, constr, fcd, graph, mdatoms, fr,
                             mu_tot, enerd, vir, pres, atom*2+1, FALSE);
             cr->nnodes = nnodes;
@@ -2822,7 +2832,7 @@ double do_nm(FILE *fplog, t_commrec *cr,
                 }
             }
 
-            if (!MASTER(cr))
+            if (!bIsMaster)
             {
 #ifdef GMX_MPI
 #ifdef GMX_DOUBLE
@@ -2879,15 +2889,15 @@ double do_nm(FILE *fplog, t_commrec *cr,
             }
         }
         /* write progress */
-        if (MASTER(cr) && bVerbose)
+        if (bIsMaster && bVerbose)
         {
             fprintf(stderr, "\rFinished step %d out of %d",
-                    min(atom+nnodes, natoms), natoms);
+                    std::min(atom+nnodes, natoms), natoms);
             fflush(stderr);
         }
     }
 
-    if (MASTER(cr))
+    if (bIsMaster)
     {
         fprintf(stderr, "\n\nWriting Hessian...\n");
         gmx_mtxio_write(ftp2fn(efMTX, nfile, fnm), sz, sz, full_matrix, sparse_matrix);
similarity index 97%
rename from src/gromacs/mdlib/shellfc.c
rename to src/gromacs/mdlib/shellfc.cpp
index 0af5fdfdcadd0b975ee1c54a24530146144191c3..0e3ed9985792266d8e9bf7fbe9b114d28afbc935 100644 (file)
@@ -41,6 +41,8 @@
 #include <stdlib.h>
 #include <string.h>
 
+#include <algorithm>
+
 #include "gromacs/domdec/domdec.h"
 #include "gromacs/legacyheaders/chargegroup.h"
 #include "gromacs/legacyheaders/constr.h"
@@ -127,7 +129,7 @@ static void predict_shells(FILE *fplog, rvec x[], rvec v[], real dt,
                            real mass[], gmx_mtop_t *mtop, gmx_bool bInit)
 {
     int                   i, m, s1, n1, n2, n3;
-    real                  dt_1, dt_2, dt_3, fudge, tm, m1, m2, m3;
+    real                  dt_1, fudge, tm, m1, m2, m3;
     rvec                 *ptr;
     gmx_mtop_atomlookup_t alook = NULL;
     t_atom               *atom;
@@ -185,8 +187,10 @@ static void predict_shells(FILE *fplog, rvec x[], rvec v[], real dt,
                 else
                 {
                     /* Not the correct masses with FE, but it is just a prediction... */
-                    m1 = atom[n1].m;
-                    m2 = atom[n2].m;
+                    gmx_mtop_atomnr_to_atom(alook, n1, &atom);
+                    m1 = atom->m;
+                    gmx_mtop_atomnr_to_atom(alook, n2, &atom);
+                    m2 = atom->m;
                 }
                 tm = dt_1/(m1+m2);
                 for (m = 0; (m < DIM); m++)
@@ -240,7 +244,7 @@ gmx_shellfc_t init_shell_flexcon(FILE *fplog,
     int                      *shell_index = NULL, *at2cg;
     t_atom                   *atom;
     int                       n[eptNR], ns, nshell, nsi;
-    int                       i, j, nmol, type, mb, mt, a_offset, cg, mol, ftype, nra;
+    int                       i, j, nmol, type, mb, a_offset, cg, mol, ftype, nra;
     real                      qS, alpha;
     int                       aS, aN = 0; /* Shell and nucleus */
     int                       bondtypes[] = { F_BONDS, F_HARMONIC, F_CUBICBONDS, F_POLARIZATION, F_ANHARM_POL, F_WATER_POL };
@@ -663,10 +667,11 @@ static void shell_pos_sd(rvec xcur[], rvec xnew[], rvec f[],
                step_scale_increment = 0.2,
                step_scale_max       = 1.2,
                step_scale_multiple  = (step_scale_max - step_scale_min) / step_scale_increment;
-    int  i, shell, d;
-    real dx, df, k_est;
+    int        i, shell, d;
+    real       dx, df, k_est;
+    const real zero = 0;
 #ifdef PRINT_STEP
-    real step_min, step_max;
+    real       step_min, step_max;
 
     step_min = 1e30;
     step_max = 0;
@@ -680,8 +685,8 @@ static void shell_pos_sd(rvec xcur[], rvec xnew[], rvec f[],
             {
                 s[i].step[d] = s[i].k_1;
 #ifdef PRINT_STEP
-                step_min = min(step_min, s[i].step[d]);
-                step_max = max(step_max, s[i].step[d]);
+                step_min = std::min(step_min, s[i].step[d]);
+                step_max = std::max(step_max, s[i].step[d]);
 #endif
             }
         }
@@ -703,7 +708,7 @@ static void shell_pos_sd(rvec xcur[], rvec xnew[], rvec f[],
                      * step_scale_multiple * s[i].step[d] */
                     s[i].step[d] =
                         step_scale_min * s[i].step[d] +
-                        step_scale_increment * min(step_scale_multiple * s[i].step[d], max(k_est, 0));
+                        step_scale_increment * std::min(step_scale_multiple * s[i].step[d], std::max(k_est, zero));
                 }
                 else
                 {
@@ -719,8 +724,8 @@ static void shell_pos_sd(rvec xcur[], rvec xnew[], rvec f[],
                     }
                 }
 #ifdef PRINT_STEP
-                step_min = min(step_min, s[i].step[d]);
-                step_max = max(step_max, s[i].step[d]);
+                step_min = std::min(step_min, s[i].step[d]);
+                step_max = std::max(step_max, s[i].step[d]);
 #endif
             }
         }
@@ -846,11 +851,9 @@ static void init_adir(FILE *log, gmx_shellfc_t shfc,
 {
     rvec           *xnold, *xnew;
     double          w_dt;
-    int             gf, ga, gt;
-    real            dt, scale;
+    real            dt;
     int             n, d;
     unsigned short *ptype;
-    rvec            p, dx;
 
     if (DOMAINDECOMP(cr))
     {
@@ -944,14 +947,13 @@ int relax_shell_flexcon(FILE *fplog, t_commrec *cr, gmx_bool bVerbose,
     t_idef    *idef;
     rvec      *pos[2], *force[2], *acc_dir = NULL, *x_old = NULL;
     real       Epot[2], df[2];
-    rvec       dx;
     real       sf_dir, invdt;
-    real       ftol, xiH, xiS, dum = 0;
+    real       ftol, dum = 0;
     char       sbuf[22];
     gmx_bool   bCont, bInit;
     int        nat, dd_ac0, dd_ac1 = 0, i;
     int        start = 0, homenr = md->homenr, end = start+homenr, cg0, cg1;
-    int        nflexcon, g, number_steps, d, Min = 0, count = 0;
+    int        nflexcon, number_steps, d, Min = 0, count = 0;
 #define  Try (1-Min)             /* At start Try = 1 */
 
     bCont        = (mdstep == inputrec->init_step) && inputrec->bContinuation;
@@ -970,7 +972,7 @@ int relax_shell_flexcon(FILE *fplog, t_commrec *cr, gmx_bool bVerbose,
         if (nflexcon > 0)
         {
             dd_get_constraint_range(cr->dd, &dd_ac0, &dd_ac1);
-            nat = max(nat, dd_ac1);
+            nat = std::max(nat, dd_ac1);
         }
     }
     else
similarity index 99%
rename from src/gromacs/mdlib/tpi.c
rename to src/gromacs/mdlib/tpi.cpp
index b0c64214462146712a811cada34ee4f8b9b05090..3d8a37fda22aa7eeb81975182c084e2c2ad7970e 100644 (file)
@@ -41,6 +41,8 @@
 #include <string.h>
 #include <time.h>
 
+#include <algorithm>
+
 #include "gromacs/domdec/domdec.h"
 #include "gromacs/ewald/pme.h"
 #include "gromacs/fileio/confio.h"
@@ -83,7 +85,7 @@ static void global_max(t_commrec *cr, int *n)
     gmx_sumi(cr->nnodes, sum, cr);
     for (i = 0; i < cr->nnodes; i++)
     {
-        *n = max(*n, sum[i]);
+        *n = std::max(*n, sum[i]);
     }
 
     sfree(sum);
@@ -125,7 +127,6 @@ double do_tpi(FILE *fplog, t_commrec *cr,
               unsigned long gmx_unused Flags,
               gmx_walltime_accounting_t walltime_accounting)
 {
-    const char     *TPI = "Test Particle Insertion";
     gmx_localtop_t *top;
     gmx_groups_t   *groups;
     gmx_enerdata_t *enerd;
@@ -145,7 +146,7 @@ double do_tpi(FILE *fplog, t_commrec *cr,
     gmx_int64_t     rnd_count_stride, rnd_count;
     gmx_int64_t     seed;
     double          rnd[4];
-    int             i, start, end;
+    int             i;
     FILE           *fp_tpi = NULL;
     char           *ptr, *dump_pdb, **leg, str[STRLEN], str2[STRLEN];
     double          dbl, dump_ener;
@@ -154,7 +155,7 @@ double do_tpi(FILE *fplog, t_commrec *cr,
     real           *mass_cavity = NULL, mass_tot;
     int             nbin;
     double          invbinw, *bin, refvolshift, logV, bUlogV;
-    real            dvdl, prescorr, enercorr, dvdlcorr;
+    real            prescorr, enercorr, dvdlcorr;
     gmx_bool        bEnergyOutOfBounds;
     const char     *tpid_leg[2] = {"direct", "reweighted"};
 
@@ -189,7 +190,7 @@ double do_tpi(FILE *fplog, t_commrec *cr,
              * The center of mass of the last atoms is then used for TPIC.
              */
             nat_cavity = 0;
-            while (sscanf(ptr, "%lf%n", &dbl, &i) > 0)
+            while (sscanf(ptr, "%20lf%n", &dbl, &i) > 0)
             {
                 srenew(mass_cavity, nat_cavity+1);
                 mass_cavity[nat_cavity] = dbl;
@@ -244,7 +245,7 @@ double do_tpi(FILE *fplog, t_commrec *cr,
     dump_ener = 0;
     if (dump_pdb)
     {
-        sscanf(dump_pdb, "%lf", &dump_ener);
+        sscanf(dump_pdb, "%20lf", &dump_ener);
     }
 
     atoms2md(top_global, inputrec, 0, NULL, top_global->natoms, mdatoms);