Add Existence Check for Array Operations

For operations other than assignment (EQUALS keyword), this commit
ensures that the array being mutated already exists in the input.
In particular, this means that a request of the form

    ADD
      SGU 0.123 /
    /

will halt input processing and generate an "Error" of the form

    Error: Problem with keyword ADD
    In USER_ERR_ADD.DATA line 202
    Target array SGU must already exist when operated upon in ADD.

unless the 'SGU' array has already been assigned earlier.

To this end, repurpose the second parameter to member function

    FieldProps::try_get<>()

and make this a bitmask of flags instead of a single 'bool'.  The
currently supported flags are 'AllowUnsupported' and 'MustExist'
with the former representing the original 'bool' parameter and the
second requesting that the property array must already exist for the
request to succeed.

opm/input/eclipse/EclipseState/Grid/FieldProps.cpp

@@ -1514,6 +1514,31 @@ void FieldProps::handle_operation(const Section      section,
                                   const DeckKeyword& keyword,
                                   Box                box)
 {
+    // Keyword handler for ADD, EQUALS, MAXVALUE, MINVALUE, and MULTIPLY.
+    //
+    // General keyword structure:
+    //
+    //   KWNAME -- e.g., ADD or EQUALS
+    //     Array Scalar Box /
+    //     Array Scalar Box /
+    //   -- ...
+    //   /
+    //
+    // For example
+    //
+    //    ADD
+    //      PERMX  12.3  1 10   2 4   5 6 /
+    //    /
+    //
+    // which adds 12.3 mD to the PERMX array in all cells within the box
+    // {1..10, 2..4, 5..6}.  The array being operated on must already exist
+    // and be well defined for all elements in the box unless the operation
+    // is "EQUALS" or we're processing one of the TRAN* arrays.  Final TRAN*
+    // array processing is deferred to a later stage and at this point we're
+    // only collecting descriptors of what operations to apply when we get
+    // to that stage.
+
+    const auto mustExist = keyword.name() != ParserKeywords::EQUALS::keywordName;
     const auto editSect  = section == Section::EDIT;
     const auto operation = fromString(keyword.name());
 
@@ -1529,11 +1554,8 @@ void FieldProps::handle_operation(const Section      section,
             FieldProps::supported<double>(target_kw) ||
             (tran_iter != this->tran.end()))
         {
-            const auto scalar_value = this->
-                getSIValue(operation, target_kw, record.getItem(1).get<double>(0));
-
             auto kw_info = Fieldprops::keywords::global_kw_info<double>
-                (target_kw, tran_iter != this->tran.end());
+                (target_kw, /* allow_unsupported = */ tran_iter != this->tran.end());
 
             auto unique_name = target_kw;
 
@@ -1558,6 +1580,17 @@ void FieldProps::handle_operation(const Section      section,
                     unique_name = tran_field_iter->second;
                 }
             }
+            else if (mustExist && (this->double_data.find(unique_name) == this->double_data.end())) {
+                throw OpmInputError {
+                    fmt::format("Target array {} must already "
+                                "exist when operated upon in {}.",
+                                target_kw, keyword.name()),
+                    keyword.location()
+                };
+            }
+
+            const auto scalar_value = this->
+                getSIValue(operation, target_kw, record.getItem(1).get<double>(0));
 
             auto& field_data = this->init_get<double>
                 (unique_name, kw_info, editSect && kw_info.multiplier);
@@ -1577,6 +1610,15 @@ void FieldProps::handle_operation(const Section      section,
         }
 
         if (FieldProps::supported<int>(target_kw)) {
+            if (mustExist && (this->int_data.find(target_kw) == this->int_data.end())) {
+                throw OpmInputError {
+                    fmt::format("Target array {} must already "
+                                "exist when operated upon in {}.",
+                                target_kw, keyword.name()),
+                    keyword.location()
+                };
+            }
+
             const auto scalar_value = static_cast<int>(record.getItem(1).get<double>(0));
 
             auto& field_data = this->init_get<int>(target_kw);
@@ -1634,8 +1676,8 @@ void FieldProps::handle_COPY(const DeckKeyword& keyword,
         }
 
         if (FieldProps::supported<double>(src_kw)) {
-            const auto& src_data = this->try_get<double>(src_kw);
-            src_data.verify_status();
+            const auto& src_data = this->try_get<double>(src_kw, TryGetFlags::MustExist);
+            src_data.verify_status(keyword.location(), "Source array", "COPY");
 
             auto& target_data = this->init_get<double>(target_kw);
             target_data.checkInitialisedCopy(src_data.field_data(), index_list,
@@ -1645,8 +1687,8 @@ void FieldProps::handle_COPY(const DeckKeyword& keyword,
         }
 
         if (FieldProps::supported<int>(src_kw)) {
-            const auto& src_data = this->try_get<int>(src_kw);
-            src_data.verify_status();
+            const auto& src_data = this->try_get<int>(src_kw, TryGetFlags::MustExist);
+            src_data.verify_status(keyword.location(), "Source array", "COPY");
 
             auto& target_data = this->init_get<int>(target_kw);
             target_data.checkInitialisedCopy(src_data.field_data(), index_list,

opm/input/eclipse/EclipseState/Grid/FieldProps.hpp

@@ -307,7 +307,7 @@ keyword_info<T> global_kw_info(const std::string& name, bool allow_unsupported =
 bool is_oper_keyword(const std::string& name);
 } // end namespace keywords
 
-} // end namespace FieldProps
+} // end namespace Fieldprops
 
 class FieldProps {
 public:
@@ -334,26 +334,76 @@ class FieldProps {
         }
     };
 
+    /// Property array existence status
     enum class GetStatus {
-         OK = 1,
-         INVALID_DATA = 2,               // std::runtime_error
-         MISSING_KEYWORD = 3,            // std::out_of_range
-         NOT_SUPPPORTED_KEYWORD = 4      // std::logic_error
+        /// Property exists and its property data is well formed.
+        OK = 1,
+
+        /// Property array has not been fully initialised.
+        ///
+        /// At least one data element is uninitialised or has an empty
+        /// default.
+        ///
+        /// Will throw an exception of type \code std::runtime_error
+        /// \endcode in FieldDataManager::verify_status().
+        INVALID_DATA = 2,
+
+        /// Property has not yet been defined in the input file.
+        ///
+        /// Will throw an exception of type \code std::out_of_range \endcode
+        /// in FieldDataManager::verify_status().
+        MISSING_KEYWORD = 3,
+
+        /// Named property is not known to the internal handling mechanism.
+        ///
+        /// Might for instance be because the client code requested a
+        /// property of type \c int, but the name is only known as property
+        /// of type \c double, or because there was a misprint in the
+        /// property name.
+        ///
+        /// Will throw an exception of type \code std::logic_error \endcode
+        /// in FieldDataManager::verify_status().
+        NOT_SUPPPORTED_KEYWORD = 4,
     };
 
+    /// Wrapper type for field properties
+    ///
+    /// \tparam T Property element type.  Typically \c double or \c int.
     template<typename T>
     struct FieldDataManager
     {
+        /// Property name.
         const std::string& keyword;
+
+        /// Request status.
         GetStatus status;
+
+        /// Property data.
         const Fieldprops::FieldData<T>* data_ptr;
 
+        /// Constructor
+        ///
+        /// \param[in] k Property name
+        /// \param[in] s Request status
+        /// \param[in] d Property data.  Pass \c nullptr for missing property data.
         FieldDataManager(const std::string& k, GetStatus s, const Fieldprops::FieldData<T>* d)
             : keyword(k)
             , status(s)
             , data_ptr(d)
         {}
 
+        /// Validate result of \code try_get<>() \endcode request
+        ///
+        /// Throws an exception of type \code OpmInputError \endcode if
+        /// \code this->status \endcode is not \code GetStatus::OK \endcode.
+        /// Does nothing otherwise.
+        ///
+        /// \param[in] Input keyword which prompted request
+        ///
+        /// \param[in] descr Textual description of context in which request
+        ///   occurred.
+        ///
+        /// \param[in] operation Name of operation which prompted request.
         void verify_status(const KeywordLocation& loc,
                            const std::string&     descr,
                            const std::string&     operation) const
@@ -385,6 +435,11 @@ class FieldProps {
             }
         }
 
+        /// Validate result of \code try_get<>() \endcode request
+        ///
+        /// Throws an exception as outlined in \c GetStatus if \code
+        /// this->status \endcode is not \code GetStatus::OK \endcode.  Does
+        /// nothing otherwise.
         void verify_status() const
         {
             switch (status) {
@@ -402,31 +457,66 @@ class FieldProps {
             }
         }
 
+        /// Access underlying property data elements
+        ///
+        /// Returns \c nullptr if property data is not well formed.
         const std::vector<T>* ptr() const
         {
             return (this->data_ptr != nullptr)
                 ? &this->data_ptr->data
                 : nullptr;
         }
 
+        /// Access underlying property data elements
+        ///
+        /// Throws an exception as outlined in \c GetStatus if property data
+        /// is not well formed.
         const std::vector<T>& data() const
         {
             this->verify_status();
             return this->data_ptr->data;
         }
 
+        /// Read-only access to contained FieldData object
+        ///
+        /// Throws an exception as outlined in \c GetStatus if property data
+        /// is not well formed.
         const Fieldprops::FieldData<T>& field_data() const
         {
             this->verify_status();
             return *this->data_ptr;
         }
 
+        /// Property validity predicate.
+        ///
+        /// Returns true if propery exists and has well defined data
+        /// elements.  False otherwise.
         bool valid() const
         {
             return this->status == GetStatus::OK;
         }
     };
 
+    /// Options to restrict or relax a try_get() request.
+    ///
+    /// For instance, the source array of a COPY operation must exist and be
+    /// fully defined, whereas the target array need not be either.  On the
+    /// other hand, certain use cases might want to look up property names
+    /// of an unsupported keyword type--e.g., transmissibility keywords
+    /// among the integer properties--and those requests should specify the
+    /// AllowUnsupported flag.
+    ///
+    /// Note: We would ideally use strong enums (i.e., "enum class") for
+    /// this, but those don't mesh very well with bitwise operations.
+    enum TryGetFlags : unsigned int {
+        /// Whether or not to permit looking up property names of unmatching
+        /// types.
+        AllowUnsupported = (1u << 0),
+
+        /// Whether or not the property must already exist.
+        MustExist = (1u << 1),
+    };
+
     /// Normal constructor for FieldProps.
     FieldProps(const Deck& deck, const Phases& phases, EclipseGrid& grid, const TableManager& table_arg);
 
@@ -451,15 +541,36 @@ class FieldProps {
     template <typename T>
     std::vector<std::string> keys() const;
 
+    /// Request read-only property array from internal cache
+    ///
+    /// Will create property array if permitted and possible.
+    ///
+    /// \tparam T Property element type.  Typically \c double or \c int.
+    ///
+    /// \param[in] keyword Property name
+    ///
+    /// \param[in] flags Options to limit or relax request processing.
+    ///   Treated as a bitwise mask of \c TryGetFlags.
+    ///
+    /// \return Access structure for requested property array.
     template <typename T>
     FieldDataManager<T>
-    try_get(const std::string& keyword, const bool allow_unsupported = false)
+    try_get(const std::string& keyword, const unsigned int flags = 0u)
     {
+        const auto allow_unsupported =
+            (flags & TryGetFlags::AllowUnsupported) != 0u;
+
         if (!allow_unsupported && !FieldProps::template supported<T>(keyword)) {
             return { keyword, GetStatus::NOT_SUPPPORTED_KEYWORD, nullptr };
         }
 
         const auto has0 = this->template has<T>(keyword);
+        if (!has0 && ((flags & TryGetFlags::MustExist) != 0)) {
+            // Client requested a property which must exist, e.g., as a
+            // source array for a COPY operation, but the property has not
+            // (yet) been defined in the run's input.
+            return { keyword, GetStatus::MISSING_KEYWORD, nullptr };
+        }
 
         const auto& field_data = this->template
             init_get<T>(keyword, std::is_same_v<T, double> && allow_unsupported);
@@ -471,11 +582,15 @@ class FieldProps {
         }
 
         if (! has0) {
+            // Client requested a property which did not exist and which
+            // could not be created from a default description.
             this->template erase<T>(keyword);
 
             return { keyword, GetStatus::MISSING_KEYWORD, nullptr };
         }
 
+        // If we get here then the property exists but has not been fully
+        // defined yet.
         return { keyword, GetStatus::INVALID_DATA, nullptr };
     }
 

opm/input/eclipse/EclipseState/Grid/FieldPropsManager.cpp

@@ -85,7 +85,11 @@ const Fieldprops::FieldData<double>&
 FieldPropsManager::get_double_field_data(const std::string& keyword,
                                          bool allow_unsupported) const
 {
-    const auto& data = this->fp->try_get<double>(keyword, allow_unsupported);
+    const auto flags = allow_unsupported
+        ? FieldProps::TryGetFlags::AllowUnsupported
+        : 0u;
+
+    const auto& data = this->fp->try_get<double>(keyword, flags);
     if (allow_unsupported || data.valid())
         return data.field_data();
 

tests/parser/FieldPropsTests.cpp

@@ -207,9 +207,13 @@ COPY
 /
 )";
 
-    EclipseGrid grid(EclipseGrid(10,10,10));
-    Deck deck = Parser{}.parseString(deck_string);
-    BOOST_CHECK_THROW( FieldPropsManager(deck, Phases{true, true, true}, grid, TableManager()), std::out_of_range);
+    const auto deck = Parser{}.parseString(deck_string);
+
+    // Note: 'grid' must be mutable.
+    auto grid = EclipseGrid { 10, 10, 10 };
+
+    BOOST_CHECK_THROW(FieldPropsManager(deck, Phases{true, true, true}, grid, TableManager{}),
+                      OpmInputError);
 }
 
 BOOST_AUTO_TEST_CASE(INVALID_COPY_UNDEFINED_BOX)
@@ -288,6 +292,41 @@ COPY
                       OpmInputError);
 }
 
+BOOST_AUTO_TEST_CASE(INVALID_ADD)
+{
+    const auto deck = Parser{}.parseString(R"(RUNSPEC
+OIL
+GAS
+WATER
+TABDIMS
+/
+DIMENS
+ 10 10 10 /
+PROPS
+SGFN
+ 0.0 0.0 0.0
+ 0.8 1.0 0.0
+/
+SWFN
+ 0.2 0.0 0.0
+ 1.0 1.0 0.0
+/
+SOF3
+ 0.0 0.0 0.0
+ 0.8 1.0 1.0
+/
+ADD
+  SGU 0.123 /
+/
+)");
+
+    // Note: 'grid' must be mutable.
+    auto grid = EclipseGrid { 10, 10, 10 };
+
+    BOOST_CHECK_THROW(FieldPropsManager(deck, Phases{true, true, true}, grid, TableManager{deck}),
+                      OpmInputError);
+}
+
 BOOST_AUTO_TEST_CASE(GRID_RESET) {
     std::string deck_string = R"(
 REGIONS