Merge pull request #3373 from verveerpj/improve_validation

Add keyword validation using a predicate function

opm/simulators/flow/KeywordValidation.cpp

@@ -109,6 +109,7 @@ namespace KeywordValidation
             // Otherwise, check all its items.
             validateKeywordItems(keyword, m_string_items, errors);
             validateKeywordItems(keyword, m_int_items, errors);
+            validateKeywordItems(keyword, m_double_items, errors);
         }
     }
 
@@ -152,8 +153,7 @@ namespace KeywordValidation
                                                const T& item_value,
                                                std::vector<ValidationError>& errors) const
     {
-        const auto& permitted = properties.permitted_values;
+        if (!properties.validator(item_value)) {
-        if (std::find(permitted.begin(), permitted.end(), item_value) == permitted.end()) {
             // If the value is not permitted, format the value to report it.
             std::string formatted_value;
             if constexpr (std::is_arithmetic<T>::value)

opm/simulators/flow/KeywordValidation.hpp

@@ -22,6 +22,8 @@
 
 #include <opm/common/OpmLog/KeywordLocation.hpp>
 
+#include <functional>
+#include <initializer_list>
 #include <map>
 #include <optional>
 #include <string>
@@ -47,7 +49,7 @@ namespace KeywordValidation
     template <typename T>
     struct PartiallySupportedKeywordProperties {
         bool critical; // Set to true if the unsupported item value should be an error
-        std::vector<T> permitted_values; // The list of permitted values
+        std::function<bool(T)> validator; // Predicate function to test values
         std::optional<std::string> message; // An optional message to show if an illegal item is encountered
     };
 
@@ -68,8 +70,8 @@ namespace KeywordValidation
     struct ValidationError {
         bool critical; // Determines if the encountered problem should be an error or a warning
         KeywordLocation location; // Location information (keyword name, file and line number)
-        size_t record_number; // Number of the offending record.
+        size_t record_number; // Number of the offending record
-        std::optional<size_t> item_number; // Number of the offending item, -1 if the kewyord is not supported at all.
+        std::optional<size_t> item_number; // Number of the offending item
         std::optional<std::string> item_value; // The offending value of a problematic item
         std::optional<std::string> user_message; // An optional message to show if a problem is encountered
     };
@@ -86,10 +88,12 @@ namespace KeywordValidation
     public:
         KeywordValidator(const UnsupportedKeywords& keywords,
                          const PartiallySupportedKeywords<std::string>& string_items,
-                         const PartiallySupportedKeywords<int>& int_items)
+                         const PartiallySupportedKeywords<int>& int_items,
+                         const PartiallySupportedKeywords<double>& double_items)
             : m_keywords(keywords)
             , m_string_items(string_items)
             , m_int_items(int_items)
+            , m_double_items(double_items)
         {
         }
 
@@ -121,6 +125,29 @@ namespace KeywordValidation
         const UnsupportedKeywords m_keywords;
         const PartiallySupportedKeywords<std::string> m_string_items;
         const PartiallySupportedKeywords<int> m_int_items;
+        const PartiallySupportedKeywords<double> m_double_items;
+    };
+
+
+    // Helper class to test if a given value is with a list of allowed values.
+    template <typename T>
+    class allow_values
+    {
+    public:
+        allow_values(const std::initializer_list<T>& allowed_values)
+        {
+            for (auto item : allowed_values) {
+                m_allowed_values.push_back(item);
+            }
+        }
+
+        bool operator()(const T& value) const
+        {
+            return std::find(m_allowed_values.begin(), m_allowed_values.end(), value) != m_allowed_values.end();
+        }
+
+    private:
+        std::vector<T> m_allowed_values;
     };
 
 

opm/simulators/utils/PartiallySupportedFlowKeywords.cpp

@@ -19,32 +19,33 @@
 
 #include <opm/simulators/utils/PartiallySupportedFlowKeywords.hpp>
 
+using namespace Opm::KeywordValidation;
 namespace Opm::FlowKeywordValidation
 {
 
-template<>
+template <>
-const KeywordValidation::PartiallySupportedKeywords<std::string>&
+const PartiallySupportedKeywords<std::string>&
 partiallySupported()
 {
-    static const KeywordValidation::PartiallySupportedKeywords<std::string> partially_supported_keywords_strings = {
+    static const PartiallySupportedKeywords<std::string> partially_supported_keywords_strings = {
         {
             "COMPORD",
             {
-                {2, {false, {"INPUT"}, std::nullopt}}, // ORDER_TYPE
+                {2, {false, allow_values<std::string>{"INPUT"}, std::nullopt}}, // ORDER_TYPE
             },
         },
         {
             "ENDSCALE",
             {
-                {1, {false, {"NODIR"}, std::nullopt}}, // DIRECT
+                {1, {false, allow_values<std::string>{"NODIR"}, std::nullopt}}, // DIRECT
-                {2, {false, {"REVERS"}, std::nullopt}}, // IRREVERS
+                {2, {false, allow_values<std::string>{"REVERS"}, std::nullopt}}, // IRREVERS
             },
         },
         {
             "PINCH",
             {
-                {2, {false, {"GAP"}, std::nullopt}}, // GAP
+                {2, {false, allow_values<std::string>{"GAP"}, std::nullopt}}, // GAP
-                {4, {false, {"TOPBOT"}, std::nullopt}}, // PINCHOUT_OPTION
+                {4, {false, allow_values<std::string>{"TOPBOT"}, std::nullopt}}, // PINCHOUT_OPTION
             },
         },
     };
@@ -52,15 +53,15 @@ partiallySupported()
     return partially_supported_keywords_strings;
 }
 
-template<>
+template <>
-const KeywordValidation::PartiallySupportedKeywords<int>&
+const PartiallySupportedKeywords<int>&
 partiallySupported()
 {
-    static const KeywordValidation::PartiallySupportedKeywords<int> partially_supported_keywords_int = {
+    static const PartiallySupportedKeywords<int> partially_supported_keywords_int = {
         {
             "EHYSTR",
             {
-                {2, {false, {0}, std::nullopt}}, //relative_perm_hyst
+                {2, {false, allow_values<int>{0}, std::nullopt}}, // relative_perm_hyst
             },
         },
     };
@@ -68,4 +69,13 @@ partiallySupported()
     return partially_supported_keywords_int;
 }
 
+template <>
+const PartiallySupportedKeywords<double>&
+partiallySupported()
+{
+    static const PartiallySupportedKeywords<double> partially_supported_keywords_double = {};
+
+    return partially_supported_keywords_double;
+}
+
 } // namespace Opm::FlowKeywordValidation

opm/simulators/utils/PartiallySupportedFlowKeywords.hpp

@@ -36,16 +36,17 @@
     This struct has the following fields:
 
     critical (bool) : if this is a critical error.
-    permitted_values (vector of strings) : the list of values that is allowed.
+    validator (function wrapper) : A function wrapper object that is used to test values.
     message (itemal string): an optional message to add to the error reported by flow.
 
-    Below is the set of partiall supported keywords, currently used by flow.
+    For convenience there is a small class KeywordValidation::allow_values which
+    can be initialized with a list of permitted values, and used as a validator.
 */
 
 namespace Opm::FlowKeywordValidation
 {
 
-template<typename T>
+template <typename T>
 const KeywordValidation::PartiallySupportedKeywords<T>& partiallySupported();
 
 } // namespace Opm::FlowKeywordValidation

opm/simulators/utils/readDeck.cpp

@@ -210,7 +210,8 @@ void readDeck(int rank, std::string& deckFilename, std::unique_ptr<Opm::Deck>& d
                 Opm::KeywordValidation::KeywordValidator keyword_validator(
                     Opm::FlowKeywordValidation::unsupportedKeywords(),
                     Opm::FlowKeywordValidation::partiallySupported<std::string>(),
-                    Opm::FlowKeywordValidation::partiallySupported<int>());
+                    Opm::FlowKeywordValidation::partiallySupported<int>(),
+                    Opm::FlowKeywordValidation::partiallySupported<double>());
                 keyword_validator.validateDeck(*deck, *parseContext, *errorGuard);
 
                 if ( checkDeck )

tests/test_keyword_validator.cpp

@@ -30,7 +30,6 @@
 using namespace Opm;
 using namespace Opm::KeywordValidation;
 
-
 const UnsupportedKeywords test_unsupported_keywords = {
     {"ECHO", {false, "This is not a critical error"}},
     {"NOECHO", {true, std::nullopt}},
@@ -41,8 +40,8 @@ const PartiallySupportedKeywords<std::string> test_string_items = {
     {
         "PINCH",
         {
-            {2, {false, {"GAP"}, std::nullopt}}, // GAP
+            {2, {false, allow_values<std::string>{"GAP", "BAR"}, std::nullopt}}, // GAP
-            {4, {true, {"TOPBOT"}, "This is a critical error"}}, // PINCHOUT_OPTION
+            {4, {true, allow_values<std::string>{"TOPBOT"}, "This is a critical error"}}, // PINCHOUT_OPTION
         },
     },
 };
@@ -52,15 +51,26 @@ const PartiallySupportedKeywords<int> test_int_items = {
     {
         "ENDSCALE",
         {
-            {3, {false, {1}, std::nullopt}}, // NTENDP
+            {3, {false, allow_values<int>{1}, std::nullopt}}, // NTENDP
-            {4, {true, {20}, std::nullopt}}, // NSENDP
+            {4, {true, allow_values<int>{20, 30, 40}, std::nullopt}}, // NSENDP
         },
     },
     {
         "COMPDAT",
         {
-            {2, {false, {1}, std::nullopt}}, // I
+            {2, {false, allow_values<int>{1}, std::nullopt}}, // I
-            {3, {false, {1}, std::nullopt}}, // J
+            {3, {false, allow_values<int>{1}, std::nullopt}}, // J
+        },
+    },
+};
+
+
+const PartiallySupportedKeywords<double> test_double_items = {
+    {
+        "EHYSTR",
+        {
+            {1, {false, [](double x) { return x > 0; }, std::nullopt}},
+            {3, {false, allow_values<double>{1.0, 2.0}, std::nullopt}},
         },
     },
 };
@@ -73,7 +83,7 @@ ECHO
 )"};
     const auto deck = Parser {}.parseString(keywords_string);
     const auto& test_keyword = deck.getKeyword("ECHO");
-    KeywordValidator validator(test_unsupported_keywords, test_string_items, test_int_items);
+    KeywordValidator validator(test_unsupported_keywords, test_string_items, test_int_items, test_double_items);
     std::vector<ValidationError> errors;
     validator.validateDeckKeyword(test_keyword, errors);
     BOOST_CHECK(!errors[0].critical);
@@ -90,7 +100,7 @@ NOECHO
 )"};
     const auto deck = Parser {}.parseString(keywords_string);
     const auto& test_keyword = deck.getKeyword("NOECHO");
-    KeywordValidator validator(test_unsupported_keywords, test_string_items, test_int_items);
+    KeywordValidator validator(test_unsupported_keywords, test_string_items, test_int_items, test_double_items);
     std::vector<ValidationError> errors;
     validator.validateDeckKeyword(test_keyword, errors);
     BOOST_CHECK(errors[0].critical);
@@ -108,7 +118,7 @@ PINCH
 )"};
     const auto deck = Parser {}.parseString(keywords_string);
     const auto& test_keyword = deck.getKeyword("PINCH");
-    KeywordValidator validator(test_unsupported_keywords, test_string_items, test_int_items);
+    KeywordValidator validator(test_unsupported_keywords, test_string_items, test_int_items, test_double_items);
     std::vector<ValidationError> errors;
     validator.validateDeckKeyword(test_keyword, errors);
     BOOST_CHECK(!errors[0].critical);
@@ -126,7 +136,7 @@ PINCH
 )"};
     const auto deck = Parser {}.parseString(keywords_string);
     const auto& test_keyword = deck.getKeyword("PINCH");
