#14423 Add cycle-safe delta ensemble dependency traversal

Delta ensembles were located by scanning the summary case main collection for objects referring to a given ensemble, and the dependent ensembles were visited by unguarded recursion. A dependency cycle, which is constructible through the UI, made that recursion run forever, and an ensemble used as both source 1 and source 2 was reported twice.

Add dependentDeltaEnsembles(), deltaEnsemblesInUpdateOrder() and wouldCreateDependencyCycle() to RimSummaryEnsembleTools. The traversal uses the PDM back references, deduplicates, is iterative with visited and on-path sets, and returns the delta ensembles in topological order so a delta ensemble is always visited before the delta ensembles using it as a source. Back edges are logged instead of traversed.

Reimplement updateDependentDeltaEnsembles on top of the new traversal and replace RimDeltaSummaryEnsemble::findReferringEnsembles() with dependentDeltaEnsembles() at its four call sites. Back references also find a delta ensemble that is detached from the project tree, which the previous ancestor scan did not.

ApplicationLibCode/ProjectDataModel/Summary/RimDeltaSummaryEnsemble.cpp

@@ -29,6 +29,7 @@
 #include "RimProject.h"
 #include "RimSummaryCaseMainCollection.h"
 #include "RimSummaryEnsemble.h"
+#include "RimSummaryEnsembleTools.h"
 
 #include "cafPdmUiButton.h"
 #include "cafPdmUiCheckBoxEditor.h"
@@ -207,7 +208,7 @@ void RimDeltaSummaryEnsemble::createDerivedEnsembleCases()
     }
 
     // If other derived ensembles are referring to this ensemble, update their cases as well
-    for ( auto referring : findReferringEnsembles() )
+    for ( auto referring : RimSummaryEnsembleTools::dependentDeltaEnsembles( this ) )
     {
         referring->createDerivedEnsembleCases();
     }
@@ -404,7 +405,7 @@ void RimDeltaSummaryEnsemble::fieldChangedByUi( const caf::PdmFieldHandle* chang
         updateReferringCurveSetsZoomAll();
 
         // If other derived ensembles are referring to this ensemble, update their cases as well
-        for ( auto referring : findReferringEnsembles() )
+        for ( auto referring : RimSummaryEnsembleTools::dependentDeltaEnsembles( this ) )
         {
             referring->updateReferringCurveSetsZoomAll();
         }
@@ -512,7 +513,7 @@ void RimDeltaSummaryEnsemble::updateDerivedEnsembleCases()
     }
 
     // If other derived ensembles are referring to this ensemble, update their cases as well
-    for ( auto referring : findReferringEnsembles() )
+    for ( auto referring : RimSummaryEnsembleTools::dependentDeltaEnsembles( this ) )
     {
         referring->updateDerivedEnsembleCases();
     }
@@ -552,31 +553,6 @@ RimSummaryCase* RimDeltaSummaryEnsemble::findCaseByRealizationNumber( const std:
     return nullptr;
 }
 
-//--------------------------------------------------------------------------------------------------
-///
-//--------------------------------------------------------------------------------------------------
-std::vector<RimDeltaSummaryEnsemble*> RimDeltaSummaryEnsemble::findReferringEnsembles() const
-{
-    std::vector<RimDeltaSummaryEnsemble*> referringEnsembles;
-
-    auto mainColl = firstAncestorOrThisOfType<RimSummaryCaseMainCollection>();
-    if ( mainColl )
-    {
-        for ( auto ensemble : mainColl->summaryEnsembles() )
-        {
-            auto derivedEnsemble = dynamic_cast<RimDeltaSummaryEnsemble*>( ensemble );
-            if ( derivedEnsemble )
-            {
-                if ( derivedEnsemble->m_ensemble1() == this || derivedEnsemble->m_ensemble2() == this )
-                {
-                    referringEnsembles.push_back( derivedEnsemble );
-                }
-            }
-        }
-    }
-    return referringEnsembles;
-}
-
 //--------------------------------------------------------------------------------------------------
 ///
 //--------------------------------------------------------------------------------------------------
