From 9ada4eed09178bdb59061dfa976deed8e47508bd Mon Sep 17 00:00:00 2001 From: Kristian Bendiksen Date: Wed, 27 May 2026 14:09:49 +0200 Subject: [PATCH] #14065 Well Event: Emit keyword items in canonical Eclipse order `add_well_keyword_event` previously emitted items in `keyword_data` dict insertion order. Eclipse keywords such as WCONHIST are positional, so a non-canonical order silently corrupted the simulator input. `RifEventKeywordFormatter::formatKeyword` now walks the OPM `ParserRecord` schema and emits items in canonical order, inserting `1*` defaults for intermediate gaps and dropping trailing unspecified items. Items that are not part of the keyword schema (e.g. mnemonic-list keywords like RPTRST/RPTSCHED) are appended after the canonical block in caller-supplied order to preserve existing behaviour. Fixes #14065. --- .../RifEventKeywordFormatter.cpp | 67 +++++++++++++++++-- .../Python/rips/tests/test_well_events.py | 47 +++++++++++++ 2 files changed, 109 insertions(+), 5 deletions(-) diff --git a/ApplicationLibCode/FileInterface/RifEventKeywordFormatter.cpp b/ApplicationLibCode/FileInterface/RifEventKeywordFormatter.cpp index a8e3ebb1f5..6e0d824527 100644 --- a/ApplicationLibCode/FileInterface/RifEventKeywordFormatter.cpp +++ b/ApplicationLibCode/FileInterface/RifEventKeywordFormatter.cpp @@ -35,8 +35,12 @@ #include "opm/input/eclipse/Parser/Parser.hpp" #include "opm/input/eclipse/Parser/ParserKeyword.hpp" #include "opm/input/eclipse/Parser/ParserKeywords/W.hpp" +#include "opm/input/eclipse/Parser/ParserRecord.hpp" +#include #include +#include +#include //-------------------------------------------------------------------------------------------------- /// @@ -52,22 +56,75 @@ QString RifEventKeywordFormatter::formatKeyword( const QString& keywordName, con const Opm::ParserKeyword& parserKw = parser.getKeyword( kwName ); Opm::DeckKeyword kw( parserKw ); - // Build deck items from stored data - std::vector deckItems; + // Eclipse keyword records are positional: items must be emitted in the schema-defined order + // regardless of how the caller supplied them (e.g. Python dict insertion order). + std::unordered_map userItemsByName; + userItemsByName.reserve( items.size() ); for ( const auto* item : items ) + { + userItemsByName.emplace( item->itemName().toStdString(), item ); + } + + const Opm::ParserRecord& parserRecord = parserKw.getRecord( 0 ); + + // Determine the highest canonical index that the user actually provided; trailing + // unspecified items are dropped, intermediate gaps become default markers ("1*"). + std::optional lastProvidedIdx; + std::unordered_set canonicalNames; + canonicalNames.reserve( parserRecord.size() ); + for ( size_t i = 0; i < parserRecord.size(); ++i ) + { + const std::string& name = parserRecord.get( i ).name(); + canonicalNames.insert( name ); + if ( userItemsByName.contains( name ) ) + { + lastProvidedIdx = i; + } + } + + auto appendDeckItem = [&]( std::vector& out, const std::string& name, const RimWellEventKeywordItem* item ) { switch ( item->itemType() ) { case RimWellEventKeywordItem::ItemType::STRING: - deckItems.push_back( RifOpmDeckTools::item( item->itemName().toStdString(), item->stringValue().toStdString() ) ); + out.push_back( RifOpmDeckTools::item( name, item->stringValue().toStdString() ) ); break; case RimWellEventKeywordItem::ItemType::INTEGER: - deckItems.push_back( RifOpmDeckTools::item( item->itemName().toStdString(), item->intValue() ) ); + out.push_back( RifOpmDeckTools::item( name, item->intValue() ) ); break; case RimWellEventKeywordItem::ItemType::DOUBLE: - deckItems.push_back( RifOpmDeckTools::item( item->itemName().toStdString(), item->doubleValue() ) ); + out.push_back( RifOpmDeckTools::item( name, item->doubleValue() ) ); break; } + }; + + std::vector deckItems; + if ( lastProvidedIdx.has_value() ) + { + deckItems.reserve( *lastProvidedIdx + 1 ); + for ( size_t i = 0; i <= *lastProvidedIdx; ++i ) + { + const std::string& name = parserRecord.get( i ).name(); + auto it = userItemsByName.find( name ); + if ( it == userItemsByName.end() ) + { + deckItems.push_back( RifOpmDeckTools::defaultItem( name ) ); + } + else + { + appendDeckItem( deckItems, name, it->second ); + } + } + } + + // Items not in the keyword's schema are still emitted (preserves behaviour for + // mnemonic-list keywords such as RPTRST/RPTSCHED) but in caller-supplied order + // after the canonical block. + for ( const auto* item : items ) + { + const std::string name = item->itemName().toStdString(); + if ( canonicalNames.contains( name ) ) continue; + appendDeckItem( deckItems, name, item ); } if ( !deckItems.empty() ) diff --git a/GrpcInterface/Python/rips/tests/test_well_events.py b/GrpcInterface/Python/rips/tests/test_well_events.py index b2495d4a20..f9cd05cd52 100644 --- a/GrpcInterface/Python/rips/tests/test_well_events.py +++ b/GrpcInterface/Python/rips/tests/test_well_events.py @@ -1428,6 +1428,53 @@ class TestKeywordEvents: assert "WRFTPLT" in schedule_text, "Schedule should contain WRFTPLT keyword" assert "DATES" in schedule_text, "Schedule should contain DATES keyword" + def test_wconhist_item_order_canonical(self, project_with_case_and_well): + """Regression for #14065: WCONHIST items must appear in the Eclipse-defined + canonical order regardless of how keyword_data was constructed in Python. + + Canonical WCONHIST order: WELL, STATUS, CMODE, ORAT, WRAT, GRAT, VFP_TABLE, + ALQ, THP, BHP, WGASRAT_HIS, NGLRAT_HIS. + """ + project, case, timeline = project_with_case_and_well + well_path = project.well_paths()[0] + + # Intentionally non-canonical insertion order: BHP placed before ORAT/WRAT/GRAT. + timeline.add_well_keyword_event( + event_date="2024-01-15", + well_path=well_path, + keyword_name="WCONHIST", + keyword_data={ + "WELL": well_path.name, + "STATUS": "OPEN", + "CMODE": "RESV", + "BHP": 250.0, + "ORAT": 3999.99, + "WRAT": 0.01, + "GRAT": 550678.44, + "VFP_TABLE": 1, + }, + ) + + schedule_text = timeline.generate_schedule_text(eclipse_case=case) + print(f"\nSchedule text for canonical-order check:\n{schedule_text}") + + assert "WCONHIST" in schedule_text + # Extract the WCONHIST record body (between the keyword and its terminating '/'). + wconhist_block = schedule_text.split("WCONHIST", 1)[1].split("/", 1)[0] + + orat_pos = wconhist_block.find("3999.99") + grat_pos = wconhist_block.find("550678") + bhp_pos = wconhist_block.find("250") + assert orat_pos >= 0, "ORAT value missing from WCONHIST output" + assert grat_pos >= 0, "GRAT value missing from WCONHIST output" + assert bhp_pos >= 0, "BHP value missing from WCONHIST output" + assert orat_pos < bhp_pos, ( + f"ORAT must precede BHP in canonical WCONHIST order; got block: {wconhist_block!r}" + ) + assert grat_pos < bhp_pos, ( + f"GRAT must precede BHP in canonical WCONHIST order; got block: {wconhist_block!r}" + ) + def test_invalid_keyword_data_unsupported_type(self, project_with_case_and_well): """Test error handling for unsupported data types in keyword events.""" project, case, timeline = project_with_case_and_well