-    KeywordValidator validator(test_unsupported_keywords, test_string_items, test_int_items);
+    KeywordValidator validator(test_unsupported_keywords, test_string_items, test_int_items, test_double_items);
     std::vector<ValidationError> errors;
     validator.validateDeckKeyword(test_keyword, errors);
     BOOST_CHECK(errors[0].critical);
@@ -144,7 +154,7 @@ ENDSCALE
 )"};
     const auto deck = Parser {}.parseString(keywords_string);
     const auto& test_keyword = deck.getKeyword("ENDSCALE");
-    KeywordValidator validator(test_unsupported_keywords, test_string_items, test_int_items);
+    KeywordValidator validator(test_unsupported_keywords, test_string_items, test_int_items, test_double_items);
     std::vector<ValidationError> errors;
     validator.validateDeckKeyword(test_keyword, errors);
     BOOST_CHECK(!errors[0].critical);
@@ -162,7 +172,7 @@ ENDSCALE
 )"};
     const auto deck = Parser {}.parseString(keywords_string);
     const auto& test_keyword = deck.getKeyword("ENDSCALE");
-    KeywordValidator validator(test_unsupported_keywords, test_string_items, test_int_items);
+    KeywordValidator validator(test_unsupported_keywords, test_string_items, test_int_items, test_double_items);
     std::vector<ValidationError> errors;
     validator.validateDeckKeyword(test_keyword, errors);
     BOOST_CHECK(errors[0].critical);
