Fixes for numeric and position selection variables.
authorTeemu Murtola <teemu.murtola@gmail.com>
Tue, 18 Sep 2012 05:11:47 +0000 (08:11 +0300)
committerTeemu Murtola <teemu.murtola@gmail.com>
Tue, 18 Sep 2012 18:37:04 +0000 (21:37 +0300)
Fixed handling of numeric and position selection variables in cases
where they were referenced multiple times.

Backported from Ifbb585ca.

Fixes #1006.

Change-Id: Ieaa479b4b870e044dc269339be57f6b33533c46e

src/gmxlib/selection/compiler.c
src/gmxlib/selection/evaluate.c
src/gmxlib/selection/selmethod.c
src/gmxlib/selection/sm_keywords.c

index 8ecc60d4ed7825c626b4bddf8b047ec9197e80b5..1b5e664d37441b9e7f217b74a8d103376293ef2d 100644 (file)
@@ -442,6 +442,9 @@ _gmx_selelem_free_compiler_data(t_selelem *sel)
  *
  * If called more than once, memory is (re)allocated to ensure that the
  * maximum of the \p isize values can be stored.
+ *
+ * Allocation of POS_VALUE selection elements is a special case, and is
+ * handled by alloc_selection_pos_data().
  */
 static gmx_bool
 alloc_selection_data(t_selelem *sel, int isize, gmx_bool bChildEval)
@@ -474,36 +477,62 @@ alloc_selection_data(t_selelem *sel, int isize, gmx_bool bChildEval)
         {
             child = child->child;
         }
-        nalloc = (sel->v.type == POS_VALUE) ? child->v.u.p->nr : child->v.nr;
-    }
-    /* For positions, we actually want to allocate just a single structure
-     * for nalloc positions. */
-    if (sel->v.type == POS_VALUE)
-    {
-        isize  = nalloc;
-        nalloc = 1;
+        nalloc = child->v.nr;
     }
     /* Allocate memory for sel->v.u if needed */
     if (sel->flags & SEL_ALLOCVAL)
     {
         _gmx_selvalue_reserve(&sel->v, nalloc);
     }
-    /* Reserve memory inside group and position structures if
-     * SEL_ALLOCDATA is set. */
-    if (sel->flags & SEL_ALLOCDATA)
+    /* Reserve memory inside group structure if SEL_ALLOCDATA is set. */
+    if ((sel->flags & SEL_ALLOCDATA) && sel->v.type == GROUP_VALUE)
     {
-        if (sel->v.type == GROUP_VALUE)
-        {
-            gmx_ana_index_reserve(sel->v.u.g, isize);
-        }
-        else if (sel->v.type == POS_VALUE)
-        {
-            gmx_ana_pos_reserve(sel->v.u.p, isize, 0);
-        }
+        gmx_ana_index_reserve(sel->v.u.g, isize);
     }
     return TRUE;
 }
 
+/*! \brief
+ * Allocates memory for storing the evaluated value of a selection element.
+ *
+ * \param     sel   Selection element to initialize.
+ *
+ * Allocation of POS_VALUE selection elements is a special case, and is
+ * handled by this function instead of by alloc_selection_data().
+ */
+static void
+alloc_selection_pos_data(t_selelem *sel)
+{
+    t_selelem *child;
+    int        nalloc, isize;
+
+    if (sel->mempool)
+    {
+        return;
+    }
+
+    child = sel;
+    if (sel->type == SEL_SUBEXPRREF)
+    {
+        child = sel->child->child;
+    }
+    nalloc = child->v.u.p->nr;
+    isize = child->v.u.p->m.b.nra;
+
+    /* For positions, we want to allocate just a single structure
+     * for nalloc positions. */
+    if (sel->flags & SEL_ALLOCVAL)
+    {
+        _gmx_selvalue_reserve(&sel->v, 1);
+    }
+    sel->v.nr = 1;
+    /* Reserve memory inside position structure if SEL_ALLOCDATA is set. */
+    if (sel->flags & SEL_ALLOCDATA)
+    {
+        gmx_ana_pos_reserve(sel->v.u.p, nalloc, isize);
+    }
+}
+
 /*! \brief
  * Replace the evaluation function of each element in the subtree.
  *
@@ -1461,8 +1490,9 @@ init_item_minmax_groups(t_selelem *sel)
  * \param[in,out] sc   Selection collection data.
  *
  * The evaluation group of each \ref SEL_ROOT element corresponding to a
- * selection in \p sc is set to \p gall.  The same is done for \ref SEL_ROOT
- * elements corresponding to subexpressions that need full evaluation.
+ * selection in \p sc is set to NULL.  The evaluation grop for \ref SEL_ROOT
+ * elements corresponding to subexpressions that need full evaluation is set
+ * to \c sc->gall.
  */
 static void
 initialize_evalgrps(gmx_ana_selcollection_t *sc)