@@ -614,7 +590,7 @@ void RimDeltaSummaryEnsemble::onSwapEnsemblesButtonClicked()
     updateConnectedEditors();
     updateReferringCurveSetsZoomAll();
 
-    for ( auto referring : findReferringEnsembles() )
+    for ( auto referring : RimSummaryEnsembleTools::dependentDeltaEnsembles( this ) )
     {
         referring->updateReferringCurveSetsZoomAll();
     }

ApplicationLibCode/ProjectDataModel/Summary/RimDeltaSummaryEnsemble.h

@@ -87,8 +87,6 @@ private:
     static RimSummaryCase* findCaseByParametersHash( const std::vector<RimSummaryCase*>& cases, size_t hash );
     static RimSummaryCase* findCaseByRealizationNumber( const std::vector<RimSummaryCase*>& cases, int realizationNumber );
 
-    std::vector<RimDeltaSummaryEnsemble*> findReferringEnsembles() const;
-
     std::vector<RimSummaryEnsemble*> allEnsembles() const;
 
 private:

ApplicationLibCode/ProjectDataModel/Summary/RimSummaryEnsembleTools.cpp

@@ -18,6 +18,8 @@
 
 #include "RimSummaryEnsembleTools.h"
 
+#include "RiaLogging.h"
+
 #include "Summary/RiaSummaryTools.h"
 
 #include "RifReaderRftInterface.h"
@@ -42,6 +44,8 @@
 
 #include "cafPdmUiTreeView.h"
 
+#include <algorithm>
+
 //--------------------------------------------------------------------------------------------------
 ///
 //--------------------------------------------------------------------------------------------------
@@ -528,6 +532,123 @@ RimSummaryCase* RimSummaryEnsembleTools::caseWithMostDataObjects( const std::vec
     return nullptr;
 }
 