@@ -171,6 +181,53 @@ ENDSCALE
     BOOST_CHECK(!errors[0].user_message);
 }
 
+BOOST_AUTO_TEST_CASE(non_critical_keyword_item_double_ok)
+{
+    const auto keywords_string = std::string {R"(
+EHYSTR
+   1.0 0 1.0/
+)"};
+    const auto deck = Parser {}.parseString(keywords_string);
+    const auto& test_keyword = deck.getKeyword("EHYSTR");
+    KeywordValidator validator(test_unsupported_keywords, test_string_items, test_int_items, test_double_items);
+    std::vector<ValidationError> errors;
+    validator.validateDeckKeyword(test_keyword, errors);
+    BOOST_CHECK(errors.size() == 0);
+}
+
+BOOST_AUTO_TEST_CASE(non_critical_keyword_item_double)
+{
+    const auto keywords_string = std::string {R"(
+EHYSTR
+   1.0 0 10.0 /
+)"};
+    const auto deck = Parser {}.parseString(keywords_string);
+    const auto& test_keyword = deck.getKeyword("EHYSTR");
+    KeywordValidator validator(test_unsupported_keywords, test_string_items, test_int_items, test_double_items);
+    std::vector<ValidationError> errors;
+    validator.validateDeckKeyword(test_keyword, errors);
+    BOOST_CHECK(!errors[0].critical);
+    BOOST_CHECK(errors[0].item_number == 3);
+    BOOST_CHECK(errors[0].item_value == "10.000000");
+    BOOST_CHECK(!errors[0].user_message);
+}
+
+BOOST_AUTO_TEST_CASE(non_critical_keyword_item_double_lambda)
+{
+    const auto keywords_string = std::string {R"(
+EHYSTR
+   -1.0 0 1.0 /
+)"};
+    const auto deck = Parser {}.parseString(keywords_string);
+    const auto& test_keyword = deck.getKeyword("EHYSTR");
+    KeywordValidator validator(test_unsupported_keywords, test_string_items, test_int_items, test_double_items);
+    std::vector<ValidationError> errors;
+    validator.validateDeckKeyword(test_keyword, errors);
+    BOOST_CHECK(!errors[0].critical);
+    BOOST_CHECK(errors[0].item_number == 1);
+    BOOST_CHECK(errors[0].item_value == "-1.000000");
+    BOOST_CHECK(!errors[0].user_message);
+}
 
 BOOST_AUTO_TEST_CASE(two_keyword_errors)
 {
@@ -183,7 +240,7 @@ ENDSCALE
     const auto deck = Parser {}.parseString(keywords_string);
     const auto& test_keyword1 = deck.getKeyword("PINCH");
     const auto& test_keyword2 = deck.getKeyword("ENDSCALE");
-    KeywordValidator validator(test_unsupported_keywords, test_string_items, test_int_items);
+    KeywordValidator validator(test_unsupported_keywords, test_string_items, test_int_items, test_double_items);
     std::vector<ValidationError> errors;
     validator.validateDeckKeyword(test_keyword1, errors);
     validator.validateDeckKeyword(test_keyword2, errors);
@@ -205,7 +262,7 @@ ECHO
 )"};
     const auto deck = Parser {}.parseString(keywords_string);
     const auto& test_keyword = deck.getKeyword("ECHO");