@@ -1473,7 +1503,11 @@ initialize_evalgrps(gmx_ana_selcollection_t *sc)
     while (root)
     {
         if (root->child->type != SEL_SUBEXPR
-            || (root->child->cdata->flags & SEL_CDATA_FULLEVAL))
+            || (root->child->v.type != GROUP_VALUE && !(root->flags & SEL_ATOMVAL)))
+        {
+            gmx_ana_index_set(&root->u.cgrp, -1, 0, root->u.cgrp.name, 0);
+        }
+        else if (root->child->cdata->flags & SEL_CDATA_FULLEVAL)
         {
             gmx_ana_index_set(&root->u.cgrp, sc->gall.isize, sc->gall.index,
                               root->u.cgrp.name, 0);
@@ -1736,7 +1770,8 @@ init_method(t_selelem *sel, t_topology *top, int isize)
             {
                 return rc;
             }
-            if (sel->v.type != POS_VALUE && sel->v.type != GROUP_VALUE)
+            if (sel->v.type != POS_VALUE && sel->v.type != GROUP_VALUE
+                && !(sel->flags & SEL_VARNUMVAL))
             {
                 alloc_selection_data(sel, isize, TRUE);
             }
@@ -2039,7 +2074,7 @@ analyze_static(gmx_sel_evaluate_t *data, t_selelem *sel, gmx_ana_index_t *g)
             {
                 return rc;
             }
-            rc = init_method(sel, data->top, g->isize);
+            rc = init_method(sel, data->top, g ? g->isize : 0);
             if (rc != 0)
             {
                 return rc;
@@ -2061,7 +2096,7 @@ analyze_static(gmx_sel_evaluate_t *data, t_selelem *sel, gmx_ana_index_t *g)
                 {
                     rc = sel->cdata->evaluate(data, sel, g);
                 }
-                if (bDoMinMax)
+                if (bDoMinMax && g)
                 {
                     gmx_ana_index_copy(sel->cdata->gmax, g, TRUE);
                 }
@@ -2180,7 +2215,14 @@ analyze_static(gmx_sel_evaluate_t *data, t_selelem *sel, gmx_ana_index_t *g)
                 /* The subexpression should have been evaluated if g is NULL
                  * (i.e., this is a method parameter or a direct value of a
                  * selection). */
-                alloc_selection_data(sel, sel->child->cdata->gmax->isize, TRUE);
+                if (sel->v.type == POS_VALUE)
+                {
+                    alloc_selection_pos_data(sel);
+                }
+                else
+                {
+                    alloc_selection_data(sel, sel->child->cdata->gmax->isize, TRUE);
+                }
             }
             rc = sel->cdata->evaluate(data, sel, g);
             if (rc != 0)
index 9fbe3c064fd6872f689514f905c8f9e9449e9eff..1913adad26fda89ea17d07a61a697e07dcbe1fea 100644 (file)
@@ -403,7 +403,14 @@ _gmx_sel_evaluate_subexpr_staticeval(gmx_sel_evaluate_t *data, t_selelem *sel, g
             return rc;
         }
         sel->v.nr = sel->child->v.nr;
-        gmx_ana_index_set(&sel->u.cgrp, g->isize, g->index, sel->u.cgrp.name, 0);
+        if (!g)
+        {
+            sel->u.cgrp.isize = -1;
+        }
+        else
+        {
+            gmx_ana_index_set(&sel->u.cgrp, g->isize, g->index, sel->u.cgrp.name, 0);
+        }
     }
     return 0;
 }
