Merge branch 'release-4-5-patches'
authorTeemu Murtola <teemu.murtola@gmail.com>
Fri, 19 Apr 2013 03:50:37 +0000 (06:50 +0300)
committerTeemu Murtola <teemu.murtola@gmail.com>
Fri, 19 Apr 2013 03:50:37 +0000 (06:50 +0300)
Tested that the selection fixes work also in release-4-6.

Conflicts:
src/gmxlib/selection/compiler.c (trivial)

Change-Id: Ib679b1babaf29457c13ca1f10fde5a612ddc3055

1  2 
src/gmxlib/selection/compiler.c
src/gmxlib/selection/evaluate.c

index 0ccb548d3e2a7902c927c06433b8747680fda2b7,a10bd7a4911b81f86c4831cacd1ad197c42df264..326635cdd0d866d1e463d5b5d70bdeb1706de4f9
@@@ -1,39 -1,32 +1,39 @@@
  /*
 + * This file is part of the GROMACS molecular simulation package.
   *
 - *                This source code is part of
 - *
 - *                 G   R   O   M   A   C   S
 - *
 - *          GROningen MAchine for Chemical Simulations
 - *
 - * Written by David van der Spoel, Erik Lindahl, Berk Hess, and others.
   * Copyright (c) 1991-2000, University of Groningen, The Netherlands.
   * Copyright (c) 2001-2009, The GROMACS development team,
   * check out http://www.gromacs.org for more information.
 -
 - * This program is free software; you can redistribute it and/or
 - * modify it under the terms of the GNU General Public License
 - * as published by the Free Software Foundation; either version 2
 + * Copyright (c) 2012,2013, by the GROMACS development team, led by
 + * David van der Spoel, Berk Hess, Erik Lindahl, and including many
 + * others, as listed in the AUTHORS file in the top-level source
 + * directory and at http://www.gromacs.org.
 + *
 + * GROMACS is free software; you can redistribute it and/or
 + * modify it under the terms of the GNU Lesser General Public License
 + * as published by the Free Software Foundation; either version 2.1
   * of the License, or (at your option) any later version.
   *
 - * If you want to redistribute modifications, please consider that
 - * scientific software is very special. Version control is crucial -
 - * bugs must be traceable. We will be happy to consider code for
 - * inclusion in the official distribution, but derived work must not
 - * be called official GROMACS. Details are found in the README & COPYING
 - * files - if they are missing, get the official version at www.gromacs.org.
 + * GROMACS is distributed in the hope that it will be useful,
 + * but WITHOUT ANY WARRANTY; without even the implied warranty of
 + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
 + * Lesser General Public License for more details.
   *
 - * To help us fund GROMACS development, we humbly ask that you cite
 - * the papers on the package - you can find them in the top README file.
 + * You should have received a copy of the GNU Lesser General Public
 + * License along with GROMACS; if not, see
 + * http://www.gnu.org/licenses, or write to the Free Software Foundation,
 + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  USA.
 + *
 + * If you want to redistribute modifications to GROMACS, please
 + * consider that scientific software is very special. Version
 + * control is crucial - bugs must be traceable. We will be happy to
 + * consider code for inclusion in the official distribution, but
 + * derived work must not be called official GROMACS. Details are found
 + * in the README & COPYING files - if they are missing, get the
 + * official version at http://www.gromacs.org.
   *
 - * For more info, check our website at http://www.gromacs.org
 + * To help us fund GROMACS development, we humbly ask that you cite
 + * the research papers on the package. Check out http://www.gromacs.org.
   */
  /*! \internal \file
   * \brief Selection compilation and optimization.
  #include <smalloc.h>
  #include <string2.h>
  #include <vec.h>
 +#include <assert.h>
  
  #include <indexutil.h>
  #include <poscalc.h>
@@@ -314,8 -306,15 +314,15 @@@ enu
      SEL_CDATA_MINMAXALLOC = 16,
      /** Whether subexpressions use simple pass evaluation functions. */
      SEL_CDATA_SIMPLESUBEXPR = 32,