-    KeywordValidator validator(test_unsupported_keywords, test_string_items, test_int_items);
+    KeywordValidator validator(test_unsupported_keywords, test_string_items, test_int_items, test_double_items);
     std::vector<ValidationError> errors;
     validator.validateDeckKeyword(test_keyword, errors);
     const auto report = get_error_report(errors, false);
@@ -224,7 +281,7 @@ ECHO
 )"};
     const auto deck = Parser {}.parseString(keywords_string);
     const auto& test_keyword = deck.getKeyword("ECHO");
-    KeywordValidator validator(test_unsupported_keywords, test_string_items, test_int_items);
+    KeywordValidator validator(test_unsupported_keywords, test_string_items, test_int_items, test_double_items);
     std::vector<ValidationError> errors;
     validator.validateDeckKeyword(test_keyword, errors);
     const auto report = get_error_report(errors, true);
@@ -239,7 +296,7 @@ NOECHO
 )"};
     const auto deck = Parser {}.parseString(keywords_string);
     const auto& test_keyword = deck.getKeyword("NOECHO");
-    KeywordValidator validator(test_unsupported_keywords, test_string_items, test_int_items);
+    KeywordValidator validator(test_unsupported_keywords, test_string_items, test_int_items, test_double_items);
     std::vector<ValidationError> errors;
     validator.validateDeckKeyword(test_keyword, errors);
     const auto report = get_error_report(errors, true);