@@ -466,6 +473,7 @@ _gmx_sel_evaluate_subexpr(gmx_sel_evaluate_t *data, t_selelem *sel, gmx_ana_inde
         {
             _gmx_sel_mempool_free_group(data->mp, &gmiss);
         }
+        gmiss.name = NULL;
     }
     if (gmiss.isize > 0)
     {
@@ -500,11 +508,11 @@ _gmx_sel_evaluate_subexpr(gmx_sel_evaluate_t *data, t_selelem *sel, gmx_ana_inde
                     {
                         if (i < 0 || (j >= 0 && sel->u.cgrp.index[i] < gmiss.index[j]))
                         {
-                            sel->v.u.i[k] = sel->v.u.i[j--];
+                            sel->v.u.i[k] = sel->child->v.u.i[j--];
                         }
                         else
                         {
-                            sel->v.u.i[k] = sel->child->v.u.i[i--];
+                            sel->v.u.i[k] = sel->v.u.i[i--];
                         }
                     }
                     break;
@@ -514,11 +522,11 @@ _gmx_sel_evaluate_subexpr(gmx_sel_evaluate_t *data, t_selelem *sel, gmx_ana_inde
                     {
                         if (i < 0 || (j >= 0 && sel->u.cgrp.index[i] < gmiss.index[j]))
                         {
-                            sel->v.u.r[k] = sel->v.u.r[j--];
+                            sel->v.u.r[k] = sel->child->v.u.r[j--];
                         }
                         else
                         {
-                            sel->v.u.r[k] = sel->child->v.u.r[i--];
+                            sel->v.u.r[k] = sel->v.u.r[i--];
                         }
                     }
                     break;
@@ -528,11 +536,11 @@ _gmx_sel_evaluate_subexpr(gmx_sel_evaluate_t *data, t_selelem *sel, gmx_ana_inde
                     {
                         if (i < 0 || (j >= 0 && sel->u.cgrp.index[i] < gmiss.index[j]))
                         {
-                            sel->v.u.s[k] = sel->v.u.s[j--];
+                            sel->v.u.s[k] = sel->child->v.u.s[j--];
                         }
                         else
                         {
-                            sel->v.u.s[k] = sel->child->v.u.s[i--];
+                            sel->v.u.s[k] = sel->v.u.s[i--];
                         }
                     }
                     break;
index 982eaed6efe2cc8a7ba03ecab341f9b4867a83c1..5d832fd3cc7f88d93051a56fbafdf19b5588aa55 100644 (file)
@@ -436,6 +436,12 @@ check_callbacks(FILE *fp, gmx_ana_selmethod_t *method)
         report_error(fp, method->name, "error: outinit should be provided because the method has POS_VALUE");
         bOk = FALSE;
     }
+    /* Check presence of outinit for variable output count methods */
+    if ((method->flags & SMETH_VARNUMVAL) && !method->outinit)
+    {
+        report_error(fp, method->name, "error: outinit should be provided because the method has SMETH_VARNUMVAL");
+        bOk = FALSE;
+    }
     /* Warn of dynamic callbacks in static methods */
     if (!(method->flags & SMETH_MODIFIER))
     {
index afcb2e36a6f356bf4c5a55a75e4ed89301a3321f..0dbec1921fdfd6acd39417c6a1692b73d6f3e184 100644 (file)
@@ -629,6 +629,7 @@ init_output_kweval(t_topology *top, gmx_ana_selvalue_t *out, void *data)
     t_methoddata_kweval *d = (t_methoddata_kweval *)data;
 
     out->nr = d->g.isize;
+    _gmx_selvalue_reserve(out, out->nr);
     return 0;
 }