+//--------------------------------------------------------------------------------------------------
+///
+//--------------------------------------------------------------------------------------------------
+std::vector<RimDeltaSummaryEnsemble*> RimSummaryEnsembleTools::dependentDeltaEnsembles( const RimSummaryEnsemble* sourceEnsemble )
+{
+    std::vector<RimDeltaSummaryEnsemble*> dependents;
+    if ( !sourceEnsemble ) return dependents;
+
+    // One entry is returned per referring field, so a delta ensemble using the same ensemble as both sources is
+    // reported twice. Report each delta ensemble once.
+    std::set<RimDeltaSummaryEnsemble*> seen;
+    for ( auto deltaEnsemble : sourceEnsemble->objectsWithReferringPtrFieldsOfType<RimDeltaSummaryEnsemble>() )
+    {
+        if ( !deltaEnsemble ) continue;
+        if ( seen.insert( deltaEnsemble ).second ) dependents.push_back( deltaEnsemble );
+    }
+
+    return dependents;
+}
+
+namespace
+{
+//--------------------------------------------------------------------------------------------------
+/// Depth first traversal of the dependency graph, following the edges from a source ensemble to the delta ensembles
+/// referring to it. The reverse postorder of that traversal is a topological order, so a delta ensemble is always
+/// visited before the delta ensembles using it as a source.
+//--------------------------------------------------------------------------------------------------
+std::vector<RimDeltaSummaryEnsemble*> dependentDeltaEnsemblesInOrder( const std::vector<const RimSummaryEnsemble*>& sourceEnsembles )
+{
+    struct StackEntry
+    {
+        RimDeltaSummaryEnsemble*              ensemble = nullptr;
+        std::vector<RimDeltaSummaryEnsemble*> dependents;
+        size_t                                nextDependent = 0;
+    };
+
+    std::vector<RimDeltaSummaryEnsemble*> postOrder;
+    std::set<const RimSummaryEnsemble*>   visited;
+    std::set<const RimSummaryEnsemble*>   onPath;
+    std::vector<StackEntry>               stack;
+
+    auto pushEnsemble = [&]( RimDeltaSummaryEnsemble* deltaEnsemble )
+    {
+        if ( visited.count( deltaEnsemble ) ) return;
+
+        if ( onPath.count( deltaEnsemble ) )
+        {
+            RiaLogging::error( QString( "Delta ensemble '%1' is part of a circular dependency. The cycle is not traversed." )
+                                   .arg( deltaEnsemble->name() )
+                                   .toStdString() );
+            return;
+        }
+
+        onPath.insert( deltaEnsemble );
+        stack.push_back( { deltaEnsemble, RimSummaryEnsembleTools::dependentDeltaEnsembles( deltaEnsemble ), 0 } );
+    };
+
+    for ( auto sourceEnsemble : sourceEnsembles )
+    {
+        for ( auto deltaEnsemble : RimSummaryEnsembleTools::dependentDeltaEnsembles( sourceEnsemble ) )
+        {
+            pushEnsemble( deltaEnsemble );
+
+            while ( !stack.empty() )
+            {
+                auto& top = stack.back();
+                if ( top.nextDependent < top.dependents.size() )
+                {
+                    auto* dependent = top.dependents[top.nextDependent++];
+                    pushEnsemble( dependent );
+                }
+                else
+                {
+                    auto* completed = top.ensemble;
+                    stack.pop_back();
+
+                    onPath.erase( completed );
+                    visited.insert( completed );
+                    postOrder.push_back( completed );
+                }
+            }
+        }
+    }
+
+    std::reverse( postOrder.begin(), postOrder.end() );
+
+    return postOrder;
+}
+} // namespace
+
+//--------------------------------------------------------------------------------------------------
+///
+//--------------------------------------------------------------------------------------------------
+std::vector<RimDeltaSummaryEnsemble*>
+    RimSummaryEnsembleTools::deltaEnsemblesInUpdateOrder( const std::vector<RimSummaryEnsemble*>& sourceEnsembles )
+{
+    return dependentDeltaEnsemblesInOrder( std::vector<const RimSummaryEnsemble*>( sourceEnsembles.begin(), sourceEnsembles.end() ) );
+}
+
+//--------------------------------------------------------------------------------------------------
+///
+//--------------------------------------------------------------------------------------------------
+bool RimSummaryEnsembleTools::wouldCreateDependencyCycle( const RimDeltaSummaryEnsemble* deltaEnsemble,
+                                                          const RimSummaryEnsemble*      candidateSource )
+{
+    if ( !deltaEnsemble || !candidateSource ) return false;
+    if ( deltaEnsemble == candidateSource ) return true;
+
+    // Everything depending on deltaEnsemble, directly or indirectly, would close a cycle if used as a source for it
+    for ( auto dependent : dependentDeltaEnsemblesInOrder( { deltaEnsemble } ) )
+    {
+        if ( dependent == candidateSource ) return true;
+    }
+
+    return false;
+}
+
 //--------------------------------------------------------------------------------------------------
 ///
 //--------------------------------------------------------------------------------------------------