-     /** Whether this expressions is a part of a common subexpression. */
-     SEL_CDATA_COMMONSUBEXPR = 64
+     /*! \brief
+      * Whether a static subexpression needs to support multiple evaluations.
+      *
+      * This flag may only be set on \ref SEL_SUBEXPR elements that also have
+      * SEL_CDATA_SIMPLESUBEXPR.
+      */
+     SEL_CDATA_STATICMULTIEVALSUBEXPR = 64,
+     /** Whether this expression is a part of a common subexpression. */
+     SEL_CDATA_COMMONSUBEXPR = 128
  };
  
  /*! \internal \brief
@@@ -397,6 -396,10 +404,10 @@@ _gmx_selelem_print_compiler_info(FILE *
      {
          fprintf(fp, "Ss");
      }
+     if (sel->cdata->flags & SEL_CDATA_STATICMULTIEVALSUBEXPR)
+     {
+         fprintf(fp, "Sm");
+     }
      if (sel->cdata->flags & SEL_CDATA_COMMONSUBEXPR)
      {
          fprintf(fp, "Sc");
@@@ -525,7 -528,7 +536,7 @@@ alloc_selection_pos_data(t_selelem *sel
          child = sel->child->child;
      }
      nalloc = child->v.u.p->nr;
 -    isize = child->v.u.p->m.b.nra;
 +    isize  = child->v.u.p->m.b.nra;
  
      /* For positions, we want to allocate just a single structure
       * for nalloc positions. */
@@@ -584,10 -587,10 +595,10 @@@ reverse_selelem_chain(t_selelem *root
      item = root;
      while (item)
      {
 -        next = item->next;
 +        next       = item->next;
          item->next = prev;
 -        prev = item;
 -        item = next;
 +        prev       = item;
 +        item       = next;
      }
      return prev;
  }
