Implement remaining SelectionOption flags.
authorTeemu Murtola <teemu.murtola@gmail.com>
Fri, 12 Apr 2013 04:19:40 +0000 (07:19 +0300)
committerGerrit Code Review <gerrit@gerrit.gromacs.org>
Wed, 17 Apr 2013 07:02:57 +0000 (09:02 +0200)
- If onlyAtoms() is specified, check the condition after the selections
  have been compiled.
- If onlyAtoms() is specified for all selection options, remove the
  -seltype command-line option, as it has no use.
- Removed unimplemented dynamicOnlyWhole() as the way it worked in 4.5
  is not really compatible with the current handling of selections.
  It was any way only affecting a default value, but not enforcing
  anything.

Change-Id: I0cd4ff6d425cc017bead090be12867b4f41e7783

src/gromacs/selection/selection.h
src/gromacs/selection/selectioncollection.cpp
src/gromacs/selection/selectionenums.h
src/gromacs/selection/selectionoption.cpp
src/gromacs/selection/selectionoption.h
src/gromacs/selection/tests/selectionoption.cpp

index cff42a0860e8641ca5f4ebb4bc05d0c53841cdc7..b91ed8b213e0751432956b0898c64d8fe863b3a4 100644 (file)
@@ -99,6 +99,9 @@ class SelectionData
         const char *selectionText() const { return selectionText_.c_str(); }
         //! Returns true if the size of the selection (posCount()) is dynamic.
         bool isDynamic() const { return bDynamic_; }
+        //! Returns the type of positions in the selection.
+        e_index_t type() const { return rawPositions_.m.type; }
+
         //! Number of positions in the selection.
         int posCount() const { return rawPositions_.nr; }
         //! Returns the root of the evaluation tree for this selection.
@@ -289,7 +292,7 @@ class Selection
         //! Returns true if the size of the selection (posCount()) is dynamic.
         bool isDynamic() const { return data().isDynamic(); }
         //! Returns the type of positions in the selection.
-        e_index_t type() const { return data().rawPositions_.m.type; }
+        e_index_t type() const { return data().type(); }
 
         //! Total number of atoms in the selection.
         int atomCount() const
@@ -548,7 +551,7 @@ class SelectionPosition
          *
          * Currently always returns the same as Selection::type().
          */
