From 86f86f19d985cd5516b5f8bd5d3dab4760d4b573 Mon Sep 17 00:00:00 2001 From: Teemu Murtola Date: Sun, 19 Jul 2015 17:38:43 +0300 Subject: [PATCH] Remove Options::isSet() Replace uses with alternative means. Add storeIsSet() to all options to make it convenient to just check whether an option is set. Remove Options parameters from methods where its only purpose was to provide access to isSet(), reducing exposure of the concrete class and making the interface cleaner. Change-Id: If1207c8cf3af86e306c52748f8878480e35e2fc0 --- .../commandline/cmdlinemodulemanager-impl.h | 3 +- .../commandline/cmdlinemodulemanager.cpp | 7 ++-- .../commandline/cmdlineoptionsmodule.cpp | 2 +- .../commandline/cmdlineoptionsmodule.h | 11 ++--- src/gromacs/commandline/pargs.cpp | 23 ++++++----- .../tests/cmdlinemodulemanagertest.h | 2 +- .../gmxpreprocess/insert-molecules.cpp | 10 +---- src/gromacs/options/abstractoption.cpp | 18 ++++++++- src/gromacs/options/abstractoption.h | 20 +++++++++- src/gromacs/options/abstractoptionstorage.h | 5 ++- src/gromacs/options/options.cpp | 6 --- src/gromacs/options/options.h | 2 - src/gromacs/options/tests/optionsassigner.cpp | 40 +++++++++++-------- .../trajectoryanalysis/analysismodule.cpp | 3 +- .../trajectoryanalysis/analysismodule.h | 4 +- .../trajectoryanalysis/cmdlinerunner.cpp | 4 +- .../trajectoryanalysis/modules/angle.cpp | 10 ++--- .../trajectoryanalysis/modules/rdf.cpp | 13 +++--- .../trajectoryanalysis/modules/select.cpp | 6 +-- .../trajectoryanalysis/runnercommon.cpp | 24 +++++++---- src/gromacs/trajectoryanalysis/runnercommon.h | 8 +--- src/testutils/refdata.cpp | 4 +- src/testutils/refdata.h | 4 +- 23 files changed, 129 insertions(+), 100 deletions(-) diff --git a/src/gromacs/commandline/cmdlinemodulemanager-impl.h b/src/gromacs/commandline/cmdlinemodulemanager-impl.h index 6ff6de239b..c7ee78023b 100644 --- a/src/gromacs/commandline/cmdlinemodulemanager-impl.h +++ b/src/gromacs/commandline/cmdlinemodulemanager-impl.h @@ -1,7 +1,7 @@ /* * This file is part of the GROMACS molecular simulation package. * - * Copyright (c) 2012,2013,2014, by the GROMACS development team, led by + * Copyright (c) 2012,2013,2014,2015, by the GROMACS development team, led by * Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl, * and including many others, as listed in the AUTHORS file in the * top-level source directory and at http://www.gromacs.org. @@ -204,6 +204,7 @@ class CommandLineCommonOptionsHolder bool bVersion_; bool bCopyright_; int niceLevel_; + bool bNiceSet_; bool bBackup_; bool bFpexcept_; int debugLevel_; diff --git a/src/gromacs/commandline/cmdlinemodulemanager.cpp b/src/gromacs/commandline/cmdlinemodulemanager.cpp index a9fa6a0ad4..83f35de08b 100644 --- a/src/gromacs/commandline/cmdlinemodulemanager.cpp +++ b/src/gromacs/commandline/cmdlinemodulemanager.cpp @@ -146,7 +146,8 @@ class CMainCommandLineModule : public ICommandLineModule CommandLineCommonOptionsHolder::CommandLineCommonOptionsHolder() : options_(NULL, NULL), bHelp_(false), bHidden_(false), bQuiet_(false), bVersion_(false), bCopyright_(true), - niceLevel_(19), bBackup_(true), bFpexcept_(false), debugLevel_(0) + niceLevel_(19), bNiceSet_(false), bBackup_(true), bFpexcept_(false), + debugLevel_(0) { binaryInfoSettings_.copyright(true); } @@ -168,7 +169,7 @@ void CommandLineCommonOptionsHolder::initOptions() .description("Print extended version information and quit")); options_.addOption(BooleanOption("copyright").store(&bCopyright_) .description("Print copyright information on startup")); - options_.addOption(IntegerOption("nice").store(&niceLevel_) + options_.addOption(IntegerOption("nice").store(&niceLevel_).storeIsSet(&bNiceSet_) .description("Set the nicelevel (default depends on command)")); options_.addOption(BooleanOption("backup").store(&bBackup_) .description("Write backups if output files exist")); @@ -193,7 +194,7 @@ bool CommandLineCommonOptionsHolder::finishOptions() void CommandLineCommonOptionsHolder::adjustFromSettings( const CommandLineModuleSettings &settings) { - if (!options_.isSet("nice")) + if (!bNiceSet_) { niceLevel_ = settings.defaultNiceLevel(); } diff --git a/src/gromacs/commandline/cmdlineoptionsmodule.cpp b/src/gromacs/commandline/cmdlineoptionsmodule.cpp index e8249492e8..581174ffba 100644 --- a/src/gromacs/commandline/cmdlineoptionsmodule.cpp +++ b/src/gromacs/commandline/cmdlineoptionsmodule.cpp @@ -164,7 +164,7 @@ void CommandLineOptionsModule::parseOptions(int argc, char *argv[]) parser.parse(&argc, argv); options.finish(); } - module_->optionsFinished(&options); + module_->optionsFinished(); } } // namespace diff --git a/src/gromacs/commandline/cmdlineoptionsmodule.h b/src/gromacs/commandline/cmdlineoptionsmodule.h index 529bc55f98..2aa8dbebdc 100644 --- a/src/gromacs/commandline/cmdlineoptionsmodule.h +++ b/src/gromacs/commandline/cmdlineoptionsmodule.h @@ -236,16 +236,13 @@ class ICommandLineOptionsModule /*! \brief * Called after all option values have been set. * - * \param[in,out] options Options object in which options are stored. - * * When running the module, this method is called after all - * command-line arguments have been parsed, but while the Options - * object still exists. + * command-line arguments have been parsed. * - * If the module needs to call, e.g., Options::isSet(), this is the - * place to do that. + * \todo + * Remove if no real need materializes. */ - virtual void optionsFinished(Options *options) = 0; + virtual void optionsFinished() = 0; /*! \brief * Runs the module. diff --git a/src/gromacs/commandline/pargs.cpp b/src/gromacs/commandline/pargs.cpp index 6daf7d554b..a05bc495c3 100644 --- a/src/gromacs/commandline/pargs.cpp +++ b/src/gromacs/commandline/pargs.cpp @@ -487,9 +487,10 @@ gmx_bool parse_common_args(int *argc, char *argv[], unsigned long Flags, try { - double tbegin = 0.0, tend = 0.0, tdelta = 0.0; - bool bView = false; - int xvgFormat = 0; + double tbegin = 0.0, tend = 0.0, tdelta = 0.0; + bool bBeginTimeSet = false, bEndTimeSet = false, bDtSet = false; + bool bView = false; + int xvgFormat = 0; gmx::TimeUnitManager timeUnitManager; gmx::OptionsAdapter adapter(*argc, argv); gmx::Options options(NULL, NULL); @@ -506,19 +507,22 @@ gmx_bool parse_common_args(int *argc, char *argv[], unsigned long Flags, if (FF(PCA_CAN_BEGIN)) { options.addOption( - gmx::DoubleOption("b").store(&tbegin).timeValue() + gmx::DoubleOption("b") + .store(&tbegin).storeIsSet(&bBeginTimeSet).timeValue() .description("First frame (%t) to read from trajectory")); } if (FF(PCA_CAN_END)) { options.addOption( - gmx::DoubleOption("e").store(&tend).timeValue() + gmx::DoubleOption("e") + .store(&tend).storeIsSet(&bEndTimeSet).timeValue() .description("Last frame (%t) to read from trajectory")); } if (FF(PCA_CAN_DT)) { options.addOption( - gmx::DoubleOption("dt").store(&tdelta).timeValue() + gmx::DoubleOption("dt") + .store(&tdelta).storeIsSet(&bDtSet).timeValue() .description("Only use frame when t MOD dt = first time (%t)")); } if (FF(PCA_TIME_UNIT)) @@ -586,16 +590,15 @@ gmx_bool parse_common_args(int *argc, char *argv[], unsigned long Flags, timeUnitManager.scaleTimeOptions(&options); /* Extract Time info from arguments */ - // TODO: Use OptionInfo objects instead of string constants - if (FF(PCA_CAN_BEGIN) && options.isSet("b")) + if (bBeginTimeSet) { setTimeValue(TBEGIN, tbegin); } - if (FF(PCA_CAN_END) && options.isSet("e")) + if (bEndTimeSet) { setTimeValue(TEND, tend); } - if (FF(PCA_CAN_DT) && options.isSet("dt")) + if (bDtSet) { setTimeValue(TDELTA, tdelta); } diff --git a/src/gromacs/commandline/tests/cmdlinemodulemanagertest.h b/src/gromacs/commandline/tests/cmdlinemodulemanagertest.h index 9cb462f6b2..fde073ba03 100644 --- a/src/gromacs/commandline/tests/cmdlinemodulemanagertest.h +++ b/src/gromacs/commandline/tests/cmdlinemodulemanagertest.h @@ -109,7 +109,7 @@ class MockOptionsModule : public gmx::ICommandLineOptionsModule MOCK_METHOD1(init, void(gmx::CommandLineModuleSettings *settings)); MOCK_METHOD2(initOptions, void(gmx::IOptionsContainer *options, gmx::ICommandLineOptionsModuleSettings *settings)); - MOCK_METHOD1(optionsFinished, void(gmx::Options *options)); + MOCK_METHOD0(optionsFinished, void()); MOCK_METHOD0(run, int()); }; diff --git a/src/gromacs/gmxpreprocess/insert-molecules.cpp b/src/gromacs/gmxpreprocess/insert-molecules.cpp index aefe099563..b166e9d22b 100644 --- a/src/gromacs/gmxpreprocess/insert-molecules.cpp +++ b/src/gromacs/gmxpreprocess/insert-molecules.cpp @@ -52,7 +52,6 @@ #include "gromacs/options/basicoptions.h" #include "gromacs/options/filenameoption.h" #include "gromacs/options/ioptionscontainer.h" -#include "gromacs/options/options.h" #include "gromacs/pbcutil/pbc.h" #include "gromacs/random/random.h" #include "gromacs/selection/nbsearch.h" @@ -321,7 +320,7 @@ class InsertMolecules : public ICommandLineOptionsModule virtual void initOptions(IOptionsContainer *options, ICommandLineOptionsModuleSettings *settings); - virtual void optionsFinished(Options *options); + virtual void optionsFinished() {} virtual int run(); @@ -404,7 +403,7 @@ void InsertMolecules::initOptions(IOptionsContainer *options, .description("Output configuration after insertion")); options->addOption(RealOption("box").vector() - .store(newBox_) + .store(newBox_).storeIsSet(&bBox_) .description("Box size (in nm)")); options->addOption(IntegerOption("nmol") .store(&nmolIns_) @@ -430,11 +429,6 @@ void InsertMolecules::initOptions(IOptionsContainer *options, .description("Rotate inserted molecules randomly")); } -void InsertMolecules::optionsFinished(Options *options) -{ - bBox_ = options->isSet("box"); -} - int InsertMolecules::run() { const bool bProt = !inputConfFile_.empty(); diff --git a/src/gromacs/options/abstractoption.cpp b/src/gromacs/options/abstractoption.cpp index a71ed24353..25d4ecab83 100644 --- a/src/gromacs/options/abstractoption.cpp +++ b/src/gromacs/options/abstractoption.cpp @@ -1,7 +1,7 @@ /* * This file is part of the GROMACS molecular simulation package. * - * Copyright (c) 2010,2011,2012,2013,2014, by the GROMACS development team, led by + * Copyright (c) 2010,2011,2012,2013,2014,2015, by the GROMACS development team, led by * Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl, * and including many others, as listed in the AUTHORS file in the * top-level source directory and at http://www.gromacs.org. @@ -60,6 +60,7 @@ namespace gmx AbstractOptionStorage::AbstractOptionStorage(const AbstractOption &settings, OptionFlags staticFlags) : flags_(settings.flags_ | staticFlags), + storeIsSet_(settings.storeIsSet_), minValueCount_(settings.minValueCount_), maxValueCount_(settings.maxValueCount_), bInSet_(false), bSetValuesHadErrors_(false) @@ -78,6 +79,10 @@ AbstractOptionStorage::AbstractOptionStorage(const AbstractOption &settings, { descr_ = settings.descr_; } + if (storeIsSet_ != NULL) + { + *storeIsSet_ = false; + } setFlag(efOption_ClearOnNextSet); } @@ -125,6 +130,15 @@ void AbstractOptionStorage::appendValue(const std::string &value) } } +void AbstractOptionStorage::markAsSet() +{ + setFlag(efOption_Set); + if (storeIsSet_ != NULL) + { + *storeIsSet_ = true; + } +} + void AbstractOptionStorage::finishSet() { GMX_RELEASE_ASSERT(bInSet_, "startSet() not called"); @@ -132,7 +146,7 @@ void AbstractOptionStorage::finishSet() // We mark the option as set even when there are errors to avoid additional // errors from required options not set. // TODO: There could be a separate flag for this purpose. - setFlag(efOption_Set); + markAsSet(); if (!bSetValuesHadErrors_) { // TODO: Correct handling of the efOption_ClearOnNextSet requires diff --git a/src/gromacs/options/abstractoption.h b/src/gromacs/options/abstractoption.h index 563c3621f6..47276f214b 100644 --- a/src/gromacs/options/abstractoption.h +++ b/src/gromacs/options/abstractoption.h @@ -101,7 +101,7 @@ class AbstractOption //! Initializes the name and default values for an option. explicit AbstractOption(const char *name) : minValueCount_(1), maxValueCount_(1), - name_(name), descr_(NULL) + name_(name), descr_(NULL), storeIsSet_(NULL) { } /*! \brief @@ -133,6 +133,8 @@ class AbstractOption //! Sets the description for the option. void setDescription(const char *descr) { descr_ = descr; } + //! Sets the storage location for whether the option is set. + void setStoreIsSet(bool *store) { storeIsSet_ = store; } //! Sets a flag for the option. void setFlag(OptionFlag flag) { flags_.set(flag); } //! Clears a flag for the option. @@ -188,6 +190,7 @@ class AbstractOption //! Pointer to description of the option. const char *descr_; OptionFlags flags_; + bool *storeIsSet_; /*! \brief * Needed to initialize an AbstractOptionStorage object from this class @@ -332,6 +335,21 @@ class OptionTemplate : public AbstractOption */ MyClass &storeVector(std::vector *store) { storeVector_ = store; return me(); } + /*! \brief + * Stores whether the option was explicitly set. + * + * \param[in] store Variable to store the flag in. + * + * The value is set to `false` on creation of the option, and to `true` + * as soon as a value is assigned to the option. A default value does + * not set the flag to `true`, but assignment that uses + * defaultValueIfSet() does. + * + * The pointer provided should remain valid as long as the associated + * Options object exists. + */ + MyClass &storeIsSet(bool *store) + { setStoreIsSet(store); return me(); } protected: /*! \cond libapi */ diff --git a/src/gromacs/options/abstractoptionstorage.h b/src/gromacs/options/abstractoptionstorage.h index 4b85b41d40..55ea22fd26 100644 --- a/src/gromacs/options/abstractoptionstorage.h +++ b/src/gromacs/options/abstractoptionstorage.h @@ -1,7 +1,7 @@ /* * This file is part of the GROMACS molecular simulation package. * - * Copyright (c) 2010,2011,2012,2014, by the GROMACS development team, led by + * Copyright (c) 2010,2011,2012,2014,2015, by the GROMACS development team, led by * Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl, * and including many others, as listed in the AUTHORS file in the * top-level source directory and at http://www.gromacs.org. @@ -220,7 +220,7 @@ class AbstractOptionStorage OptionFlags staticFlags); //! Marks the option as set. - void markAsSet() { flags_.set(efOption_Set); } + void markAsSet(); //! Returns true if the given flag is set. bool hasFlag(OptionFlag flag) const { return flags_.test(flag); } //! Sets the given flag. @@ -320,6 +320,7 @@ class AbstractOptionStorage std::string descr_; //! Flags for the option. OptionFlags flags_; + bool *storeIsSet_; //! Minimum number of values required (in one set). int minValueCount_; //! Maximum allowed number of values (in one set), or -1 if no limit. diff --git a/src/gromacs/options/options.cpp b/src/gromacs/options/options.cpp index 97dabb99d6..f6a018ad7b 100644 --- a/src/gromacs/options/options.cpp +++ b/src/gromacs/options/options.cpp @@ -221,12 +221,6 @@ OptionInfo *Options::addOption(const AbstractOption &settings) return impl_->rootGroup_.addOption(settings); } -bool Options::isSet(const char *name) const -{ - AbstractOptionStorage *option = impl_->findOption(name); - return (option != NULL ? option->isSet() : false); -} - void Options::finish() { // TODO: Consider how to customize these error messages based on context. diff --git a/src/gromacs/options/options.h b/src/gromacs/options/options.h index 67fcf25737..43afe21d40 100644 --- a/src/gromacs/options/options.h +++ b/src/gromacs/options/options.h @@ -175,8 +175,6 @@ class Options : public IOptionsContainer virtual OptionInfo *addOption(const AbstractOption &settings); using IOptionsContainer::addOption; - //! Returns true if option \p name is set. - bool isSet(const char *name) const; /*! \brief * Notifies the collection that all option values are assigned. * diff --git a/src/gromacs/options/tests/optionsassigner.cpp b/src/gromacs/options/tests/optionsassigner.cpp index 0925a4900a..3dcffd6c4c 100644 --- a/src/gromacs/options/tests/optionsassigner.cpp +++ b/src/gromacs/options/tests/optionsassigner.cpp @@ -59,6 +59,8 @@ #include "gromacs/utility/exceptions.h" #include "gromacs/utility/stringutil.h" +#include "testutils/testasserts.h" + namespace { @@ -94,9 +96,12 @@ TEST(OptionsAssignerTest, HandlesInvalidMultipleParameter) { gmx::Options options(NULL, NULL); std::vector values; + bool bIsSet; using gmx::IntegerOption; ASSERT_NO_THROW(options.addOption( - IntegerOption("p").storeVector(&values).multiValue())); + IntegerOption("p") + .storeVector(&values).storeIsSet(&bIsSet) + .multiValue())); gmx::OptionsAssigner assigner(&options); EXPECT_NO_THROW(assigner.start()); @@ -107,7 +112,7 @@ TEST(OptionsAssignerTest, HandlesInvalidMultipleParameter) EXPECT_NO_THROW(assigner.finish()); EXPECT_NO_THROW(options.finish()); - EXPECT_TRUE(options.isSet("p")); + EXPECT_TRUE(bIsSet); ASSERT_EQ(1U, values.size()); EXPECT_EQ(1, values[0]); } @@ -116,22 +121,25 @@ TEST(OptionsAssignerTest, HandlesMultipleParameter) { gmx::Options options(NULL, NULL); std::vector values; + bool bIsSet; using gmx::IntegerOption; ASSERT_NO_THROW(options.addOption( - IntegerOption("p").storeVector(&values).allowMultiple())); - - gmx::OptionsAssigner assigner(&options); - EXPECT_NO_THROW(assigner.start()); - ASSERT_NO_THROW(assigner.startOption("p")); - ASSERT_NO_THROW(assigner.appendValue("1")); - EXPECT_NO_THROW(assigner.finishOption()); - ASSERT_NO_THROW(assigner.startOption("p")); - ASSERT_NO_THROW(assigner.appendValue("2")); - EXPECT_NO_THROW(assigner.finishOption()); - EXPECT_NO_THROW(assigner.finish()); - EXPECT_NO_THROW(options.finish()); - - EXPECT_TRUE(options.isSet("p")); + IntegerOption("p") + .storeVector(&values).storeIsSet(&bIsSet) + .allowMultiple())); + + gmx::OptionsAssigner assigner(&options); + EXPECT_NO_THROW_GMX(assigner.start()); + ASSERT_NO_THROW_GMX(assigner.startOption("p")); + ASSERT_NO_THROW_GMX(assigner.appendValue("1")); + EXPECT_NO_THROW_GMX(assigner.finishOption()); + ASSERT_NO_THROW_GMX(assigner.startOption("p")); + ASSERT_NO_THROW_GMX(assigner.appendValue("2")); + EXPECT_NO_THROW_GMX(assigner.finishOption()); + EXPECT_NO_THROW_GMX(assigner.finish()); + EXPECT_NO_THROW_GMX(options.finish()); + + EXPECT_TRUE(bIsSet); ASSERT_EQ(2U, values.size()); EXPECT_EQ(1, values[0]); EXPECT_EQ(2, values[1]); diff --git a/src/gromacs/trajectoryanalysis/analysismodule.cpp b/src/gromacs/trajectoryanalysis/analysismodule.cpp index 0f1a437edc..06a2b84f87 100644 --- a/src/gromacs/trajectoryanalysis/analysismodule.cpp +++ b/src/gromacs/trajectoryanalysis/analysismodule.cpp @@ -1,7 +1,7 @@ /* * This file is part of the GROMACS molecular simulation package. * - * Copyright (c) 2010,2011,2012,2013,2014, by the GROMACS development team, led by + * Copyright (c) 2010,2011,2012,2013,2014,2015, by the GROMACS development team, led by * Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl, * and including many others, as listed in the AUTHORS file in the * top-level source directory and at http://www.gromacs.org. @@ -289,7 +289,6 @@ TrajectoryAnalysisModule::~TrajectoryAnalysisModule() void TrajectoryAnalysisModule::optionsFinished( - Options * /*options*/, TrajectoryAnalysisSettings * /*settings*/) { } diff --git a/src/gromacs/trajectoryanalysis/analysismodule.h b/src/gromacs/trajectoryanalysis/analysismodule.h index 6735b21c4c..8553342b4c 100644 --- a/src/gromacs/trajectoryanalysis/analysismodule.h +++ b/src/gromacs/trajectoryanalysis/analysismodule.h @@ -255,7 +255,6 @@ class TrajectoryAnalysisModule /*! \brief * Called after all option values have been set. * - * \param[in,out] options Options object in which options are stored. * \param[in,out] settings Settings to pass to and from the module. * * This method is called after option values have been assigned (but @@ -268,8 +267,7 @@ class TrajectoryAnalysisModule * * The default implementation does nothing. */ - virtual void optionsFinished(Options *options, - TrajectoryAnalysisSettings *settings); + virtual void optionsFinished(TrajectoryAnalysisSettings *settings); /*! \brief * Initializes the analysis. * diff --git a/src/gromacs/trajectoryanalysis/cmdlinerunner.cpp b/src/gromacs/trajectoryanalysis/cmdlinerunner.cpp index a32a591e35..056d3ac5ef 100644 --- a/src/gromacs/trajectoryanalysis/cmdlinerunner.cpp +++ b/src/gromacs/trajectoryanalysis/cmdlinerunner.cpp @@ -130,8 +130,8 @@ TrajectoryAnalysisCommandLineRunner::Impl::parseOptions( options.finish(); } - common->optionsFinished(&options); - module_->optionsFinished(&options, settings); + common->optionsFinished(); + module_->optionsFinished(settings); common->initIndexGroups(selections, bUseDefaultGroups_); diff --git a/src/gromacs/trajectoryanalysis/modules/angle.cpp b/src/gromacs/trajectoryanalysis/modules/angle.cpp index c9826aed25..5de3634684 100644 --- a/src/gromacs/trajectoryanalysis/modules/angle.cpp +++ b/src/gromacs/trajectoryanalysis/modules/angle.cpp @@ -56,7 +56,6 @@ #include "gromacs/options/basicoptions.h" #include "gromacs/options/filenameoption.h" #include "gromacs/options/ioptionscontainer.h" -#include "gromacs/options/options.h" #include "gromacs/pbcutil/pbc.h" #include "gromacs/selection/selection.h" #include "gromacs/selection/selectionoption.h" @@ -248,8 +247,7 @@ class Angle : public TrajectoryAnalysisModule virtual void initOptions(IOptionsContainer *options, TrajectoryAnalysisSettings *settings); - virtual void optionsFinished(Options *options, - TrajectoryAnalysisSettings *settings); + virtual void optionsFinished(TrajectoryAnalysisSettings *settings); virtual void initAnalysis(const TrajectoryAnalysisSettings &settings, const TopologyInformation &top); @@ -393,7 +391,7 @@ Angle::initOptions(IOptionsContainer *options, TrajectoryAnalysisSettings *setti void -Angle::optionsFinished(Options *options, TrajectoryAnalysisSettings * /* settings */) +Angle::optionsFinished(TrajectoryAnalysisSettings * /* settings */) { const bool bSingle = (g1type_[0] == 'a' || g1type_[0] == 'd'); @@ -402,7 +400,7 @@ Angle::optionsFinished(Options *options, TrajectoryAnalysisSettings * /* setting GMX_THROW(InconsistentInputError("Cannot use a second group (-g2) with " "-g1 angle or dihedral")); } - if (bSingle && options->isSet("group2")) + if (bSingle && sel2info_->isSet()) { GMX_THROW(InconsistentInputError("Cannot provide a second selection " "(-group2) with -g1 angle or dihedral")); @@ -434,7 +432,7 @@ Angle::optionsFinished(Options *options, TrajectoryAnalysisSettings * /* setting default: GMX_THROW(InternalError("invalid -g2 value")); } - if (natoms2_ == 0 && options->isSet("group2")) + if (natoms2_ == 0 && sel2info_->isSet()) { GMX_THROW(InconsistentInputError("Cannot provide a second selection (-group2) with -g2 t0 or z")); } diff --git a/src/gromacs/trajectoryanalysis/modules/rdf.cpp b/src/gromacs/trajectoryanalysis/modules/rdf.cpp index 5a11f9f369..a8d9b3bd39 100644 --- a/src/gromacs/trajectoryanalysis/modules/rdf.cpp +++ b/src/gromacs/trajectoryanalysis/modules/rdf.cpp @@ -62,7 +62,6 @@ #include "gromacs/options/basicoptions.h" #include "gromacs/options/filenameoption.h" #include "gromacs/options/ioptionscontainer.h" -#include "gromacs/options/options.h" #include "gromacs/pbcutil/pbc.h" #include "gromacs/selection/nbsearch.h" #include "gromacs/selection/selection.h" @@ -99,8 +98,7 @@ class Rdf : public TrajectoryAnalysisModule virtual void initOptions(IOptionsContainer *options, TrajectoryAnalysisSettings *settings); - virtual void optionsFinished(Options *options, - TrajectoryAnalysisSettings *settings); + virtual void optionsFinished(TrajectoryAnalysisSettings *settings); virtual void initAnalysis(const TrajectoryAnalysisSettings &settings, const TopologyInformation &top); virtual void initAfterFirstFrame(const TrajectoryAnalysisSettings &settings, @@ -172,6 +170,7 @@ class Rdf : public TrajectoryAnalysisModule double cutoff_; double rmax_; bool bNormalize_; + bool bNormalizationSet_; bool bXY_; bool bExclusions_; @@ -188,7 +187,8 @@ Rdf::Rdf() pairCounts_(new AnalysisDataSimpleHistogramModule()), normAve_(new AnalysisDataAverageModule()), binwidth_(0.002), cutoff_(0.0), rmax_(0.0), - bNormalize_(true), bXY_(false), bExclusions_(false), + bNormalize_(true), bNormalizationSet_(false), bXY_(false), + bExclusions_(false), cut2_(0.0), rmax2_(0.0), surfaceGroupCount_(0) { pairDist_.setMultipoint(true); @@ -253,6 +253,7 @@ Rdf::initOptions(IOptionsContainer *options, TrajectoryAnalysisSettings *setting options->addOption(DoubleOption("bin").store(&binwidth_) .description("Bin width (nm)")); options->addOption(BooleanOption("norm").store(&bNormalize_) + .storeIsSet(&bNormalizationSet_) .description("Normalize for bin volume and density")); options->addOption(BooleanOption("xy").store(&bXY_) .description("Use only the x and y components of the distance")); @@ -276,13 +277,13 @@ Rdf::initOptions(IOptionsContainer *options, TrajectoryAnalysisSettings *setting } void -Rdf::optionsFinished(Options *options, TrajectoryAnalysisSettings *settings) +Rdf::optionsFinished(TrajectoryAnalysisSettings *settings) { if (surface_ != "no") { settings->setFlag(TrajectoryAnalysisSettings::efRequireTop); - if (options->isSet("norm") && bNormalize_) + if (bNormalizationSet_ && bNormalize_) { GMX_THROW(InconsistentInputError("-surf cannot be combined with -norm")); } diff --git a/src/gromacs/trajectoryanalysis/modules/select.cpp b/src/gromacs/trajectoryanalysis/modules/select.cpp index 7a49a0e839..15bca787db 100644 --- a/src/gromacs/trajectoryanalysis/modules/select.cpp +++ b/src/gromacs/trajectoryanalysis/modules/select.cpp @@ -263,8 +263,7 @@ class Select : public TrajectoryAnalysisModule virtual void initOptions(IOptionsContainer *options, TrajectoryAnalysisSettings *settings); - virtual void optionsFinished(Options *options, - TrajectoryAnalysisSettings *settings); + virtual void optionsFinished(TrajectoryAnalysisSettings *settings); virtual void initAnalysis(const TrajectoryAnalysisSettings &settings, const TopologyInformation &top); @@ -439,8 +438,7 @@ Select::initOptions(IOptionsContainer *options, TrajectoryAnalysisSettings *sett } void -Select::optionsFinished(Options * /*options*/, - TrajectoryAnalysisSettings *settings) +Select::optionsFinished(TrajectoryAnalysisSettings *settings) { if (!fnPDB_.empty()) { diff --git a/src/gromacs/trajectoryanalysis/runnercommon.cpp b/src/gromacs/trajectoryanalysis/runnercommon.cpp index befa025b17..4483a2949d 100644 --- a/src/gromacs/trajectoryanalysis/runnercommon.cpp +++ b/src/gromacs/trajectoryanalysis/runnercommon.cpp @@ -93,6 +93,9 @@ class TrajectoryAnalysisRunnerCommon::Impl double startTime_; double endTime_; double deltaTime_; + bool bStartTimeSet_; + bool bEndTimeSet_; + bool bDeltaTimeSet_; gmx_ana_indexgrps_t *grps_; bool bTrajOpen_; @@ -108,6 +111,7 @@ class TrajectoryAnalysisRunnerCommon::Impl TrajectoryAnalysisRunnerCommon::Impl::Impl(TrajectoryAnalysisSettings *settings) : settings_(*settings), startTime_(0.0), endTime_(0.0), deltaTime_(0.0), + bStartTimeSet_(false), bEndTimeSet_(false), bDeltaTimeSet_(false), grps_(NULL), bTrajOpen_(false), fr(NULL), gpbc_(NULL), status_(NULL), oenv_(NULL) { @@ -190,11 +194,17 @@ TrajectoryAnalysisRunnerCommon::initOptions(IOptionsContainer *options) .description("Extra index groups")); // Add options for trajectory time control. - options->addOption(DoubleOption("b").store(&impl_->startTime_).timeValue() + options->addOption(DoubleOption("b") + .store(&impl_->startTime_).storeIsSet(&impl_->bStartTimeSet_) + .timeValue() .description("First frame (%t) to read from trajectory")); - options->addOption(DoubleOption("e").store(&impl_->endTime_).timeValue() + options->addOption(DoubleOption("e") + .store(&impl_->endTime_).storeIsSet(&impl_->bEndTimeSet_) + .timeValue() .description("Last frame (%t) to read from trajectory")); - options->addOption(DoubleOption("dt").store(&impl_->deltaTime_).timeValue() + options->addOption(DoubleOption("dt") + .store(&impl_->deltaTime_).storeIsSet(&impl_->bDeltaTimeSet_) + .timeValue() .description("Only use frame if t MOD dt == first time (%t)")); // Add time unit option. @@ -227,7 +237,7 @@ TrajectoryAnalysisRunnerCommon::scaleTimeOptions(Options *options) void -TrajectoryAnalysisRunnerCommon::optionsFinished(Options *options) +TrajectoryAnalysisRunnerCommon::optionsFinished() { impl_->settings_.impl_->plotSettings.setTimeUnit( impl_->settings_.impl_->timeUnitManager.timeUnit()); @@ -237,15 +247,15 @@ TrajectoryAnalysisRunnerCommon::optionsFinished(Options *options) GMX_THROW(InconsistentInputError("No trajectory or topology provided, nothing to do!")); } - if (options->isSet("b")) + if (impl_->bStartTimeSet_) { setTimeValue(TBEGIN, impl_->startTime_); } - if (options->isSet("e")) + if (impl_->bEndTimeSet_) { setTimeValue(TEND, impl_->endTime_); } - if (options->isSet("dt")) + if (impl_->bDeltaTimeSet_) { setTimeValue(TDELTA, impl_->deltaTime_); } diff --git a/src/gromacs/trajectoryanalysis/runnercommon.h b/src/gromacs/trajectoryanalysis/runnercommon.h index 82fea3bf79..119aa5a66e 100644 --- a/src/gromacs/trajectoryanalysis/runnercommon.h +++ b/src/gromacs/trajectoryanalysis/runnercommon.h @@ -83,12 +83,8 @@ class TrajectoryAnalysisRunnerCommon void initOptions(IOptionsContainer *options); //! Scales time option values according to the time unit set. void scaleTimeOptions(Options *options); - /*! \brief - * Processes common option values after they have been parsed. - * - * \param[in,out] options Options object in which options are stored. - */ - void optionsFinished(Options *options); + //! Processes common option values after they have been parsed. + void optionsFinished(); //! Initialize index groups for selections. void initIndexGroups(SelectionCollection *selections, bool bUseDefaults); diff --git a/src/testutils/refdata.cpp b/src/testutils/refdata.cpp index f9bab1ecf3..263ad7d53f 100644 --- a/src/testutils/refdata.cpp +++ b/src/testutils/refdata.cpp @@ -54,7 +54,7 @@ #include #include "gromacs/options/basicoptions.h" -#include "gromacs/options/options.h" +#include "gromacs/options/ioptionscontainer.h" #include "gromacs/utility/exceptions.h" #include "gromacs/utility/gmxassert.h" #include "gromacs/utility/path.h" @@ -97,7 +97,7 @@ namespace gmx namespace test { -void initReferenceData(Options *options) +void initReferenceData(IOptionsContainer *options) { // Needs to correspond to the enum order in refdata.h. const char *const refDataEnum[] = { "check", "create", "update" }; diff --git a/src/testutils/refdata.h b/src/testutils/refdata.h index acd1d9364b..083fbf38a9 100644 --- a/src/testutils/refdata.h +++ b/src/testutils/refdata.h @@ -52,7 +52,7 @@ namespace gmx { -class Options; +class IOptionsContainer; namespace test { @@ -103,7 +103,7 @@ enum ReferenceDataMode * * \ingroup module_testutils */ -void initReferenceData(Options *options); +void initReferenceData(IOptionsContainer *options); class TestReferenceChecker; -- 2.22.0