Add unit tests for index groups in selections.
authorTeemu Murtola <teemu.murtola@gmail.com>
Wed, 1 May 2013 17:55:45 +0000 (20:55 +0300)
committerGerrit Code Review <gerrit@gerrit.gromacs.org>
Mon, 5 Aug 2013 04:16:59 +0000 (06:16 +0200)
Add tests for using the "group" keyword in selections to select external
index groups, and in particular for directly providing an index group as
the selection value.

Fixed a master-only issue in handling the selection name in the latter
cases.  Also fixed master-only memory leaks and other issues in
gmx_ana_indexgrps_init().

Part of #651.

Change-Id: I0d908213f109ced9ab6708e55c6acf53371f6aa1

15 files changed:
src/gromacs/selection/indexutil.cpp
src/gromacs/selection/parser.cpp
src/gromacs/selection/parser.y
src/gromacs/selection/parsetree.cpp
src/gromacs/selection/selection.cpp
src/gromacs/selection/selection.h
src/gromacs/selection/selectioncollection.cpp
src/gromacs/selection/selelem.cpp
src/gromacs/selection/selelem.h
src/gromacs/selection/tests/refdata/SelectionCollectionDataTest_HandlesIndexGroupsInSelections.xml [new file with mode: 0644]
src/gromacs/selection/tests/refdata/SelectionCollectionDataTest_HandlesIndexGroupsInSelectionsDelayed.xml [new file with mode: 0644]
src/gromacs/selection/tests/refdata/SelectionCollectionDataTest_HandlesSelectionNames.xml [new file with mode: 0644]
src/gromacs/selection/tests/refdata/referencedata.xsl
src/gromacs/selection/tests/selectioncollection.cpp
src/gromacs/selection/tests/simple.ndx [new file with mode: 0644]

index 819340f2a8f35a5259d091b32d15d3c4f3b0cd67..ad640ae8173150bd91a0925c9076153f7bd74973 100644 (file)
@@ -138,22 +138,22 @@ gmx_ana_indexgrps_init(gmx_ana_indexgrps_t **g, t_topology *top,
     }
     catch (...)
     {
-        done_blocka(block);
-        sfree(block);
         for (int i = 0; i < block->nr; ++i)
         {
             sfree(names[i]);
         }
         sfree(names);
+        done_blocka(block);
+        sfree(block);
         throw;
     }
-    done_blocka(block);
-    sfree(block);
     for (int i = 0; i < block->nr; ++i)
     {
         sfree(names[i]);
     }
     sfree(names);
+    done_blocka(block);
+    sfree(block);
 }
 
 /*!
index 9f627a93983d0692ed8efc3f34b34cb02212d3f6..5986ba6eb7a5be5c43c0c668bb28681fd1f7b343 100644 (file)
@@ -1997,7 +1997,7 @@ yyreduce:
                  SelectionTreeElementPointer p
                         = _gmx_sel_init_position(s, NULL, scanner);
                  if (!p) YYERROR;
-                 set((yyval.sel), _gmx_sel_init_selection(s->name().c_str(), p, scanner));
+                 set((yyval.sel), _gmx_sel_init_selection(NULL, p, scanner));
                  END_ACTION;
              }
     break;
@@ -2014,7 +2014,7 @@ yyreduce:
                  SelectionTreeElementPointer p
                         = _gmx_sel_init_position(s, NULL, scanner);
                  if (!p) YYERROR;
-                 set((yyval.sel), _gmx_sel_init_selection(s->name().c_str(), p, scanner));
+                 set((yyval.sel), _gmx_sel_init_selection(NULL, p, scanner));
                  END_ACTION;
              }
     break;
index 757ce0cbbb8ffd8352431eaf82653a8eb7562d7f..dd5f95db1cda08607c442278da4abbb7ae1c8016 100644 (file)
@@ -256,7 +256,7 @@ cmd_plain:   /* empty */
                  SelectionTreeElementPointer p
                         = _gmx_sel_init_position(s, NULL, scanner);
                  if (!p) YYERROR;