@@ -535,10 +656,8 @@ void RimSummaryEnsembleTools::updateDependentDeltaEnsembles( const RimSummaryEns
 {
     if ( !sourceEnsemble ) return;
 
-    auto referringObjects = sourceEnsemble->objectsWithReferringPtrFieldsOfType<RimDeltaSummaryEnsemble>();
-    for ( auto referringEnsemble : referringObjects )
+    for ( auto deltaEnsemble : dependentDeltaEnsemblesInOrder( { sourceEnsemble } ) )
     {
-        referringEnsemble->onSourceEnsembleChanged();
-        updateDependentDeltaEnsembles( referringEnsemble );
+        deltaEnsemble->onSourceEnsembleChanged();
     }
 }

ApplicationLibCode/ProjectDataModel/Summary/RimSummaryEnsembleTools.h

@@ -27,6 +27,7 @@ class RimSummaryCase;
 class RimPlotCurve;
 class RigEnsembleParameter;
 class RimSummaryEnsemble;
+class RimDeltaSummaryEnsemble;
 
 namespace RimSummaryEnsembleTools
 {
@@ -47,6 +48,17 @@ void                         resetHighlightAllPlots();
 
 RimSummaryCase* caseWithMostDataObjects( const std::vector<RimSummaryCase*>& sourceCases );
 
+// The delta ensembles referring to sourceEnsemble, one entry per delta ensemble. An ensemble used as both source 1 and
+// source 2 is reported once.
+std::vector<RimDeltaSummaryEnsemble*> dependentDeltaEnsembles( const RimSummaryEnsemble* sourceEnsemble );
+
+// All delta ensembles depending directly or indirectly on any of sourceEnsembles, ordered so that a delta ensemble
+// always appears before the delta ensembles using it as a source. Cycles are reported and not traversed.
+std::vector<RimDeltaSummaryEnsemble*> deltaEnsemblesInUpdateOrder( const std::vector<RimSummaryEnsemble*>& sourceEnsembles );
+
+// True if using candidateSource as a source for deltaEnsemble would make the dependency graph cyclic.
+bool wouldCreateDependencyCycle( const RimDeltaSummaryEnsemble* deltaEnsemble, const RimSummaryEnsemble* candidateSource );
+
 void updateDependentDeltaEnsembles( const RimSummaryEnsemble* sourceEnsemble );
 
 } // namespace RimSummaryEnsembleTools

ApplicationLibCode/UnitTests/CMakeLists.txt

@@ -86,6 +86,7 @@ set(SOURCE_UNITTEST_FILES
     ${CMAKE_CURRENT_LIST_DIR}/RimDataFilterCollection-Test.cpp
     ${CMAKE_CURRENT_LIST_DIR}/RimSummaryCaseCollection-Test.cpp
     ${CMAKE_CURRENT_LIST_DIR}/RimSummaryCaseMainCollection-Test.cpp
+    ${CMAKE_CURRENT_LIST_DIR}/RimDeltaSummaryEnsemble-Test.cpp
     ${CMAKE_CURRENT_LIST_DIR}/RifActiveCellsReader-Test.cpp
     ${CMAKE_CURRENT_LIST_DIR}/RifCsvDataTableFormatter-Test.cpp
     ${CMAKE_CURRENT_LIST_DIR}/RiaSummaryAddressAnalyzer-Test.cpp

ApplicationLibCode/UnitTests/RimDeltaSummaryEnsemble-Test.cpp

@@ -0,0 +1,184 @@
+#include "gtest/gtest.h"
+
+#include "Summary/RiaSummaryTools.h"
+
+#include "RigCaseRealizationParameters.h"
+
+#include "RimDeltaSummaryEnsemble.h"
+#include "RimMockSummaryCase.h"
+#include "RimSummaryCaseMainCollection.h"
+#include "RimSummaryEnsemble.h"
+#include "RimSummaryEnsembleTools.h"
+
+#include "cafPdmPtrField.h"
+
+#include <algorithm>
+#include <memory>
+#include <vector>
+
+namespace
+{
+RimSummaryCase* createMockCase( int realizationNumber )
+{
+    auto* summaryCase = new RimMockSummaryCase();
+
+    auto parameters = std::make_shared<RigCaseRealizationParameters>();
+    parameters->setRealizationNumber( realizationNumber );
+    summaryCase->setCaseRealizationParameters( parameters );
+
+    return summaryCase;
+}
+
+size_t countOf( const std::vector<RimDeltaSummaryEnsemble*>& ensembles, const RimDeltaSummaryEnsemble* ensemble )
+{
+    return static_cast<size_t>( std::count( ensembles.begin(), ensembles.end(), ensemble ) );
+}
+
+//--------------------------------------------------------------------------------------------------
+/// Assign a source ensemble without going through setEnsemble1()/setEnsemble2(). The setters trigger
+/// an ensemble name update, and auto generated names never converge for a cyclic dependency.
+//--------------------------------------------------------------------------------------------------
+void forceSourceEnsemble( RimDeltaSummaryEnsemble* deltaEnsemble, const QString& fieldKeyword, RimSummaryEnsemble* sourceEnsemble )
+{
+    auto* field = dynamic_cast<caf::PdmPtrField<RimSummaryEnsemble*>*>( deltaEnsemble->findField( fieldKeyword ) );
+    ASSERT_TRUE( field != nullptr );
+
+    field->setValue( sourceEnsemble );
+}
+} // namespace
+
+//--------------------------------------------------------------------------------------------------
+/// The summary case main collection is a shared global object, so every test must leave it empty to
+/// keep the tests order independent.
+//--------------------------------------------------------------------------------------------------
+class RimDeltaSummaryEnsembleTest : public ::testing::Test
+{
+protected:
+    RimSummaryCaseMainCollection* mainCollection() const { return RiaSummaryTools::summaryCaseMainCollection(); }
+
+    RimSummaryEnsemble* createEnsemble( const QString& name, const std::vector<int>& realizationNumbers ) const
+    {
+        std::vector<RimSummaryCase*> cases;
+        for ( auto realizationNumber : realizationNumbers )
+        {
+            cases.push_back( createMockCase( realizationNumber ) );
+        }
+
+        return mainCollection()->addEnsemble( cases, name, true );
+    }
+
+    RimDeltaSummaryEnsemble* createDeltaEnsemble( RimSummaryEnsemble* ensemble1, RimSummaryEnsemble* ensemble2 ) const
+    {
+        auto* deltaEnsemble = new RimDeltaSummaryEnsemble();
+        mainCollection()->addEnsemble( deltaEnsemble );
+        deltaEnsemble->setEnsemble1( ensemble1 );
+        deltaEnsemble->setEnsemble2( ensemble2 );
+
+        return deltaEnsemble;
+    }
+
+    void TearDown() override
+    {
+        auto ensembles = mainCollection()->summaryEnsembles();
+
+        // Delete the delta ensembles first, they refer to the other ensembles
+        std::stable_partition( ensembles.begin(),
+                               ensembles.end(),
+                               []( RimSummaryEnsemble* ensemble ) { return dynamic_cast<RimDeltaSummaryEnsemble*>( ensemble ) != nullptr; } );
+
+        for ( auto* ensemble : ensembles )
+        {
+            mainCollection()->removeEnsemble( ensemble );
+            delete ensemble;
+        }
+
+        for ( auto* summaryCase : mainCollection()->topLevelSummaryCases() )
+        {
+            mainCollection()->removeCase( summaryCase, false );
+            delete summaryCase;
+        }
+    }
+};
+
+//--------------------------------------------------------------------------------------------------
+/// objectsWithReferringPtrFields() returns one entry per referring field, so an ensemble used as both
+/// sources of the same delta ensemble is reported twice by the raw PDM call.
+//--------------------------------------------------------------------------------------------------
+TEST_F( RimDeltaSummaryEnsembleTest, DependentDeltaEnsembles_Deduplicated )
+{
+    auto* ensemble      = createEnsemble( "Ensemble", { 0, 1 } );
+    auto* deltaEnsemble = createDeltaEnsemble( ensemble, ensemble );
+
+    EXPECT_EQ( size_t( 2 ), ensemble->objectsWithReferringPtrFieldsOfType<RimDeltaSummaryEnsemble>().size() );
+
+    auto dependents = RimSummaryEnsembleTools::dependentDeltaEnsembles( ensemble );
+    ASSERT_EQ( size_t( 1 ), dependents.size() );
+    EXPECT_EQ( deltaEnsemble, dependents.front() );
+}
+
+//--------------------------------------------------------------------------------------------------
+/// A delta ensemble must be regenerated before the delta ensembles using it as a source.
+//--------------------------------------------------------------------------------------------------
+TEST_F( RimDeltaSummaryEnsembleTest, DependencyOrder_ChainedDeltaEnsembles )
+{
+    auto* ensemble1 = createEnsemble( "Ensemble 1", { 0, 1 } );
+    auto* ensemble2 = createEnsemble( "Ensemble 2", { 0, 1 } );
+    auto* ensemble3 = createEnsemble( "Ensemble 3", { 0, 1 } );
+
+    auto* deltaA = createDeltaEnsemble( ensemble1, ensemble2 );
+    auto* deltaB = createDeltaEnsemble( deltaA, ensemble3 );
+
+    auto orderFromEnsemble1 = RimSummaryEnsembleTools::deltaEnsemblesInUpdateOrder( { ensemble1 } );
+    ASSERT_EQ( size_t( 2 ), orderFromEnsemble1.size() );
+    EXPECT_EQ( deltaA, orderFromEnsemble1[0] );
+    EXPECT_EQ( deltaB, orderFromEnsemble1[1] );
+
+    // Ensemble 3 is only a source of the second delta ensemble
+    auto orderFromEnsemble3 = RimSummaryEnsembleTools::deltaEnsemblesInUpdateOrder( { ensemble3 } );
+    ASSERT_EQ( size_t( 1 ), orderFromEnsemble3.size() );
+    EXPECT_EQ( deltaB, orderFromEnsemble3[0] );
+}
+
+//--------------------------------------------------------------------------------------------------
+/// A cycle is constructible through the UI. The traversal must terminate and report each delta
+/// ensemble once.
+//--------------------------------------------------------------------------------------------------
+TEST_F( RimDeltaSummaryEnsembleTest, DependencyOrder_CycleTerminates )
+{
+    auto* ensemble1 = createEnsemble( "Ensemble 1", { 0, 1 } );
+    auto* ensemble2 = createEnsemble( "Ensemble 2", { 0, 1 } );
+
+    auto* deltaA = createDeltaEnsemble( ensemble1, ensemble2 );
+    auto* deltaB = createDeltaEnsemble( deltaA, ensemble2 );
+
+    // Close the cycle, A now refers to B and B refers to A
+    forceSourceEnsemble( deltaA, "Ensemble1", deltaB );
+
+    auto order = RimSummaryEnsembleTools::deltaEnsemblesInUpdateOrder( { ensemble2 } );
+    ASSERT_EQ( size_t( 2 ), order.size() );
+    EXPECT_EQ( size_t( 1 ), countOf( order, deltaA ) );
+    EXPECT_EQ( size_t( 1 ), countOf( order, deltaB ) );
+
+    EXPECT_TRUE( RimSummaryEnsembleTools::wouldCreateDependencyCycle( deltaB, deltaA ) );
+
+    // Break the cycle before tear down
+    forceSourceEnsemble( deltaA, "Ensemble1", ensemble1 );
+}
+
+//--------------------------------------------------------------------------------------------------
+///
+//--------------------------------------------------------------------------------------------------
+TEST_F( RimDeltaSummaryEnsembleTest, WouldCreateDependencyCycle )
+{
+    auto* ensemble1 = createEnsemble( "Ensemble 1", { 0, 1 } );
+    auto* ensemble2 = createEnsemble( "Ensemble 2", { 0, 1 } );
+
+    auto* deltaA = createDeltaEnsemble( ensemble1, ensemble2 );
+    auto* deltaB = createDeltaEnsemble( deltaA, ensemble2 );
+
+    EXPECT_TRUE( RimSummaryEnsembleTools::wouldCreateDependencyCycle( deltaA, deltaA ) );
+    EXPECT_TRUE( RimSummaryEnsembleTools::wouldCreateDependencyCycle( deltaA, deltaB ) );
+
+    EXPECT_FALSE( RimSummaryEnsembleTools::wouldCreateDependencyCycle( deltaB, deltaA ) );
+    EXPECT_FALSE( RimSummaryEnsembleTools::wouldCreateDependencyCycle( deltaA, ensemble1 ) );
+}