From bcf1e6a084e472d6179703db694480003b418596 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A5rd=20Skaflestad?= Date: Fri, 14 Jun 2024 12:55:18 +0200 Subject: [PATCH] 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. --- .../eclipse/EclipseState/Grid/FieldProps.cpp | 63 +++++++-- .../eclipse/EclipseState/Grid/FieldProps.hpp | 127 +++++++++++++++++- .../EclipseState/Grid/FieldPropsManager.cpp | 6 +- tests/parser/FieldPropsTests.cpp | 45 ++++++- 4 files changed, 222 insertions(+), 19 deletions(-) diff --git a/opm/input/eclipse/EclipseState/Grid/FieldProps.cpp b/opm/input/eclipse/EclipseState/Grid/FieldProps.cpp index 7d3a883872c..69c1773e1bd 100644 --- a/opm/input/eclipse/EclipseState/Grid/FieldProps.cpp +++ b/opm/input/eclipse/EclipseState/Grid/FieldProps.cpp @@ -1526,6 +1526,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 or PORV in + // the EDIT section. 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()); @@ -1541,11 +1566,8 @@ void FieldProps::handle_operation(const Section section, FieldProps::supported(target_kw) || (tran_iter != this->tran.end())) { - const auto scalar_value = this-> - getSIValue(operation, target_kw, record.getItem(1).get(0)); - auto kw_info = Fieldprops::keywords::global_kw_info - (target_kw, tran_iter != this->tran.end()); + (target_kw, /* allow_unsupported = */ tran_iter != this->tran.end()); auto unique_name = target_kw; @@ -1570,9 +1592,23 @@ void FieldProps::handle_operation(const Section section, unique_name = tran_field_iter->second; } } + else if (mustExist && + !(editSect && (unique_name == ParserKeywords::PORV::keywordName)) && + (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(0)); auto& field_data = this->init_get - (unique_name, kw_info, editSect && kw_info.multiplier); + (unique_name, kw_info, /* multiplier_in_edit =*/ editSect && kw_info.multiplier); apply(operation, keyword.location(), target_kw, field_data.data, field_data.value_status, @@ -1589,6 +1625,15 @@ void FieldProps::handle_operation(const Section section, } if (FieldProps::supported(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(record.getItem(1).get(0)); auto& field_data = this->init_get(target_kw); @@ -1646,8 +1691,8 @@ void FieldProps::handle_COPY(const DeckKeyword& keyword, } if (FieldProps::supported(src_kw)) { - const auto& src_data = this->try_get(src_kw); - src_data.verify_status(); + const auto& src_data = this->try_get(src_kw, TryGetFlags::MustExist); + src_data.verify_status(keyword.location(), "Source array", "COPY"); auto& target_data = this->init_get(target_kw); target_data.checkInitialisedCopy(src_data.field_data(), index_list, @@ -1657,8 +1702,8 @@ void FieldProps::handle_COPY(const DeckKeyword& keyword, } if (FieldProps::supported(src_kw)) { - const auto& src_data = this->try_get(src_kw); - src_data.verify_status(); + const auto& src_data = this->try_get(src_kw, TryGetFlags::MustExist); + src_data.verify_status(keyword.location(), "Source array", "COPY"); auto& target_data = this->init_get(target_kw); target_data.checkInitialisedCopy(src_data.field_data(), index_list, diff --git a/opm/input/eclipse/EclipseState/Grid/FieldProps.hpp b/opm/input/eclipse/EclipseState/Grid/FieldProps.hpp index c22df240f67..93eb1652633 100644 --- a/opm/input/eclipse/EclipseState/Grid/FieldProps.hpp +++ b/opm/input/eclipse/EclipseState/Grid/FieldProps.hpp @@ -310,7 +310,7 @@ keyword_info 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: @@ -337,26 +337,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 struct FieldDataManager { + /// Property name. const std::string& keyword; + + /// Request status. GetStatus status; + + /// Property data. const Fieldprops::FieldData* 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* 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 @@ -388,6 +438,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) { @@ -405,6 +460,9 @@ class FieldProps { } } + /// Access underlying property data elements + /// + /// Returns \c nullptr if property data is not well formed. const std::vector* ptr() const { return (this->data_ptr != nullptr) @@ -412,24 +470,56 @@ class FieldProps { : nullptr; } + /// Access underlying property data elements + /// + /// Throws an exception as outlined in \c GetStatus if property data + /// is not well formed. const std::vector& 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& 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, const std::size_t ncomps); @@ -455,15 +545,36 @@ class FieldProps { template std::vector 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 FieldDataManager - 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(keyword)) { return { keyword, GetStatus::NOT_SUPPPORTED_KEYWORD, nullptr }; } const auto has0 = this->template has(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(keyword, std::is_same_v && allow_unsupported); @@ -475,11 +586,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(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 }; } diff --git a/opm/input/eclipse/EclipseState/Grid/FieldPropsManager.cpp b/opm/input/eclipse/EclipseState/Grid/FieldPropsManager.cpp index f9be5d2300a..2f1d4009eff 100644 --- a/opm/input/eclipse/EclipseState/Grid/FieldPropsManager.cpp +++ b/opm/input/eclipse/EclipseState/Grid/FieldPropsManager.cpp @@ -86,7 +86,11 @@ const Fieldprops::FieldData& FieldPropsManager::get_double_field_data(const std::string& keyword, bool allow_unsupported) const { - const auto& data = this->fp->try_get(keyword, allow_unsupported); + const auto flags = allow_unsupported + ? FieldProps::TryGetFlags::AllowUnsupported + : 0u; + + const auto& data = this->fp->try_get(keyword, flags); if (allow_unsupported || data.valid()) return data.field_data(); diff --git a/tests/parser/FieldPropsTests.cpp b/tests/parser/FieldPropsTests.cpp index d3e199cd3ca..c762c5ff949 100644 --- a/tests/parser/FieldPropsTests.cpp +++ b/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