@@ -257,7 +314,7 @@ NOECHO
 )"};
     const auto deck = Parser {}.parseString(keywords_string);
     const auto& test_keyword = deck.getKeyword("NOECHO");
-    KeywordValidator validator(test_unsupported_keywords, test_string_items, test_int_items);
+    KeywordValidator validator(test_unsupported_keywords, test_string_items, test_int_items, test_double_items);
     std::vector<ValidationError> errors;
     validator.validateDeckKeyword(test_keyword, errors);
     const auto report = get_error_report(errors, false);
@@ -273,7 +330,7 @@ PINCH
 )"};
     const auto deck = Parser {}.parseString(keywords_string);
     const auto& test_keyword = deck.getKeyword("PINCH");
-    KeywordValidator validator(test_unsupported_keywords, test_string_items, test_int_items);
+    KeywordValidator validator(test_unsupported_keywords, test_string_items, test_int_items, test_double_items);
     std::vector<ValidationError> errors;
     validator.validateDeckKeyword(test_keyword, errors);
     const auto report = get_error_report(errors, false);
@@ -296,7 +353,7 @@ COMPDAT
 )"};
     const auto deck = Parser {}.parseString(keywords_string);
     const auto& test_keyword = deck.getKeyword("COMPDAT");
-    KeywordValidator validator(test_unsupported_keywords, test_string_items, test_int_items);
+    KeywordValidator validator(test_unsupported_keywords, test_string_items, test_int_items, test_double_items);
     std::vector<ValidationError> errors;
     validator.validateDeckKeyword(test_keyword, errors);
     const auto report = get_error_report(errors, false);
