Support normalizing values in KeyValueTree

Make it possible to normalize values stored in a KeyValueTree to match
an Options structure.  This makes the types etc. consistent in the tree.
This is mainly useful for reducing variability in tpr files for better
forward/backward compatibility management, but can also be nice for
producing consistent mdp output files from grompp (if we want to keep
that feature).

Change-Id: Ib5ea03faaa690c84076c1ada0c0a08ddf9a1a128

diff --git a/src/gromacs/options/abstractoption.cpp b/src/gromacs/options/abstractoption.cpp
index 74cbe53a888136433a4d7fa1fa3cb9860e73918d..1439837fdee251798c4a2bca175f8a57d8a9d63f 100644 (file)
--- a/src/gromacs/options/abstractoption.cpp
+++ b/src/gromacs/options/abstractoption.cpp
@@ -271,4 +271,9 @@ std::vector<std::string> OptionInfo::defaultValuesAsStrings() const
     return option().defaultValuesAsStrings();
 }
 
+std::vector<Variant> OptionInfo::normalizeValues(const std::vector<Variant> &values) const
+{
+    return option().normalizeValues(values);
+}
+
 } // namespace gmx

diff --git a/src/gromacs/options/abstractoption.h b/src/gromacs/options/abstractoption.h
index ac9c758670f7538db4d4d5f32a2325d077ffb059..d3f8050b6f00b5a73a676f2a4641a4892745cf20 100644 (file)
--- a/src/gromacs/options/abstractoption.h
+++ b/src/gromacs/options/abstractoption.h
@@ -507,6 +507,17 @@ class OptionInfo
          * assigned.
          */
         std::vector<std::string> defaultValuesAsStrings() const;
+        /*! \brief
+         * Converts given values to native representation for this option.
+         *
+         * For example, strings are parsed to the type that is actually used to
+         * store the options.
+         *
+         * The return value only depends on the option type, not on the current
+         * value of the option, and the current value in the option is not
+         * changed.
+         */
+        std::vector<Variant> normalizeValues(const std::vector<Variant> &values) const;
 
     protected:
         /*! \cond libapi */

diff --git a/src/gromacs/options/abstractoptionstorage.h b/src/gromacs/options/abstractoptionstorage.h
index 6875fcaf43e68950e7c81c2674a34a6fa522cf1a..dd4e2eae764d2a4413f70d05794026bd2ec8724f 100644 (file)
--- a/src/gromacs/options/abstractoptionstorage.h
+++ b/src/gromacs/options/abstractoptionstorage.h
@@ -143,6 +143,8 @@ class AbstractOptionStorage
         virtual std::vector<Variant> defaultValues() const = 0;
         //! \copydoc OptionInfo::defaultValuesAsStrings()
         virtual std::vector<std::string> defaultValuesAsStrings() const = 0;
+        //! \copydoc OptionInfo::normalizeValues()
+        virtual std::vector<Variant> normalizeValues(const std::vector<Variant> &values) const = 0;
 
         /*! \brief
          * Starts adding values from a new source for the option.

diff --git a/src/gromacs/options/basicoptions.cpp b/src/gromacs/options/basicoptions.cpp
index 2364b7d8f6d10bf1dfd07176591daea28dbecaee..4e2d83eb8c6246905453b3ca9543bcbda77f7f3f 100644 (file)
--- a/src/gromacs/options/basicoptions.cpp
+++ b/src/gromacs/options/basicoptions.cpp
@@ -277,7 +277,7 @@ void DoubleOptionStorage::initConverter(ConverterType *converter)
     converter->addCastConversion<float>();
 }
 
-double DoubleOptionStorage::processValue(const double &value)
+double DoubleOptionStorage::processValue(const double &value) const
 {
     // TODO: Consider testing for overflow when scaling with factor_.
     return value * factor_;
@@ -370,7 +370,7 @@ void FloatOptionStorage::initConverter(ConverterType *converter)
     converter->addCastConversion<double>();
 }
 
-float FloatOptionStorage::processValue(const float &value)
+float FloatOptionStorage::processValue(const float &value) const
 {
     // TODO: Consider testing for overflow when scaling with factor_.
     return value * factor_;
@@ -504,7 +504,7 @@ void StringOptionStorage::initConverter(ConverterType * /*converter*/)
 {
 }
 