-        e_index_t type() const { return sel_->rawPositions_.m.type; }
+        e_index_t type() const { return sel_->type(); }
         //! Returns coordinates for this position.
         const rvec &x() const
         {
index e8e609ef31de4938018982bd518df9a17dc23cc6..b23539a068b784524ac905f177f55632ced21814 100644 (file)
@@ -63,6 +63,7 @@
 #include "parser.h"
 #include "poscalc.h"
 #include "scanner.h"
+#include "selection.h"
 #include "selectioncollection-impl.h"
 #include "selelem.h"
 #include "selhelp.h"
@@ -370,26 +371,35 @@ void
 SelectionCollection::initOptions(Options *options)
 {
     const char * const debug_levels[]
-        = {"no", "basic", "compile", "eval", "full" };
-    /*
-       static const char * const desc[] = {
-        "This program supports selections in addition to traditional",
-        "index files. Use [TT]-select help[tt] for additional information,",
-        "or type 'help' in the selection prompt.",
-        NULL,
-       };
-       options.setDescription(desc);
-     */
+        = { "no", "basic", "compile", "eval", "full" };
+
+    bool bAllowNonAtomOutput = false;
+    SelectionDataList::const_iterator iter;
+    for (iter = impl_->sc_.sel.begin(); iter != impl_->sc_.sel.end(); ++iter)
+    {
+        const internal::SelectionData &sel = **iter;
+        if (!sel.hasFlag(efSelection_OnlyAtoms))
+        {
+            bAllowNonAtomOutput = true;
+        }
+    }
 
     const char *const *postypes = PositionCalculationCollection::typeEnumValues;
     options->addOption(StringOption("selrpos")
                            .enumValueFromNullTerminatedArray(postypes)
                            .store(&impl_->rpost_).defaultValue(postypes[0])
                            .description("Selection reference positions"));
-    options->addOption(StringOption("seltype")
-                           .enumValueFromNullTerminatedArray(postypes)
-                           .store(&impl_->spost_).defaultValue(postypes[0])
-                           .description("Default selection output positions"));
+    if (bAllowNonAtomOutput)
+    {
+        options->addOption(StringOption("seltype")
+                               .enumValueFromNullTerminatedArray(postypes)
+                               .store(&impl_->spost_).defaultValue(postypes[0])
+                               .description("Default selection output positions"));
+    }
+    else
+    {
+        impl_->spost_ = postypes[0];
+    }
     GMX_RELEASE_ASSERT(impl_->debugLevel_ >= 0 && impl_->debugLevel_ <= 4,
                        "Debug level out of range");
     options->addOption(StringOption("seldebug").hidden(impl_->debugLevel_ == 0)
@@ -596,6 +606,25 @@ SelectionCollection::compile()
         impl_->sc_.pcc.printTree(stderr);
         std::fprintf(stderr, "\n");
     }
+
+    // TODO: It would be nicer to associate the name of the selection option
+    // (if available) to the error message.
+    SelectionDataList::const_iterator iter;
+    for (iter = impl_->sc_.sel.begin(); iter != impl_->sc_.sel.end(); ++iter)
+    {
+        const internal::SelectionData &sel = **iter;
+        if (sel.hasFlag(efSelection_OnlyAtoms))
+        {
+            if (sel.type() != INDEX_ATOM)
+            {
+                std::string message = formatString(
+                            "Selection '%s' does not evaluate to individual atoms. "
+                            "This is not allowed in this context.",
+                            sel.selectionText());
+                GMX_THROW(InvalidInputError(message));
+            }
+        }
+    }
 }
 
 
index 3841c77204e1155642e41c4ef34d8682c14cb757..41542f3a004045d1929512d55fbd1dfc0716912a 100644 (file)
@@ -1,7 +1,7 @@
 /*
  * This file is part of the GROMACS molecular simulation package.
  *
- * Copyright (c) 2010,2012, by the GROMACS development team, led by
+ * Copyright (c) 2010,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.
@@ -71,7 +71,6 @@ enum SelectionFlag
     efSelection_OnlyAtoms               = 1<<1,
     //! Whether ::POS_MASKONLY should be used for output position evaluation.
     efSelection_DynamicMask             = 1<<2,
-    efSelection_DynamicOnlyWhole        = 1<<3,
     //! Whether velocities of output positions should be evaluated.
     efSelection_EvaluateVelocities      = 1<<5,
     //! Whether forces on output positions should be evaluated.
index 7555d95755b6decc81aadf02effda4d989f73838..22903f81de8aa80a7688125a86e981f9e0a31753 100644 (file)
@@ -1,7 +1,7 @@
 /*
  * This file is part of the GROMACS molecular simulation package.
  *
- * Copyright (c) 2010,2011,2012, by the GROMACS development team, led by
+ * Copyright (c) 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.
@@ -260,11 +260,6 @@ void SelectionOptionInfo::setDynamicMask(bool bEnabled)
     option().setSelectionFlag(efSelection_DynamicMask, bEnabled);
 }
 
-void SelectionOptionInfo::setDynamicOnlyWhole(bool bEnabled)
-{
-    option().setSelectionFlag(efSelection_DynamicOnlyWhole, bEnabled);
-}
-
 
 /********************************************************************
  * SelectionOption
index 348f9fa875da6d63391cb0a807c0d057deee36ac..7ade5a97752fb1d68bc267607c0bcef89f2f9704 100644 (file)
@@ -96,8 +96,6 @@ class SelectionOption : public OptionTemplate<Selection, SelectionOption>
         { selectionFlags_.set(efSelection_EvaluateForces); return me(); }
         /*! \brief
          * Only accept selections that evaluate to atom positions.
-         *
-         * TODO: This option is not yet implemented.
          */
         MyClass &onlyAtoms()
         { selectionFlags_.set(efSelection_OnlyAtoms); return me(); }
@@ -114,13 +112,6 @@ class SelectionOption : public OptionTemplate<Selection, SelectionOption>
          */
         MyClass &dynamicMask()
         { selectionFlags_.set(efSelection_DynamicMask); return me(); }
-        /*! \brief
-         * Disallow using atom coordinates as the reference positions.
-         *
-         * TODO: This option is not yet implemented.
-         */
-        MyClass &dynamicOnlyWhole()
-        { selectionFlags_.set(efSelection_DynamicOnlyWhole); return me(); }
 
     private:
         // Disable possibility to allow multiple occurrences, since it isn't
@@ -249,8 +240,6 @@ class SelectionOptionInfo : public OptionInfo
          * \param[in] bEnabled  If true, the option accepts only positions that
          *      evaluate to atom positions.
          *
-         * TODO: This is not yet implemented.
-         *
          * \see SelectionOption::onlyAtoms()
          */
         void setOnlyAtoms(bool bEnabled);
@@ -278,17 +267,6 @@ class SelectionOptionInfo : public OptionInfo
          * \see SelectionOption::dynamicMask()
          */
         void setDynamicMask(bool bEnabled);
-        /*! \brief
-         * Sets whether atom coordinates are allowed as reference positions.
-         *
-         * \param[in] bEnabled  If true, the option does not accept atom
-         *      coordinates as reference positions.
-         *
-         * TODO: This is not yet implemented.
-         *
-         * \see SelectionOption::dynamicOnlyWhole()
-         */
-        void setDynamicOnlyWhole(bool bEnabled);
 
     private:
         SelectionOptionStorage &option();
index 7fc687126f03c348ccf089db77c3624725c63dfb..690f0b99ac01ffabb822586d20b55890b809c2ac 100644 (file)
@@ -41,6 +41,9 @@
  */
 #include <gtest/gtest.h>
 
+#include "gromacs/legacyheaders/smalloc.h"
+#include "gromacs/legacyheaders/tpxio.h"
+
 #include "gromacs/options/options.h"
 #include "gromacs/options/optionsassigner.h"
 #include "gromacs/selection/selection.h"
@@ -66,27 +69,54 @@ class SelectionOptionTestBase : public ::testing::Test
 {
     public:
         SelectionOptionTestBase();
+        ~SelectionOptionTestBase();
 
         void setManager();
+        void loadTopology(const char *filename);
 
         gmx::SelectionCollection    sc_;
         gmx::SelectionOptionManager manager_;
         gmx::Options                options_;
+        t_topology                 *top_;
 };
 
 SelectionOptionTestBase::SelectionOptionTestBase()
-    : manager_(&sc_), options_(NULL, NULL)
+    : manager_(&sc_), options_(NULL, NULL), top_(NULL)
 {
     sc_.setReferencePosType("atom");
     sc_.setOutputPosType("atom");
 }
 
+SelectionOptionTestBase::~SelectionOptionTestBase()
+{
+    if (top_ != NULL)
+    {
+        free_t_atoms(&top_->atoms, TRUE);
+        done_top(top_);
+        sfree(top_);
+    }
+}
+
 void SelectionOptionTestBase::setManager()
 {
     setManagerForSelectionOptions(&options_, &manager_);
 }
 
 
+void SelectionOptionTestBase::loadTopology(const char *filename)
+{
+    char    title[STRLEN];
+    int     ePBC;
+    matrix  box;
+
+    snew(top_, 1);
+    read_tps_conf(gmx::test::TestFileManager::getInputFilePath(filename).c_str(),
+                  title, top_, &ePBC, NULL, NULL, box, FALSE);
+
+    ASSERT_NO_THROW_GMX(sc_.setTopology(top_, -1));
+}
+
+
 /********************************************************************
  * Tests for SelectionOption
  */
@@ -129,6 +159,27 @@ TEST_F(SelectionOptionTest, HandlesDynamicSelectionWhenStaticRequired)
 }
 
 
+TEST_F(SelectionOptionTest, HandlesNonAtomicSelectionWhenAtomsRequired)
+{
+    gmx::Selection sel;
+    using gmx::SelectionOption;
+    ASSERT_NO_THROW_GMX(options_.addOption(
+                                SelectionOption("sel").store(&sel).onlyAtoms()));
+    setManager();
+
+    gmx::OptionsAssigner assigner(&options_);
+    EXPECT_NO_THROW_GMX(assigner.start());
+    ASSERT_NO_THROW_GMX(assigner.startOption("sel"));
+    EXPECT_NO_THROW_GMX(assigner.appendValue("res_cog of resname RA RB"));
+    EXPECT_NO_THROW_GMX(assigner.finishOption());
+    EXPECT_NO_THROW_GMX(assigner.finish());
+    EXPECT_NO_THROW_GMX(options_.finish());
+
+    ASSERT_NO_FATAL_FAILURE(loadTopology("simple.gro"));
+    EXPECT_THROW_GMX(sc_.compile(), gmx::InvalidInputError);
+}
+
+
 TEST_F(SelectionOptionTest, HandlesTooManySelections)
 {
     gmx::Selection sel;