-                 set($$, _gmx_sel_init_selection(s->name().c_str(), p, scanner));
+                 set($$, _gmx_sel_init_selection(NULL, p, scanner));
                  END_ACTION;
              }
            | string
@@ -269,7 +269,7 @@ cmd_plain:   /* empty */
                  SelectionTreeElementPointer p
                         = _gmx_sel_init_position(s, NULL, scanner);
                  if (!p) YYERROR;
-                 set($$, _gmx_sel_init_selection(s->name().c_str(), p, scanner));
+                 set($$, _gmx_sel_init_selection(NULL, p, scanner));
                  END_ACTION;
              }
            | selection
index 6790798160d3bd6c0db71558c729a003841c4573..b268a57786c0fd95ebbfc9b05b0e62f8f738639d 100644 (file)
@@ -1038,35 +1038,7 @@ _gmx_sel_init_selection(const char                        *name,
     /* Update the flags */
     _gmx_selelem_update_flags(root, scanner);
 
-    /* If there is no name provided by the user, check whether the actual
-     * selection given was from an external group, and if so, use the name
-     * of the external group. */
-    if (root->name().empty())
-    {
-        SelectionTreeElementPointer child = root->child;
-        while (child->type == SEL_MODIFIER)
-        {
-            if (!child->child || child->child->type != SEL_SUBEXPRREF
-                || !child->child->child)
-            {
-                break;
-            }
-            child = child->child->child;
-        }
-        if (child->type == SEL_EXPRESSION
-            && child->child && child->child->type == SEL_SUBEXPRREF
-            && child->child->child
-            && child->child->child->type == SEL_CONST
-            && child->child->child->v.type == GROUP_VALUE)
-        {
-            root->setName(child->child->child->name());
-        }
-    }
-    /* If there still is no name, use the selection string */
-    if (root->name().empty())
-    {
-        root->setName(_gmx_sel_lexer_pselstr(scanner));
-    }
+    root->fillNameIfMissing(_gmx_sel_lexer_pselstr(scanner));
 
     /* Print out some information if the parser is interactive */
     if (_gmx_sel_is_lexer_interactive(scanner))
index 573b086ba1bae83f3d17f1ccc179cc76807ffa9f..ea476706647ade23ae96ae20b22d1d1afbb6bb39 100644 (file)
@@ -170,6 +170,13 @@ void computeMassesAndCharges(const t_topology *top, const gmx_ana_pos_t &pos,
 
 }       // namespace
 
+void
+SelectionData::refreshName()
+{
+    rootElement_.fillNameIfMissing(selectionText_.c_str());
+    name_ = rootElement_.name();
+}
+
 void
 SelectionData::initializeMassesAndCharges(const t_topology *top)
 {
index f910ab817bcb36e1d72453389240f1d25f1b1595..861f407633135c11cd760302f8ed1a7e617b600a 100644 (file)
@@ -96,6 +96,8 @@ class SelectionData
         SelectionData(SelectionTreeElement *elem, const char *selstr);
         ~SelectionData();
 
+        //! Returns the name for this selection.
+        const char *name() const { return name_.c_str(); }
         //! Returns the string that was parsed to produce this selection.
         const char *selectionText() const { return selectionText_.c_str(); }
         //! Returns true if the size of the selection (posCount()) is dynamic.
@@ -119,6 +121,18 @@ class SelectionData
         //! \copydoc Selection::initCoveredFraction()
         bool initCoveredFraction(e_coverfrac_t type);
 
+        /*! \brief
+         * Updates the name of the selection if missing.
+         *
+         * \throws    std::bad_alloc if out of memory.
+         *
+         * If selections get their value from a group reference that cannot be
+         * resolved during parsing, the name is final only after group
+         * references have been resolved.
+         *
+         * This function is called by SelectionCollection::setIndexGroups().
+         */
+        void refreshName();
         /*! \brief
          * Computes total masses and charges for all selection positions.
          *
@@ -287,7 +301,7 @@ class Selection
         explicit Selection(internal::SelectionData *sel) : sel_(sel) {}
 
         //! Returns the name of the selection.
-        const char *name() const  { return data().name_.c_str(); }
+        const char *name() const  { return data().name(); }
         //! Returns the string that was parsed to produce this selection.
         const char *selectionText() const { return data().selectionText(); }
         //! Returns true if the size of the selection (posCount()) is dynamic.
index 656ccb78c78b52bb3001f9e756460f2b03014737..71ea23a42a240197a83a38e83966d675b018bd68 100644 (file)
@@ -318,9 +318,9 @@ void SelectionCollection::Impl::resolveExternalGroups(
             char *name = root->u.gref.name;
             bOk = gmx_ana_indexgrps_find(&root->u.cgrp, &foundName, grps_, name);
             sfree(name);
-            root->u.gref.name = NULL;
             if (!bOk)
             {
+                root->u.gref.name = NULL;
                 // TODO: Improve error messages
                 errors->append("Unknown group referenced in a selection");
             }
@@ -477,6 +477,10 @@ SelectionCollection::setIndexGroups(gmx_ana_indexgrps_t *grps)
     {
         GMX_THROW(InvalidInputError(errors.toString()));
     }
+    for (size_t i = 0; i < impl_->sc_.sel.size(); ++i)
+    {
+        impl_->sc_.sel[i]->refreshName();
+    }
 }
 
 
index bc9d99388abb889c551f902b593a37ff7ec4334a..f496ea8b505f55518257503979a495b2f0136384 100644 (file)
@@ -1,7 +1,7 @@
 /*
  * This file is part of the GROMACS molecular simulation package.
  *
- * Copyright (c) 2009,2010,2011,2012, by the GROMACS development team, led by
+ * Copyright (c) 2009,2010,2011,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.
@@ -290,6 +290,46 @@ void SelectionTreeElement::mempoolRelease()
     }
 }
 
+void SelectionTreeElement::fillNameIfMissing(const char *selectionText)
+{
+    GMX_RELEASE_ASSERT(type == SEL_ROOT,
+                       "Should not be called for non-root elements");
+    if (name().empty())
+    {
+        // Check whether the actual selection given was from an external group,
+        // and if so, use the name of the external group.
+        SelectionTreeElementPointer child = this->child;
+        while (child->type == SEL_MODIFIER)
+        {
+            if (!child->child || child->child->type != SEL_SUBEXPRREF
+                || !child->child->child)
+            {
+                break;
+            }
+            child = child->child->child;
+        }
+        if (child->type == SEL_EXPRESSION
+            && child->child && child->child->type == SEL_SUBEXPRREF
+            && child->child->child)
+        {
+            if (child->child->child->type == SEL_CONST
+                && child->child->child->v.type == GROUP_VALUE)
+            {
+                setName(child->child->child->name());
+                return;
+            }
+            // If the group reference is still unresolved, leave the name empty
+            // and fill it later.
+            if (child->child->child->type == SEL_GROUPREF)
+            {
+                return;
+            }
+        }
+        // If there still is no name, use the selection string.
+        setName(selectionText);
+    }
+}
+
 } // namespace gmx
 
 /*!
index 5971c051bf09ae2de3820a30a6b4b25ea13c920e..9f4691e30194cfd525d0b6a989a12c13e40790d9 100644 (file)
@@ -1,7 +1,7 @@
 /*
  * This file is part of the GROMACS molecular simulation package.
  *
- * Copyright (c) 2009,2010,2011,2012, by the GROMACS development team, led by
+ * Copyright (c) 2009,2010,2011,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.
@@ -306,6 +306,18 @@ class SelectionTreeElement
         void setName(const char *name) { name_ = (name != NULL ? name : ""); }
         //! \copydoc setName(const char *)
         void setName(const std::string &name) { name_ = name; }
+        /*! \brief
+         * Sets the name of a root element if it is missing.
+         *
+         * \param[in] selectionText  Full selection text to use as a fallback.
+         * \throws    std::bad_alloc if out of memory.
+         *
+         * If index groups have not yet been set and the selection is a result
+         * of a group reference, the name may still be empty after this call.
+         *
+         * Strong exception safety guarantee.
+         */
+        void fillNameIfMissing(const char *selectionText);
 
         //! Type of the element.
         e_selelem_t                         type;
diff --git a/src/gromacs/selection/tests/refdata/SelectionCollectionDataTest_HandlesIndexGroupsInSelections.xml b/src/gromacs/selection/tests/refdata/SelectionCollectionDataTest_HandlesIndexGroupsInSelections.xml
new file mode 100644 (file)
index 0000000..9cd47b9
--- /dev/null
@@ -0,0 +1,74 @@
+<?xml version="1.0"?>
+<?xml-stylesheet type="text/xsl" href="referencedata.xsl"?>
+<ReferenceData>
+  <ParsedSelections Name="Parsed">
+    <ParsedSelection Name="Selection1">
+      <String Name="Input">group "GrpA"</String>
+      <String Name="Name">GrpA</String>
+      <String Name="Text">group "GrpA"</String>
+      <Bool Name="Dynamic">false</Bool>
+    </ParsedSelection>
+    <ParsedSelection Name="Selection2">
+      <String Name="Input">GrpB</String>
+      <String Name="Name">GrpB</String>
+      <String Name="Text">GrpB</String>
+      <Bool Name="Dynamic">false</Bool>
+    </ParsedSelection>
+    <ParsedSelection Name="Selection3">
+      <String Name="Input">1</String>
+      <String Name="Name">GrpB</String>
+      <String Name="Text">1</String>
+      <Bool Name="Dynamic">false</Bool>
+    </ParsedSelection>
+    <ParsedSelection Name="Selection4">
+      <String Name="Input">group "GrpB" and resname RB</String>
+      <String Name="Name">group "GrpB" and resname RB</String>
+      <String Name="Text">group "GrpB" and resname RB</String>
+      <Bool Name="Dynamic">false</Bool>
+    </ParsedSelection>
+  </ParsedSelections>
+  <CompiledSelections Name="Compiled">
+    <Selection Name="Selection1">
+      <String Name="Name">GrpA</String>
+      <Sequence Name="Atoms">
+        <Int Name="Length">5</Int>
+        <Int>0</Int>
+        <Int>1</Int>
+        <Int>2</Int>
+        <Int>3</Int>
+        <Int>4</Int>
+      </Sequence>
+    </Selection>
+    <Selection Name="Selection2">
+      <String Name="Name">GrpB</String>
+      <Sequence Name="Atoms">
+        <Int Name="Length">5</Int>
+        <Int>3</Int>
+        <Int>4</Int>
+        <Int>5</Int>
+        <Int>6</Int>
+        <Int>7</Int>
+      </Sequence>
+    </Selection>
+    <Selection Name="Selection3">
+      <String Name="Name">GrpB</String>
+      <Sequence Name="Atoms">
+        <Int Name="Length">5</Int>
+        <Int>3</Int>
+        <Int>4</Int>
+        <Int>5</Int>
+        <Int>6</Int>
+        <Int>7</Int>
+      </Sequence>
+    </Selection>
+    <Selection Name="Selection4">
+      <String Name="Name">group "GrpB" and resname RB</String>
+      <Sequence Name="Atoms">
+        <Int Name="Length">3</Int>
+        <Int>3</Int>
+        <Int>4</Int>
+        <Int>5</Int>
+      </Sequence>
+    </Selection>
+  </CompiledSelections>
+</ReferenceData>
diff --git a/src/gromacs/selection/tests/refdata/SelectionCollectionDataTest_HandlesIndexGroupsInSelectionsDelayed.xml b/src/gromacs/selection/tests/refdata/SelectionCollectionDataTest_HandlesIndexGroupsInSelectionsDelayed.xml
new file mode 100644 (file)
index 0000000..b840304
--- /dev/null
@@ -0,0 +1,74 @@
+<?xml version="1.0"?>
+<?xml-stylesheet type="text/xsl" href="referencedata.xsl"?>
+<ReferenceData>
+  <ParsedSelections Name="Parsed">
+    <ParsedSelection Name="Selection1">
+      <String Name="Input">group "GrpA"</String>
+      <String Name="Name"/>
+      <String Name="Text">group "GrpA"</String>
+      <Bool Name="Dynamic">false</Bool>
+    </ParsedSelection>
+    <ParsedSelection Name="Selection2">
+      <String Name="Input">GrpB</String>
+      <String Name="Name"/>
+      <String Name="Text">GrpB</String>
+      <Bool Name="Dynamic">false</Bool>
+    </ParsedSelection>
+    <ParsedSelection Name="Selection3">
+      <String Name="Input">1</String>
+      <String Name="Name"/>
+      <String Name="Text">1</String>
+      <Bool Name="Dynamic">false</Bool>
+    </ParsedSelection>
+    <ParsedSelection Name="Selection4">
+      <String Name="Input">group "GrpB" and resname RB</String>
+      <String Name="Name">group "GrpB" and resname RB</String>
+      <String Name="Text">group "GrpB" and resname RB</String>
+      <Bool Name="Dynamic">false</Bool>
+    </ParsedSelection>
+  </ParsedSelections>
+  <CompiledSelections Name="Compiled">
+    <Selection Name="Selection1">
+      <String Name="Name">GrpA</String>
+      <Sequence Name="Atoms">
+        <Int Name="Length">5</Int>
+        <Int>0</Int>
+        <Int>1</Int>
+        <Int>2</Int>
+        <Int>3</Int>
+        <Int>4</Int>
+      </Sequence>
+    </Selection>
+    <Selection Name="Selection2">
+      <String Name="Name">GrpB</String>
+      <Sequence Name="Atoms">
+        <Int Name="Length">5</Int>
+        <Int>3</Int>
+        <Int>4</Int>
+        <Int>5</Int>
+        <Int>6</Int>
+        <Int>7</Int>
+      </Sequence>
+    </Selection>
+    <Selection Name="Selection3">
+      <String Name="Name">GrpB</String>
+      <Sequence Name="Atoms">
+        <Int Name="Length">5</Int>
+        <Int>3</Int>
+        <Int>4</Int>
+        <Int>5</Int>
+        <Int>6</Int>
+        <Int>7</Int>
+      </Sequence>
+    </Selection>
+    <Selection Name="Selection4">
+      <String Name="Name">group "GrpB" and resname RB</String>
+      <Sequence Name="Atoms">
+        <Int Name="Length">3</Int>
+        <Int>3</Int>
+        <Int>4</Int>
+        <Int>5</Int>
+      </Sequence>
+    </Selection>
+  </CompiledSelections>
+</ReferenceData>
diff --git a/src/gromacs/selection/tests/refdata/SelectionCollectionDataTest_HandlesSelectionNames.xml b/src/gromacs/selection/tests/refdata/SelectionCollectionDataTest_HandlesSelectionNames.xml
new file mode 100644 (file)
index 0000000..b1c43e5
--- /dev/null
@@ -0,0 +1,69 @@
+<?xml version="1.0"?>
+<?xml-stylesheet type="text/xsl" href="referencedata.xsl"?>
+<ReferenceData>
+  <ParsedSelections Name="Parsed">
+    <ParsedSelection Name="Selection1">
+      <String Name="Input">"GroupSelection" group "GrpA"</String>
+      <String Name="Name">GroupSelection</String>
+      <String Name="Text">"GroupSelection" group "GrpA"</String>
+      <Bool Name="Dynamic">false</Bool>
+    </ParsedSelection>
+    <ParsedSelection Name="Selection2">
+      <String Name="Input">"DynamicSelection" x &lt; 5</String>
+      <String Name="Name">DynamicSelection</String>
+      <String Name="Text">"DynamicSelection" x &lt; 5</String>
+      <Bool Name="Dynamic">true</Bool>
+    </ParsedSelection>
+    <ParsedSelection Name="Selection3">
+      <String Name="Input">y &lt; 3</String>
+      <String Name="Name">y &lt; 3</String>
+      <String Name="Text">y &lt; 3</String>
+      <Bool Name="Dynamic">true</Bool>
+    </ParsedSelection>
+  </ParsedSelections>
+  <CompiledSelections Name="Compiled">
+    <Selection Name="Selection1">
+      <String Name="Name">GroupSelection</String>
+      <Sequence Name="Atoms">
+        <Int Name="Length">5</Int>
+        <Int>0</Int>
+        <Int>1</Int>
+        <Int>2</Int>
+        <Int>3</Int>
+        <Int>4</Int>
+      </Sequence>
+    </Selection>
+    <Selection Name="Selection2">
+      <String Name="Name">DynamicSelection</String>
+      <Sequence Name="Atoms">
+        <Int Name="Length">10</Int>
+        <Int>0</Int>
+        <Int>1</Int>
+        <Int>2</Int>
+        <Int>3</Int>
+        <Int>4</Int>
+        <Int>5</Int>
+        <Int>6</Int>
+        <Int>7</Int>
+        <Int>8</Int>
+        <Int>9</Int>
+      </Sequence>
+    </Selection>
+    <Selection Name="Selection3">
+      <String Name="Name">y &lt; 3</String>
+      <Sequence Name="Atoms">
+        <Int Name="Length">10</Int>
+        <Int>0</Int>
+        <Int>1</Int>
+        <Int>2</Int>
+        <Int>3</Int>
+        <Int>4</Int>
+        <Int>5</Int>
+        <Int>6</Int>
+        <Int>7</Int>
+        <Int>8</Int>
+        <Int>9</Int>
+      </Sequence>
+    </Selection>
+  </CompiledSelections>
+</ReferenceData>
index 5f6cb924d267b745064a2cea77f589b965149baf..f61376e39e5e24a41f93e410c86ee42ea552d9b8 100644 (file)
 
 <xsl:template match="Selection">
     <h3><xsl:value-of select="@Name"/></h3>
-    <p>
-        Selection text:<br/>
-        <xsl:value-of select="key('SelectionName', @Name)/String[@Name='Text']"/>
-    </p>
-    <xsl:apply-templates />
-</xsl:template>
-
-<xsl:template match="Selection/Sequence[@Name='Atoms']">
-    <p>
-        Atoms:
-        <xsl:call-template name="SequenceAsHorizontalTable"/>
-    </p>
+    <table>
+        <xsl:if test="String[@Name='Name']">
+            <tr>
+                <td>Name:</td>
+                <td><xsl:value-of select="String[@Name='Name']"/></td>
+            </tr>
+        </xsl:if>
+        <tr>
+            <td>Selection text:</td>
+            <td>
+                <xsl:value-of select="key('SelectionName', @Name)/String[@Name='Text']"/>
+            </td>
+        </tr>
+        <xsl:if test="Sequence[@Name='Atoms']">
+            <tr>
+                <td>Atoms (<xsl:value-of select="Sequence[@Name='Atoms']/Int[@Name='Length']"/>):</td>
+                <td>
+                    <xsl:call-template name="SequenceAsCSV">
+                        <xsl:with-param name="root" select="Sequence[@Name='Atoms']"/>
+                    </xsl:call-template>
+                </td>
+            </tr>
+        </xsl:if>
+    </table>
+    <xsl:apply-templates select="Sequence[@Name='Positions']"/>
 </xsl:template>
 
 <xsl:template match="Selection/Sequence[@Name='Positions']">
index 212cffc6303f02bf880a220b5ac36ed9f3168fb2..521514e60870c3e65e9297e67f2d806c520d8cd8 100644 (file)
@@ -43,6 +43,7 @@
 
 #include "gromacs/options/basicoptions.h"
 #include "gromacs/options/options.h"
+#include "gromacs/selection/indexutil.h"
 #include "gromacs/selection/selectioncollection.h"
 #include "gromacs/selection/selection.h"
 #include "gromacs/utility/exceptions.h"
@@ -70,6 +71,7 @@ class SelectionCollectionTest : public ::testing::Test
         static int               s_debugLevel;
 
         SelectionCollectionTest();
+        ~SelectionCollectionTest();
 
         void setAtomCount(int natoms)
         {
@@ -77,12 +79,14 @@ class SelectionCollectionTest : public ::testing::Test
         }
         void loadTopology(const char *filename);
         void setTopology();
+        void loadIndexGroups(const char *filename);
 
         gmx::test::TopologyManager  topManager_;
         gmx::SelectionCollection    sc_;
         gmx::SelectionList          sel_;
         t_topology                 *top_;
         t_trxframe                 *frame_;
+        gmx_ana_indexgrps_t        *grps_;
 };
 
 int SelectionCollectionTest::s_debugLevel = 0;
@@ -95,7 +99,7 @@ GMX_TEST_OPTIONS(SelectionCollectionTestOptions, options)
 }
 
 SelectionCollectionTest::SelectionCollectionTest()