-std::string StringOptionStorage::processValue(const std::string &value)
+std::string StringOptionStorage::processValue(const std::string &value) const
 {
     if (allowed_.size() > 0)
     {

diff --git a/src/gromacs/options/basicoptionstorage.h b/src/gromacs/options/basicoptionstorage.h
index e4ae7e3441df75d803fd11ddd7db5febcdf4a0c0..92aefa31fa6065a0b0693fb59a1ec797a97b412d 100644 (file)
--- a/src/gromacs/options/basicoptionstorage.h
+++ b/src/gromacs/options/basicoptionstorage.h
@@ -151,7 +151,7 @@ class DoubleOptionStorage : public OptionStorageTemplateSimple<double>
 
     private:
         virtual void initConverter(ConverterType *converter);
-        virtual double processValue(const double &value);
+        virtual double processValue(const double &value) const;
         virtual void processSetValues(ValueList *values);
 
         DoubleOptionInfo        info_;
@@ -179,7 +179,7 @@ class FloatOptionStorage : public OptionStorageTemplateSimple<float>
 
     private:
         virtual void initConverter(ConverterType *converter);
-        virtual float processValue(const float &value);
+        virtual float processValue(const float &value) const;
         virtual void processSetValues(ValueList *values);
 
         FloatOptionInfo         info_;
@@ -207,7 +207,7 @@ class StringOptionStorage : public OptionStorageTemplateSimple<std::string>
 
     private:
         virtual void initConverter(ConverterType *converter);
-        virtual std::string processValue(const std::string &value);
+        virtual std::string processValue(const std::string &value) const;
 
         StringOptionInfo        info_;
         ValueList               allowed_;

diff --git a/src/gromacs/options/filenameoption.cpp b/src/gromacs/options/filenameoption.cpp
index a23b6469fae4adeb5f2df69130b84e6f09039786..746ea5d2f8ae63930a34377c1cb8401c6f0cb113 100644 (file)
--- a/src/gromacs/options/filenameoption.cpp
+++ b/src/gromacs/options/filenameoption.cpp
@@ -317,7 +317,7 @@ void FileNameOptionStorage::initConverter(ConverterType * /*converter*/)
 {
 }
 
-std::string FileNameOptionStorage::processValue(const std::string &value)
+std::string FileNameOptionStorage::processValue(const std::string &value) const
 {
     if (manager_ != NULL)
     {

diff --git a/src/gromacs/options/filenameoptionstorage.h b/src/gromacs/options/filenameoptionstorage.h
index 26ad20e5b99e61c86b599e196a2e956faa1f7018..97fda90324b99ce8d9adb8780d0acf496fa3cba9 100644 (file)
--- a/src/gromacs/options/filenameoptionstorage.h
+++ b/src/gromacs/options/filenameoptionstorage.h
@@ -101,7 +101,7 @@ class FileNameOptionStorage : public OptionStorageTemplateSimple<std::string>
 
     private:
         virtual void initConverter(ConverterType *converter);
-        virtual std::string processValue(const std::string &value);
+        virtual std::string processValue(const std::string &value) const;
         virtual void processAll();
 
         FileNameOptionInfo      info_;

diff --git a/src/gromacs/options/optionstoragetemplate.h b/src/gromacs/options/optionstoragetemplate.h
index fa67dc40b45549b627af9cce9af7b4e38c879162..6ccc2915150da27b609d89ccfa1317bf9438f74a 100644 (file)
--- a/src/gromacs/options/optionstoragetemplate.h
+++ b/src/gromacs/options/optionstoragetemplate.h
@@ -327,6 +327,20 @@ class OptionStorageTemplateSimple : public OptionStorageTemplate<T>
         {
         }
 
+        virtual std::vector<Variant>
+        normalizeValues(const std::vector<Variant> &values) const
+        {
+            const_cast<MyBase *>(this)->ensureConverterInitialized();
+            std::vector<Variant> result;
+            for (const auto &value : values)
+            {
+                result.push_back(
+                        Variant::create<T>(
+                                processValue(converter_.convert(value))));
+            }
+            return result;
+        }
+
         /*! \brief
          * Specifies how different types are converted.
          *
@@ -341,20 +355,24 @@ class OptionStorageTemplateSimple : public OptionStorageTemplate<T>
          *
          * The default implementation only provides an identity mapping.
          */
-        virtual T processValue(const T &value)
+        virtual T processValue(const T &value) const
         {
             return value;
         }
 
     private:
         virtual void convertValue(const Variant &variant)
+        {
+            ensureConverterInitialized();
+            this->addValue(processValue(converter_.convert(variant)));
+        }
+        void ensureConverterInitialized()
         {
             if (!initialized_)
             {
                 initConverter(&converter_);
                 initialized_ = true;
             }
-            this->addValue(processValue(converter_.convert(variant)));
         }
 
         ConverterType  converter_;

diff --git a/src/gromacs/options/tests/abstractoptionstorage.cpp b/src/gromacs/options/tests/abstractoptionstorage.cpp
index 729607aeec2742b686ec2fd711755e33035c52a5..1cfab3a8a6eb4de64dad1519d15bc823744da804 100644 (file)
--- a/src/gromacs/options/tests/abstractoptionstorage.cpp
+++ b/src/gromacs/options/tests/abstractoptionstorage.cpp
@@ -110,6 +110,11 @@ class MockOptionStorage : public gmx::OptionStorageTemplate<std::string>
         {
             return "";
         }
+        virtual std::vector<gmx::Variant>
+        normalizeValues(const std::vector<gmx::Variant> &values) const
+        {
+            return values;
+        }
 
         virtual void convertValue(const gmx::Variant &value)
         {

diff --git a/src/gromacs/options/tests/refdata/TreeValueSupportAdjustTest_NormalizesValues.xml b/src/gromacs/options/tests/refdata/TreeValueSupportAdjustTest_NormalizesValues.xml
new file mode 100644 (file)
index 0000000..b7f224f
--- /dev/null
+++ b/src/gromacs/options/tests/refdata/TreeValueSupportAdjustTest_NormalizesValues.xml
@@ -0,0 +1,10 @@
+<?xml version="1.0"?>
+<?xml-stylesheet type="text/xsl" href="referencedata.xsl"?>
+<ReferenceData>
+  <Object Name="Input">
+    <String Name="a">2</String>
+  </Object>
+  <Object Name="Output">
+    <Int Name="a">2</Int>
+  </Object>
+</ReferenceData>

diff --git a/src/gromacs/options/tests/treesupport.cpp b/src/gromacs/options/tests/treesupport.cpp
index 08cb46380bc77881120e637fffc39ce9acf185ee..cf1470733382dd62f80c223e442d4c32d0487582 100644 (file)
--- a/src/gromacs/options/tests/treesupport.cpp
+++ b/src/gromacs/options/tests/treesupport.cpp
@@ -189,6 +189,13 @@ TEST_F(TreeValueSupportAdjustTest, FillsDefaultObjectValues)
     runTest();
 }
 
+TEST_F(TreeValueSupportAdjustTest, NormalizesValues)
+{
+    options_.addOption(gmx::IntegerOption("a"));
+    builder_.rootObject().addValue<std::string>("a", "2");
+    runTest();
+}
+
 TEST_F(TreeValueSupportAdjustTest, MergesDefaultValues)
 {
     builder_.rootObject().addValue<int>("b", 1);

diff --git a/src/gromacs/options/treesupport.cpp b/src/gromacs/options/treesupport.cpp
index 3a24a320f2d2a834fefa1c3889644bc357cfb390..2a45831647dfdb91df1ebbabd35298b002563ac0 100644 (file)
--- a/src/gromacs/options/treesupport.cpp
+++ b/src/gromacs/options/treesupport.cpp
@@ -195,7 +195,38 @@ class TreeIterationHelper : private OptionsVisitor
             }
             else
             {
-                currentObjectBuilder_.addRawValue(name, KeyValueTreeValue((*currentSourceObject_)[name]));
+                const KeyValueTreeValue &value = (*currentSourceObject_)[name];
+                GMX_RELEASE_ASSERT(!value.isObject(), "Value objects not supported in this context");
+                std::vector<Variant>     values;
+                if (value.isArray())
+                {
+                    for (const auto &arrayValue : value.asArray().values())
+                    {
+                        GMX_RELEASE_ASSERT(!value.isObject() && !value.isArray(),
+                                           "Complex values not supported in this context");
+                        values.push_back(arrayValue.asVariant());
+                    }
+                }
+                else
+                {
+                    values.push_back(value.asVariant());
+                }
+                values = option.normalizeValues(values);
+                if (values.empty())
+                {
+                }
+                else if (values.size() == 1)
+                {
+                    currentObjectBuilder_.addRawValue(name, std::move(values[0]));
+                }
+                else
+                {
+                    auto array = currentObjectBuilder_.addArray(name);
+                    for (auto &arrayValue : values)
+                    {
+                        array.addRawValue(std::move(arrayValue));
+                    }
+                }
             }
         }
 

diff --git a/src/gromacs/options/treesupport.h b/src/gromacs/options/treesupport.h
index fe6ebf97015c30ebd4ce026864ceb9922360db7e..8fc4322c775f061dfe6617b6d9e7a59511669ab8 100644 (file)
--- a/src/gromacs/options/treesupport.h
+++ b/src/gromacs/options/treesupport.h
@@ -69,9 +69,11 @@ void assignOptionsFromKeyValueTree(Options                   *options,
  *
  * Assumes that all values in the input KeyValueTreeObject are valid values for
  * the options.  The output has all the values in the input, but in the order
- * they are in the options.  For any option that does not have a corresponding
- * value in the input, the output has it with a default value (if one exists
- * for the option).
+ * they are in the options.  Values are also converted to the native type for
+ * the underlying option (e.g., strings are parsed to integers if the option
+ * accepts those).  For any option that does not have a corresponding value in
+ * the input, the output has it with a default value (if one exists for the
+ * option).
  *
  * Does not currently work for option sections in an array.
  *

diff --git a/src/gromacs/selection/selectionfileoptionstorage.h b/src/gromacs/selection/selectionfileoptionstorage.h
index c2643e680772de8039cba5743a7ae8c9e2dc838a..2d45bf55f1da3a1c1993ea68b29672128df33c15 100644 (file)
--- a/src/gromacs/selection/selectionfileoptionstorage.h
+++ b/src/gromacs/selection/selectionfileoptionstorage.h
@@ -73,6 +73,8 @@ class SelectionFileOptionStorage : public AbstractOptionStorage
         virtual int valueCount() const { return 0; }
         virtual std::vector<Variant> defaultValues() const { return {}; }
         virtual std::vector<std::string> defaultValuesAsStrings() const { return {}; }
+        virtual std::vector<Variant>
+        normalizeValues(const std::vector<Variant> &values) const { return values; }
 
     private:
         virtual void clearSet();

diff --git a/src/gromacs/selection/selectionoption.cpp b/src/gromacs/selection/selectionoption.cpp
index 909e3f0e0ca3acc93ae06c8e67d340660ea3a856..1343907baa3ef72cddf657079e1afd689b9df968 100644 (file)
--- a/src/gromacs/selection/selectionoption.cpp
+++ b/src/gromacs/selection/selectionoption.cpp
@@ -84,6 +84,13 @@ std::string SelectionOptionStorage::formatSingleValue(const Selection &value) co
 }
 
 
+std::vector<Variant>
+SelectionOptionStorage::normalizeValues(const std::vector<Variant> & /*values*/) const
+{
+    GMX_THROW(NotImplementedError("Selection options not supported in this context"));
+}
+
+
 void SelectionOptionStorage::addSelections(
         const SelectionList &selections,
         bool                 bFullValue)

diff --git a/src/gromacs/selection/selectionoptionstorage.h b/src/gromacs/selection/selectionoptionstorage.h
index 7ccec2901cdf40cffd9ab6fab6c7419fca719bbf..795bdffa4c90cc53ecd627ab2ebcd2d2122b1e9b 100644 (file)
--- a/src/gromacs/selection/selectionoptionstorage.h
+++ b/src/gromacs/selection/selectionoptionstorage.h
@@ -77,6 +77,8 @@ class SelectionOptionStorage : public OptionStorageTemplate<Selection>
         virtual OptionInfo &optionInfo() { return info_; }
         virtual std::string typeString() const { return "selection"; }
         virtual std::string formatSingleValue(const Selection &value) const;
+        virtual std::vector<Variant>
+        normalizeValues(const std::vector<Variant> &values) const;
 
         /*! \brief
          * Adds selections to the storage.