@@@ -686,7 -689,7 +697,7 @@@ extract_item_subselections(t_selelem *s
      t_selelem *subexpr;
      t_selelem *child;
  
 -    root = subexpr = NULL;
 +    root  = subexpr = NULL;
      child = sel->child;
      while (child)
      {
  
  /*! \brief
   * Extracts subexpressions of the selection chain.
 - * 
 + *
   * \param   sel First selection in the whole selection chain.
   * \returns The new first element for the chain.
   *
@@@ -767,8 -770,8 +778,8 @@@ extract_subexpressions(t_selelem *sel
      int          subexprn;
  
      subexprn = 0;
 -    root = NULL;
 -    next = sel;
 +    root     = NULL;
 +    next     = sel;
      while (next)
      {
          item = extract_item_subselections(next, &subexprn);
          {
              root = next;
          }
 -        sel = next;
 +        sel  = next;
          next = next->next;
      }
      return root;
@@@ -881,7 -884,7 +892,7 @@@ optimize_gmx_boolean_expressions(t_sele
          }
          else
          {
 -            prev = child;
 +            prev  = child;
              child = child->next;
          }
      }
@@@ -918,8 -921,8 +929,8 @@@ reorder_gmx_boolean_static_children(t_s
          t_selelem  start;
  
          start.next = sel->child;
 -        prev  = &start;
 -        child = &start;
 +        prev       = &start;
 +        child      = &start;
          while (child->next)
          {
              /* child is the last handled static expression */
  static gmx_bool
  optimize_arithmetic_expressions(t_selelem *sel)
  {
 -    t_selelem  *child;
 +    t_selelem      *child;
      gmx_bool        bOk;
  
      /* Do recursively for children. */
              snew(r, 1);
              r[0] = child->v.u.i[0];
              sfree(child->v.u.i);
 -            child->v.u.r = r;
 +            child->v.u.r  = r;
              child->v.type = REAL_VALUE;
          }
          else if (child->v.type != REAL_VALUE)
@@@ -1102,19 -1105,25 +1113,25 @@@ init_item_evalfunc(t_selelem *sel
              break;
  
          case SEL_SUBEXPR:
-             sel->evaluate = (sel->refcount == 2
-                              ? &_gmx_sel_evaluate_subexpr_simple
-                              : &_gmx_sel_evaluate_subexpr);
+             if (sel->refcount == 2
+                 && !(sel->cdata->flags & SEL_CDATA_STATICMULTIEVALSUBEXPR))
+             {
+                 sel->evaluate = &_gmx_sel_evaluate_subexpr_simple;
+             }
+             else
+             {
+                 sel->evaluate = &_gmx_sel_evaluate_subexpr;
+             }
              break;
  
          case SEL_SUBEXPRREF:
              sel->name     = sel->child->name;
-             sel->evaluate = (sel->child->refcount == 2
+             sel->evaluate = ((sel->cdata->flags & SEL_CDATA_SIMPLESUBEXPR)
                               ? &_gmx_sel_evaluate_subexprref_simple
                               : &_gmx_sel_evaluate_subexprref);
              break;
      }
+     sel->cdata->evaluate = sel->evaluate;
      return TRUE;
  }
  
@@@ -1163,9 -1172,6 +1180,9 @@@ setup_memory_pooling(t_selelem *sel, gm
  static void
  init_item_evaloutput(t_selelem *sel)
  {
 +    assert(!(sel->child == NULL &&
 +             (sel->type == SEL_SUBEXPRREF || sel->type == SEL_SUBEXPR)));
 +
      /* Process children. */
      if (sel->type != SEL_SUBEXPRREF)
      {
          }
      }
  
-     if (sel->type == SEL_SUBEXPR && sel->refcount == 2)
+     if (sel->type == SEL_SUBEXPR && sel->refcount == 2
+         && !(sel->cdata->flags & SEL_CDATA_STATICMULTIEVALSUBEXPR))
      {
          sel->flags &= ~(SEL_ALLOCVAL | SEL_ALLOCDATA);
          if (sel->v.type == GROUP_VALUE || sel->v.type == POS_VALUE)
      else if (sel->type == SEL_SUBEXPR
               && (sel->cdata->flags & SEL_CDATA_FULLEVAL))
      {
 -        sel->evaluate = &_gmx_sel_evaluate_subexpr_staticeval;
 +        sel->evaluate        = &_gmx_sel_evaluate_subexpr_staticeval;
          sel->cdata->evaluate = sel->evaluate;
 -        sel->child->mempool = NULL;
 -        sel->flags &= ~(SEL_ALLOCVAL | SEL_ALLOCDATA);
 +        sel->child->mempool  = NULL;
 +        sel->flags          &= ~(SEL_ALLOCVAL | SEL_ALLOCDATA);
          if (sel->v.type == GROUP_VALUE || sel->v.type == POS_VALUE)
          {
              _gmx_selvalue_setstore(&sel->v, sel->child->v.u.ptr);
          }
      }
-     else if (sel->type == SEL_SUBEXPRREF && sel->child->refcount == 2)
+     else if (sel->type == SEL_SUBEXPRREF
+              && (sel->cdata->flags & SEL_CDATA_SIMPLESUBEXPR))
      {
          if (sel->v.u.ptr)
          {
@@@ -1296,7 -1304,7 +1315,7 @@@ init_item_compilerdata(t_selelem *sel
          gmx_bool  bEvalMax;
  
          bEvalMax = (sel->u.boolt == BOOL_AND);
 -        child = sel->child;
 +        child    = sel->child;
          while (child)
          {
              if (bEvalMax)
          while (child)
          {
              child->cdata->flags |= SEL_CDATA_EVALMAX;
 -            child = child->next;
 +            child                = child->next;
          }
      }
  }
@@@ -1361,6 -1369,15 +1380,15 @@@ init_item_staticeval(t_selelem *sel
                      init_item_staticeval(child);
                  }
              }
+             /* If an expression is evaluated for a dynamic group, then also
+              * atom-valued parameters need to be evaluated every time. */
+             if ((sel->flags & SEL_DYNAMIC)
+                 && (sel->type == SEL_EXPRESSION || sel->type == SEL_MODIFIER)
+                 && (child->flags & SEL_ATOMVAL))
+             {
+                 child->flags        |= SEL_DYNAMIC;
+                 child->cdata->flags &= ~SEL_CDATA_STATIC;
+             }
              child = child->next;
          }
      }
              while (child)
              {
                  child->cdata->flags &= ~SEL_CDATA_STATICEVAL;
 -                child = child->next;
 +                child                = child->next;
              }
          }
  
@@@ -1417,7 -1434,17 +1445,17 @@@ init_item_subexpr_flags(t_selelem *sel
      }
      else if (sel->type == SEL_SUBEXPRREF && sel->child->refcount == 2)
      {
-         sel->cdata->flags |= SEL_CDATA_SIMPLESUBEXPR;
+         /* See similar condition in init_item_staticeval(). */
+         if ((sel->flags & SEL_ATOMVAL)
+             && (sel->flags & SEL_DYNAMIC)
+             && !(sel->child->flags & SEL_DYNAMIC))
+         {
+             sel->child->cdata->flags |= SEL_CDATA_STATICMULTIEVALSUBEXPR;
+         }
+         else
+         {
+             sel->cdata->flags |= SEL_CDATA_SIMPLESUBEXPR;
+         }
      }
  
      /* Process children, but only follow subexpression references if the
@@@ -1478,7 -1505,6 +1516,7 @@@ init_item_minmax_groups(t_selelem *sel
                   && ((sel->cdata->flags & SEL_CDATA_SIMPLESUBEXPR)
                       || (sel->cdata->flags & SEL_CDATA_FULLEVAL)))
          {
 +            assert(sel->child);
              sel->cdata->gmin = sel->child->cdata->gmin;
              sel->cdata->gmax = sel->child->cdata->gmax;
          }
@@@ -1629,14 -1655,14 +1667,14 @@@ make_static(t_selelem *sel
      {
          if (sel->child->child->flags & SEL_ALLOCDATA)
          {
 -            sel->flags |= SEL_ALLOCDATA;
 +            sel->flags               |= SEL_ALLOCDATA;
              sel->child->child->flags &= ~SEL_ALLOCDATA;
          }
          if (sel->child->child->flags & SEL_ALLOCVAL)
          {
 -            sel->flags |= SEL_ALLOCVAL;
 -            sel->v.nalloc = sel->child->child->v.nalloc;
 -            sel->child->child->flags &= ~SEL_ALLOCVAL;
 +            sel->flags                 |= SEL_ALLOCVAL;
 +            sel->v.nalloc               = sel->child->child->v.nalloc;
 +            sel->child->child->flags   &= ~SEL_ALLOCVAL;
              sel->child->child->v.nalloc = -1;
          }
      }
      if (sel->type == SEL_SUBEXPRREF && sel->u.param)
      {
          sel->u.param->val.nalloc = sel->v.nalloc;
 -        sel->flags &= ~(SEL_ALLOCVAL | SEL_ALLOCDATA);
 -        sel->v.nalloc = -1;
 +        sel->flags              &= ~(SEL_ALLOCVAL | SEL_ALLOCDATA);
 +        sel->v.nalloc            = -1;
      }
      /* Free the children. */
      release_subexpr_memory(sel);
@@@ -1744,13 -1770,13 +1782,13 @@@ store_param_val(t_selelem *sel
  static int
  init_method(t_selelem *sel, t_topology *top, int isize)
  {
 -    t_selelem *child;
 +    t_selelem     *child;
      gmx_bool       bAtomVal;
 -    int        rc;
 +    int            rc;
  
      /* Find out whether there are any atom-valued parameters */
      bAtomVal = FALSE;
 -    child = sel->child;
 +    child    = sel->child;
      while (child)
      {
          if (child->flags & SEL_ATOMVAL)
          && (bAtomVal || !(sel->flags & SEL_METHODINIT)))
      {
          sel->flags |= SEL_METHODINIT;
 -        rc = sel->u.expr.method->init(top, sel->u.expr.method->nparams,
 -                sel->u.expr.method->param, sel->u.expr.mdata);
 +        rc          = sel->u.expr.method->init(top, sel->u.expr.method->nparams,
 +                                               sel->u.expr.method->param, sel->u.expr.mdata);
          if (rc != 0)
          {
              return rc;
   */
  static int
  evaluate_gmx_boolean_static_part(gmx_sel_evaluate_t *data, t_selelem *sel,
 -                             gmx_ana_index_t *g)
 +                                 gmx_ana_index_t *g)
  {
      t_selelem *child, *next;
      int        rc;
      /* Evalute the static part if there is more than one expression */
      if (child != sel->child)
      {
 -        next  = child->next;
 +        next        = child->next;
          child->next = NULL;
 -        rc = sel->cdata->evaluate(data, sel, g);
 +        rc          = sel->cdata->evaluate(data, sel, g);
          if (rc != 0)
          {
              return rc;
          init_item_minmax_groups(child);
          child->cdata->flags &= ~SEL_CDATA_STATICEVAL;
          child->cdata->flags |= sel->cdata->flags & SEL_CDATA_STATICEVAL;
 -        child->next = next;
 -        sel->child = child;
 +        child->next          = next;
 +        sel->child           = child;
      }
      else if (child->evaluate)
      {
   */
  static void
  evaluate_gmx_boolean_minmax_grps(t_selelem *sel, gmx_ana_index_t *g,
 -                             gmx_ana_index_t *gmin, gmx_ana_index_t *gmax)
 +                                 gmx_ana_index_t *gmin, gmx_ana_index_t *gmax)
  {
      t_selelem *child;
  
  
  /*! \brief
   * Evaluates the static parts of \p sel and analyzes the structure.
 - * 
 + *
   * \param[in]     data Evaluation data.
   * \param[in,out] sel  Selection currently being evaluated.
   * \param[in]     g    Group for which \p sel should be evaluated.
   * It does the single most complex task in the compiler: after all elements
   * have been processed, the \p gmin and \p gmax fields of \p t_compiler_data
   * have been properly initialized, enough memory has been allocated for
 - * storing the value of each expression, and the static parts of the 
 + * storing the value of each expression, and the static parts of the
   * expressions have been evaluated.
   * The above is exactly true only for elements other than subexpressions:
   * another pass is required for subexpressions that are referred to more than
  static int
  analyze_static(gmx_sel_evaluate_t *data, t_selelem *sel, gmx_ana_index_t *g)
  {
 -    t_selelem       *child, *next;
 +    t_selelem           *child, *next;
      gmx_bool             bDoMinMax;
 -    int              rc;
 +    int                  rc;
  
      if (sel->type != SEL_ROOT && g)
      {
  
                  /* Evaluate minimal and maximal selections */
                  evaluate_gmx_boolean_minmax_grps(sel, g, sel->cdata->gmin,
 -                                             sel->cdata->gmax);
 +                                                 sel->cdata->gmax);
              }
              break;
  
              break;
  
          case SEL_SUBEXPR:
-             if (sel->cdata->flags & (SEL_CDATA_SIMPLESUBEXPR | SEL_CDATA_FULLEVAL))
+             if (((sel->cdata->flags & SEL_CDATA_SIMPLESUBEXPR) &&
+                  !(sel->cdata->flags & SEL_CDATA_STATICMULTIEVALSUBEXPR))
+                 || (sel->cdata->flags & SEL_CDATA_FULLEVAL))
              {
                  rc = sel->cdata->evaluate(data, sel, g);
                  _gmx_selvalue_setstore(&sel->v, sel->child->v.u.ptr);
              }
              else if (sel->u.cgrp.isize == 0)
              {
 +                assert(g);
                  gmx_ana_index_reserve(&sel->u.cgrp, g->isize);
                  rc = sel->cdata->evaluate(data, sel, g);
                  if (bDoMinMax)
@@@ -2424,9 -2451,6 +2464,9 @@@ init_root_item(t_selelem *root, gmx_ana
  static void
  postprocess_item_subexpressions(t_selelem *sel)
  {
 +    assert(!(sel->child == NULL &&
 +             (sel->type == SEL_SUBEXPRREF || sel->type == SEL_SUBEXPR)));
 +
      /* Process children. */
      if (sel->type != SEL_SUBEXPRREF)
      {
          gmx_ana_index_deinit(&sel->u.cgrp);
          sel->u.cgrp.name = name;
  
 -        sel->evaluate = &_gmx_sel_evaluate_subexpr_staticeval;
 -        if (sel->cdata)
 -        {
 -            sel->cdata->evaluate = sel->evaluate;
 -        }
 +        sel->evaluate        = &_gmx_sel_evaluate_subexpr_staticeval;
 +        sel->cdata->evaluate = sel->evaluate;
 +
          _gmx_selelem_free_values(sel->child);
          sel->child->mempool = NULL;
          _gmx_selvalue_setstore(&sel->child->v, sel->v.u.ptr);
      {
          if (sel->child->child->flags & SEL_ALLOCVAL)
          {
 -            sel->flags |= SEL_ALLOCVAL;
 -            sel->flags |= (sel->child->child->flags & SEL_ALLOCDATA);
 -            sel->v.nalloc = sel->child->child->v.nalloc;
 -            sel->child->child->flags &= ~(SEL_ALLOCVAL | SEL_ALLOCDATA);
 +            sel->flags                 |= SEL_ALLOCVAL;
 +            sel->flags                 |= (sel->child->child->flags & SEL_ALLOCDATA);
 +            sel->v.nalloc               = sel->child->child->v.nalloc;
 +            sel->child->child->flags   &= ~(SEL_ALLOCVAL | SEL_ALLOCDATA);
              sel->child->child->v.nalloc = -1;
          }
      }
          && !(sel->cdata->flags & SEL_CDATA_SIMPLESUBEXPR)
          && (sel->cdata->flags & SEL_CDATA_FULLEVAL))
      {
 -        sel->flags |= SEL_ALLOCVAL;
 -        sel->flags |= (sel->child->flags & SEL_ALLOCDATA);
 -        sel->v.nalloc = sel->child->v.nalloc;
 -        sel->child->flags &= ~(SEL_ALLOCVAL | SEL_ALLOCDATA);
 +        sel->flags          |= SEL_ALLOCVAL;
 +        sel->flags          |= (sel->child->flags & SEL_ALLOCDATA);
 +        sel->v.nalloc        = sel->child->v.nalloc;
 +        sel->child->flags   &= ~(SEL_ALLOCVAL | SEL_ALLOCDATA);
          sel->child->v.nalloc = -1;
      }
+     /* For static subexpressions with a dynamic evaluation group, there is
+      * no need to evaluate them again, as the SEL_SUBEXPRREF takes care of
+      * everything during evaluation. */
+     if (sel->type == SEL_SUBEXPR
+         && (sel->cdata->flags & SEL_CDATA_SIMPLESUBEXPR)
+         && (sel->cdata->flags & SEL_CDATA_STATICMULTIEVALSUBEXPR))
+     {
+         sel->evaluate        = NULL;
+         sel->cdata->evaluate = NULL;
+     }
  }
  
  
@@@ -2539,7 -2576,7 +2590,7 @@@ init_item_comg(t_selelem *sel, gmx_ana_
              if (!sel->u.expr.pc)
              {
                  cflags |= flags;
 -                rc = gmx_ana_poscalc_create(&sel->u.expr.pc, pcc, type, cflags);
 +                rc      = gmx_ana_poscalc_create(&sel->u.expr.pc, pcc, type, cflags);
                  if (rc != 0)
                  {
                      return rc;
  gmx_ana_selcollection_compile(gmx_ana_selcollection_t *sc)
  {
      gmx_sel_evaluate_t  evaldata;
 -    t_selelem   *item;
 -    e_poscalc_t  post;
 -    int          flags;
 -    int          rc;
 +    t_selelem          *item;
 +    e_poscalc_t         post;
 +    int                 flags;
 +    int                 rc;
  
      rc = _gmx_sel_mempool_create(&sc->mempool);
      if (rc != 0)
              /* FIXME: Clean up the collection */
              return -1;
          }
-         /* Initialize evaluation function. */
-         if (!init_item_evalfunc(item))
-         {
-             /* FIXME: Clean up the collection */
-             return -1;
-         }
-         setup_memory_pooling(item, sc->mempool);
          /* Initialize the compiler data */
          init_item_compilerdata(item);
+         item = item->next;
+     }
+     /* Initialize the static evaluation compiler flags.
+      * Requires the FULLEVAL compiler flag for the whole tree. */
+     item = sc->root;
+     while (item)
+     {
          init_item_staticeval(item);
          item = item->next;
      }
-     /* Initialize subexpression flags and evaluation output.
+     /* Initialize subexpression flags.
       * Requires compiler flags for the full tree. */
      item = sc->root;
      while (item)
      {
          init_item_subexpr_flags(item);
+         item = item->next;
+     }
+     /* Initialize evaluation.
+      * Requires subexpression flags. */
+     item = sc->root;
+     while (item)
+     {
+         if (!init_item_evalfunc(item))
+         {
+             /* FIXME: Clean up the collection */
+             return -1;
+         }
+         setup_memory_pooling(item, sc->mempool);
          init_item_evaloutput(item);
          item = item->next;
      }
       * compilation. */
      /* By default, use whole residues/molecules. */
      flags = POS_COMPLWHOLE;
 -    rc = gmx_ana_poscalc_type_from_enum(sc->rpost, &post, &flags);
 +    rc    = gmx_ana_poscalc_type_from_enum(sc->rpost, &post, &flags);
      if (rc != 0)
      {
          gmx_bug("invalid default reference position type");
index b77a93eff301999a6eb7d20ab5b2af6d9fe43495,c8bee7156f955ac1ee886cf9e3cb076e54e2a0af..bd09533686c037012d9f1858862fac8d007b19af
@@@ -1,39 -1,32 +1,39 @@@
  /*
 + * This file is part of the GROMACS molecular simulation package.
   *
 - *                This source code is part of
 - *
 - *                 G   R   O   M   A   C   S
 - *
 - *          GROningen MAchine for Chemical Simulations
 - *
 - * Written by David van der Spoel, Erik Lindahl, Berk Hess, and others.
   * Copyright (c) 1991-2000, University of Groningen, The Netherlands.
   * Copyright (c) 2001-2009, The GROMACS development team,
   * check out http://www.gromacs.org for more information.
 -
 - * This program is free software; you can redistribute it and/or
 - * modify it under the terms of the GNU General Public License
 - * as published by the Free Software Foundation; either version 2
 + * Copyright (c) 2012,2013, by the GROMACS development team, led by
 + * David van der Spoel, Berk Hess, Erik Lindahl, and including many
 + * others, as listed in the AUTHORS file in the top-level source
 + * directory and at http://www.gromacs.org.
 + *
 + * GROMACS is free software; you can redistribute it and/or
 + * modify it under the terms of the GNU Lesser General Public License
 + * as published by the Free Software Foundation; either version 2.1
   * of the License, or (at your option) any later version.
   *
 - * If you want to redistribute modifications, please consider that
 - * scientific software is very special. Version control is crucial -
 - * bugs must be traceable. We will be happy to consider code for
 - * inclusion in the official distribution, but derived work must not
 - * be called official GROMACS. Details are found in the README & COPYING
 - * files - if they are missing, get the official version at www.gromacs.org.
 + * GROMACS is distributed in the hope that it will be useful,
 + * but WITHOUT ANY WARRANTY; without even the implied warranty of
 + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
 + * Lesser General Public License for more details.
   *
 - * To help us fund GROMACS development, we humbly ask that you cite
 - * the papers on the package - you can find them in the top README file.
 + * You should have received a copy of the GNU Lesser General Public
 + * License along with GROMACS; if not, see
 + * http://www.gnu.org/licenses, or write to the Free Software Foundation,
 + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  USA.
   *
 - * For more info, check our website at http://www.gromacs.org
 + * If you want to redistribute modifications to GROMACS, please
 + * consider that scientific software is very special. Version
 + * control is crucial - bugs must be traceable. We will be happy to
 + * consider code for inclusion in the official distribution, but
 + * derived work must not be called official GROMACS. Details are found
 + * in the README & COPYING files - if they are missing, get the
 + * official version at http://www.gromacs.org.
 + *
 + * To help us fund GROMACS development, we humbly ask that you cite
 + * the research papers on the package. Check out http://www.gromacs.org.
   */
  /*! \internal \file
   * \brief Implementation of functions in evaluate.h.
@@@ -58,7 -51,6 +58,7 @@@
  #include <maths.h>
  #include <smalloc.h>
  #include <vec.h>
 +#include <assert.h>
  
  #include <indexutil.h>
  #include <poscalc.h>
@@@ -78,65 -70,35 +78,65 @@@ voi
  _gmx_sel_print_evalfunc_name(FILE *fp, sel_evalfunc evalfunc)
  {
      if (!evalfunc)
 +    {
          fprintf(fp, "none");
 +    }
      else if (evalfunc == &_gmx_sel_evaluate_root)
 +    {
          fprintf(fp, "root");
 +    }
      else if (evalfunc == &_gmx_sel_evaluate_static)
 +    {
          fprintf(fp, "static");
 +    }
      else if (evalfunc == &_gmx_sel_evaluate_subexpr_simple)
 +    {
          fprintf(fp, "subexpr_simple");
 +    }
      else if (evalfunc == &_gmx_sel_evaluate_subexpr_staticeval)
 +    {
          fprintf(fp, "subexpr_staticeval");
 +    }
      else if (evalfunc == &_gmx_sel_evaluate_subexpr)
 +    {
          fprintf(fp, "subexpr");
 +    }
      else if (evalfunc == &_gmx_sel_evaluate_subexprref_simple)
 +    {
          fprintf(fp, "ref_simple");
 +    }
      else if (evalfunc == &_gmx_sel_evaluate_subexprref)
 +    {
          fprintf(fp, "ref");
 +    }
      else if (evalfunc == &_gmx_sel_evaluate_method)
 +    {
          fprintf(fp, "method");
 +    }
      else if (evalfunc == &_gmx_sel_evaluate_modifier)
 +    {
          fprintf(fp, "mod");
 +    }
      else if (evalfunc == &_gmx_sel_evaluate_not)
 +    {
          fprintf(fp, "not");
 +    }
      else if (evalfunc == &_gmx_sel_evaluate_and)
 +    {
          fprintf(fp, "and");
 +    }
      else if (evalfunc == &_gmx_sel_evaluate_or)
 +    {
          fprintf(fp, "or");
 +    }
      else if (evalfunc == &_gmx_sel_evaluate_arithmetic)
 +    {
          fprintf(fp, "arithmetic");
 +    }
      else
 +    {
          fprintf(fp, "%p", (void*)(evalfunc));
 +    }
  }
  
  /*!
@@@ -220,7 -182,8 +220,8 @@@ gmx_ana_selcollection_evaluate(gmx_ana_
      while (sel)
      {
          /* Clear the evaluation group of subexpressions */
-         if (sel->child && sel->child->type == SEL_SUBEXPR)
+         if (sel->child && sel->child->type == SEL_SUBEXPR
+             && sel->child->evaluate != NULL)
          {
              sel->child->u.cgrp.isize = 0;
              /* Not strictly necessary, because the value will be overwritten
          }
          if (sel->bCFracDyn)
          {
 -            sel->cfrac = _gmx_selelem_estimate_coverfrac(sel->selelem);
 +            sel->cfrac     = _gmx_selelem_estimate_coverfrac(sel->selelem);
              sel->avecfrac += sel->cfrac;
          }
      }
@@@ -496,7 -459,7 +497,7 @@@ _gmx_sel_evaluate_subexpr(gmx_sel_evalu
          name = sel->u.cgrp.name;
          gmx_ana_index_copy(&sel->u.cgrp, g, FALSE);
          sel->u.cgrp.name = name;
 -        gmiss.isize = 0;
 +        gmiss.isize      = 0;
      }
      else
      {
@@@ -664,7 -627,7 +665,7 @@@ _gmx_sel_evaluate_subexprref(gmx_sel_ev
      t_selelem *expr;
      int        i, j;
  
-     if (g)
+     if (g != NULL && sel->child->evaluate != NULL)
      {
          int rc;
  
@@@ -807,7 -770,7 +808,7 @@@ _gmx_sel_evaluate_method_params(gmx_sel
              }
              else
              {
 -                rc = child->evaluate(data, child, NULL);
 +                rc            = child->evaluate(data, child, NULL);
                  child->flags |= SEL_EVALFRAME;
              }
              if (rc != 0)
@@@ -914,8 -877,8 +915,8 @@@ _gmx_sel_evaluate_modifier(gmx_sel_eval
          return -1;
      }
      rc = sel->u.expr.method->pupdate(data->top, data->fr, data->pbc,
 -                                    sel->child->v.u.p,
 -                                    &sel->v, sel->u.expr.mdata);
 +                                     sel->child->v.u.p,
 +                                     &sel->v, sel->u.expr.mdata);
      return rc;
  }
  
   * \returns   0 on success, a non-zero error code on error.
   *
   * Evaluates the child element (there should be only one) in the group
 - * \p g, and then sets the value of \p sel to the complement of the 
 + * \p g, and then sets the value of \p sel to the complement of the
   * child value.
   *
   * This function is used as \c t_selelem::evaluate for \ref SEL_BOOLEAN
@@@ -1052,7 -1015,7 +1053,7 @@@ _gmx_sel_evaluate_and(gmx_sel_evaluate_
  int
  _gmx_sel_evaluate_or(gmx_sel_evaluate_t *data, t_selelem *sel, gmx_ana_index_t *g)
  {
 -    t_selelem     *child;
 +    t_selelem       *child;
      gmx_ana_index_t  tmp, tmp2;
      int              rc;
  
      while (child && tmp.isize > 0)
      {
          tmp.name = NULL;
 -        rc = _gmx_selelem_mempool_reserve(child, tmp.isize);
 +        rc       = _gmx_selelem_mempool_reserve(child, tmp.isize);
          if (rc == 0)
          {
              rc = child->evaluate(data, child, &tmp);
          gmx_ana_index_partition(&tmp, &tmp2, &tmp, child->v.u.g);
          _gmx_selelem_mempool_release(child);
          sel->v.u.g->isize += tmp.isize;
 -        tmp.isize = tmp2.isize;
 -        tmp.index = tmp2.index;
 -        child = child->next;
 +        tmp.isize          = tmp2.isize;
 +        tmp.index          = tmp2.index;
 +        child              = child->next;
      }
      gmx_ana_index_sort(sel->v.u.g);
      return 0;
  _gmx_sel_evaluate_arithmetic(gmx_sel_evaluate_t *data, t_selelem *sel,
                               gmx_ana_index_t *g)
  {
 -    t_selelem  *left, *right;
 -    int         n, i, i1, i2;
 -    real        lval, rval=0., val=0.;
 -    int         rc;
 +    int               n, i, i1, i2;
 +    real              lval, rval = 0., val = 0.;
 +    int               rc;
 +    gmx_bool          bArithNeg;
  
 -    left  = sel->child;
 -    right = left->next;
 +    t_selelem  *const left  = sel->child;
 +    t_selelem  *const right = left->next;
  
      if (left->mempool)
      {
      }
      rc = _gmx_sel_evaluate_children(data, sel, g);
  
 -    n = (sel->flags & SEL_SINGLEVAL) ? 1 : g->isize;
 +    n         = (sel->flags & SEL_SINGLEVAL) ? 1 : g->isize;
      sel->v.nr = n;
 +
 +    bArithNeg = (sel->u.arith.type == ARITH_NEG);
 +    assert(right || bArithNeg);
      for (i = i1 = i2 = 0; i < n; ++i)
      {
          lval = left->v.u.r[i1];
 -        if (sel->u.arith.type != ARITH_NEG)
 +        if (!bArithNeg)
          {
              rval = right->v.u.r[i2];
          }
          {
              ++i1;
          }
 -        if (sel->u.arith.type != ARITH_NEG && !(right->flags & SEL_SINGLEVAL))
 +        if (!bArithNeg && !(right->flags & SEL_SINGLEVAL))
          {
              ++i2;
          }