@@ -317,7 +374,7 @@ PINCH
 )"};
     const auto deck = Parser {}.parseString(keywords_string);
     const auto& test_keyword = deck.getKeyword("PINCH");
-    KeywordValidator validator(test_unsupported_keywords, test_string_items, test_int_items);
+    KeywordValidator validator(test_unsupported_keywords, test_string_items, test_int_items, test_double_items);
     std::vector<ValidationError> errors;
     validator.validateDeckKeyword(test_keyword, errors);
     const auto report = get_error_report(errors, true);
@@ -333,7 +390,7 @@ ENDSCALE
 )"};
     const auto deck = Parser {}.parseString(keywords_string);
     const auto& test_keyword = deck.getKeyword("ENDSCALE");
-    KeywordValidator validator(test_unsupported_keywords, test_string_items, test_int_items);
+    KeywordValidator validator(test_unsupported_keywords, test_string_items, test_int_items, test_double_items);
     std::vector<ValidationError> errors;
     validator.validateDeckKeyword(test_keyword, errors);
     const auto report = get_error_report(errors, false);
@@ -352,7 +409,7 @@ ENDSCALE
 )"};
     const auto deck = Parser {}.parseString(keywords_string);
     const auto& test_keyword = deck.getKeyword("ENDSCALE");
-    KeywordValidator validator(test_unsupported_keywords, test_string_items, test_int_items);
+    KeywordValidator validator(test_unsupported_keywords, test_string_items, test_int_items, test_double_items);
     std::vector<ValidationError> errors;
     validator.validateDeckKeyword(test_keyword, errors);
     const auto report = get_error_report(errors, true);
@@ -368,7 +425,7 @@ ENDSCALE
 )"};
     const auto deck = Parser {}.parseString(keywords_string);
     const auto& test_keyword = deck.getKeyword("ENDSCALE");
-    KeywordValidator validator(test_unsupported_keywords, test_string_items, test_int_items);
+    KeywordValidator validator(test_unsupported_keywords, test_string_items, test_int_items, test_double_items);
     std::vector<ValidationError> errors;
     validator.validateDeckKeyword(test_keyword, errors);
     const auto report = get_error_report(errors, true);
@@ -387,7 +444,7 @@ ENDSCALE
 )"};
     const auto deck = Parser {}.parseString(keywords_string);
     const auto& test_keyword = deck.getKeyword("ENDSCALE");
-    KeywordValidator validator(test_unsupported_keywords, test_string_items, test_int_items);
+    KeywordValidator validator(test_unsupported_keywords, test_string_items, test_int_items, test_double_items);
     std::vector<ValidationError> errors;
     validator.validateDeckKeyword(test_keyword, errors);
     const auto report = get_error_report(errors, false);
@@ -410,7 +467,7 @@ ENDSCALE
     const auto& test_keyword2 = deck.getKeyword("NOECHO");
     const auto& test_keyword3 = deck.getKeyword("PINCH");
     const auto& test_keyword4 = deck.getKeyword("ENDSCALE");
-    KeywordValidator validator(test_unsupported_keywords, test_string_items, test_int_items);
+    KeywordValidator validator(test_unsupported_keywords, test_string_items, test_int_items, test_double_items);
     std::vector<ValidationError> errors;
     validator.validateDeckKeyword(test_keyword1, errors);
     validator.validateDeckKeyword(test_keyword2, errors);
@@ -441,7 +498,7 @@ ENDSCALE
     const auto& test_keyword2 = deck.getKeyword("NOECHO");
     const auto& test_keyword3 = deck.getKeyword("PINCH");
     const auto& test_keyword4 = deck.getKeyword("ENDSCALE");
-    KeywordValidator validator(test_unsupported_keywords, test_string_items, test_int_items);
+    KeywordValidator validator(test_unsupported_keywords, test_string_items, test_int_items, test_double_items);
     std::vector<ValidationError> errors;
     validator.validateDeckKeyword(test_keyword1, errors);
     validator.validateDeckKeyword(test_keyword2, errors);