-    : top_(NULL), frame_(NULL)
+    : top_(NULL), frame_(NULL), grps_(NULL)
 {
     topManager_.requestFrame();
     sc_.setDebugLevel(s_debugLevel);
@@ -103,6 +107,14 @@ SelectionCollectionTest::SelectionCollectionTest()
     sc_.setOutputPosType("atom");
 }
 
+SelectionCollectionTest::~SelectionCollectionTest()
+{
+    if (grps_ != NULL)
+    {
+        gmx_ana_indexgrps_free(grps_);
+    }
+}
+
 void
 SelectionCollectionTest::loadTopology(const char *filename)
 {
@@ -119,6 +131,17 @@ SelectionCollectionTest::setTopology()
     ASSERT_NO_THROW_GMX(sc_.setTopology(top_, -1));
 }
 
+void
+SelectionCollectionTest::loadIndexGroups(const char *filename)
+{
+    GMX_RELEASE_ASSERT(grps_ == NULL,
+                       "External groups can only be loaded once");
+    std::string fullpath =
+        gmx::test::TestFileManager::getInputFilePath(filename);
+    gmx_ana_indexgrps_init(&grps_, NULL, fullpath.c_str());
+    sc_.setIndexGroups(grps_);
+}
+
 
 /********************************************************************
  * Test fixture for selection testing with reference data
@@ -135,6 +158,7 @@ class SelectionCollectionDataTest : public SelectionCollectionTest
             efTestPositionMapping       = 1<<3,
             efTestPositionMasses        = 1<<4,
             efTestPositionCharges       = 1<<5,
+            efTestSelectionNames        = 1<<6,
             efDontTestCompiledAtoms     = 1<<8
         };
         typedef gmx::FlagsTemplate<TestFlag> TestFlags;
@@ -298,6 +322,10 @@ SelectionCollectionDataTest::checkCompiled()
         std::string          id = gmx::formatString("Selection%d", static_cast<int>(i + 1));
         TestReferenceChecker selcompound(
                 compound.checkCompound("Selection", id.c_str()));
+        if (flags_.test(efTestSelectionNames))
+        {
+            selcompound.checkString(sel_[i].name(), "Name");
+        }
         if (!flags_.test(efDontTestCompiledAtoms))
         {
             checkSelection(&selcompound, sel_[i], flags_ & mask);
@@ -445,7 +473,21 @@ TEST_F(SelectionCollectionTest, HandlesHelpKeywordInInvalidContext)
 
 // TODO: Tests for more parser errors
 
-TEST_F(SelectionCollectionTest, RecoversFromUnknownGroupReference)
+TEST_F(SelectionCollectionTest, HandlesUnknownGroupReferenceParser1)
+{
+    ASSERT_NO_THROW_GMX(sc_.setIndexGroups(NULL));
+    EXPECT_THROW_GMX(sc_.parseFromString("group \"foo\""), gmx::InvalidInputError);
+    EXPECT_THROW_GMX(sc_.parseFromString("4"), gmx::InvalidInputError);
+}
+
+TEST_F(SelectionCollectionTest, HandlesUnknownGroupReferenceParser2)
+{
+    ASSERT_NO_THROW_GMX(loadIndexGroups("simple.ndx"));
+    EXPECT_THROW_GMX(sc_.parseFromString("group \"foo\""), gmx::InvalidInputError);
+    EXPECT_THROW_GMX(sc_.parseFromString("4"), gmx::InvalidInputError);
+}
+
+TEST_F(SelectionCollectionTest, HandlesUnknownGroupReferenceDelayed1)
 {
     ASSERT_NO_THROW_GMX(sc_.parseFromString("group \"foo\""));
     ASSERT_NO_FATAL_FAILURE(setAtomCount(10));
@@ -453,6 +495,14 @@ TEST_F(SelectionCollectionTest, RecoversFromUnknownGroupReference)
     EXPECT_THROW_GMX(sc_.compile(), gmx::APIError);
 }
 
+TEST_F(SelectionCollectionTest, HandlesUnknownGroupReferenceDelayed2)
+{
+    ASSERT_NO_THROW_GMX(sc_.parseFromString("group 4; group \"foo\""));
+    ASSERT_NO_FATAL_FAILURE(setAtomCount(10));
+    EXPECT_THROW_GMX(loadIndexGroups("simple.ndx"), gmx::InvalidInputError);
+    EXPECT_THROW_GMX(sc_.compile(), gmx::APIError);
+}
+
 TEST_F(SelectionCollectionTest, RecoversFromMissingMoleculeInfo)
 {
     ASSERT_NO_THROW_GMX(sc_.parseFromString("molindex 1 to 5"));
@@ -835,6 +885,46 @@ TEST_F(SelectionCollectionDataTest, ComputesMassesAndChargesWithoutTopology)
  * Tests for selection syntactic constructs
  */
 
+TEST_F(SelectionCollectionDataTest, HandlesSelectionNames)
+{
+    static const char * const selections[] = {
+        "\"GroupSelection\" group \"GrpA\"",
+        "\"DynamicSelection\" x < 5",
+        "y < 3"
+    };
+    setFlags(TestFlags() | efTestSelectionNames);
+    ASSERT_NO_THROW_GMX(loadIndexGroups("simple.ndx"));
+    runTest(10, selections);
+}
+
+TEST_F(SelectionCollectionDataTest, HandlesIndexGroupsInSelections)
+{
+    static const char * const selections[] = {
+        "group \"GrpA\"",
+        "GrpB",
+        "1",
+        "group \"GrpB\" and resname RB"
+    };
+    setFlags(TestFlags() | efTestSelectionNames);
+    ASSERT_NO_THROW_GMX(loadIndexGroups("simple.ndx"));
+    runTest("simple.gro", selections);
+}
+
+TEST_F(SelectionCollectionDataTest, HandlesIndexGroupsInSelectionsDelayed)
+{
+    static const char * const selections[] = {
+        "group \"GrpA\"",
+        "GrpB",
+        "1",
+        "group \"GrpB\" and resname RB"
+    };
+    setFlags(TestFlags() | efTestSelectionNames);
+    ASSERT_NO_FATAL_FAILURE(runParser(selections));
+    ASSERT_NO_FATAL_FAILURE(loadTopology("simple.gro"));
+    ASSERT_NO_THROW_GMX(loadIndexGroups("simple.ndx"));
+    ASSERT_NO_FATAL_FAILURE(runCompiler());
+}
+
 TEST_F(SelectionCollectionDataTest, HandlesConstantPositions)
 {
     static const char * const selections[] = {
diff --git a/src/gromacs/selection/tests/simple.ndx b/src/gromacs/selection/tests/simple.ndx
new file mode 100644 (file)
index 0000000..4ca7873
--- /dev/null
@@ -0,0 +1,8 @@
+[ GrpA ]
+1 2 3 4 5
+
+[ GrpB ]
+4 5 6 7 8
+
+[ GrpUnsorted ]
+1 2 4 3 2