diff --git a/ApplicationLibCode/ProjectDataModel/Summary/RimDeltaSummaryCase.cpp b/ApplicationLibCode/ProjectDataModel/Summary/RimDeltaSummaryCase.cpp index df74724baf..22aaa86b0d 100644 --- a/ApplicationLibCode/ProjectDataModel/Summary/RimDeltaSummaryCase.cpp +++ b/ApplicationLibCode/ProjectDataModel/Summary/RimDeltaSummaryCase.cpp @@ -183,34 +183,10 @@ RimDeltaSummaryCase::RimDeltaSummaryCase() CAF_PDM_InitFieldNoDefault( &m_useFixedTimeStep, "UseFixedTimeStep", "Use Fixed Time Step" ); CAF_PDM_InitField( &m_fixedTimeStepIndex, "FixedTimeStepIndex", 0, "Time Step" ); - CAF_PDM_InitField( &m_inUse, "InUse", false, "In Use" ); m_fixedTimeStepIndex.uiCapability()->setUiEditorTypeName( caf::PdmUiTreeSelectionEditor::uiEditorTypeName() ); m_fixedTimeStepIndex.uiCapability()->setUiLabelPosition( caf::PdmUiItemInfo::LabelPosition::HIDDEN ); } -//-------------------------------------------------------------------------------------------------- -/// -//-------------------------------------------------------------------------------------------------- -void RimDeltaSummaryCase::setInUse( bool inUse ) -{ - m_inUse = inUse; - - if ( !m_inUse ) - { - m_summaryCase1 = nullptr; - m_summaryCase2 = nullptr; - m_dataCache.clear(); - } -} - -//-------------------------------------------------------------------------------------------------- -/// -//-------------------------------------------------------------------------------------------------- -bool RimDeltaSummaryCase::isInUse() const -{ - return m_inUse; -} - //-------------------------------------------------------------------------------------------------- /// //-------------------------------------------------------------------------------------------------- @@ -222,6 +198,18 @@ void RimDeltaSummaryCase::setSummaryCases( RimSummaryCase* sumCase1, RimSummaryC clearCache(); } +//-------------------------------------------------------------------------------------------------- +/// Sever the references to the source cases. Used when a derived case is detached from its ensemble, +/// so it does not keep the source cases alive in any way after the ensemble has moved on. +//-------------------------------------------------------------------------------------------------- +void RimDeltaSummaryCase::clearSourceCases() +{ + m_summaryCase1 = nullptr; + m_summaryCase2 = nullptr; + + clearCache(); +} + //-------------------------------------------------------------------------------------------------- /// //-------------------------------------------------------------------------------------------------- diff --git a/ApplicationLibCode/ProjectDataModel/Summary/RimDeltaSummaryCase.h b/ApplicationLibCode/ProjectDataModel/Summary/RimDeltaSummaryCase.h index 9e27fab0b5..cbc53c02bc 100644 --- a/ApplicationLibCode/ProjectDataModel/Summary/RimDeltaSummaryCase.h +++ b/ApplicationLibCode/ProjectDataModel/Summary/RimDeltaSummaryCase.h @@ -56,9 +56,8 @@ public: public: RimDeltaSummaryCase(); - void setInUse( bool inUse ); - bool isInUse() const; void setSummaryCases( RimSummaryCase* sumCase1, RimSummaryCase* sumCase2 ); + void clearSourceCases(); void setOperator( DerivedSummaryOperator oper ); void setFixedTimeSteps( int fixedTimeStepCase1, int fixedTimeStepCase2 ); @@ -111,8 +110,6 @@ private: caf::PdmField> m_useFixedTimeStep; caf::PdmField m_fixedTimeStepIndex; - caf::PdmField m_inUse; - // Local cache considered mutable mutable std::map, std::vector>> m_dataCache; }; diff --git a/ApplicationLibCode/ProjectDataModel/Summary/RimDeltaSummaryEnsemble.cpp b/ApplicationLibCode/ProjectDataModel/Summary/RimDeltaSummaryEnsemble.cpp index a01bd4cfc2..27e8557b2a 100644 --- a/ApplicationLibCode/ProjectDataModel/Summary/RimDeltaSummaryEnsemble.cpp +++ b/ApplicationLibCode/ProjectDataModel/Summary/RimDeltaSummaryEnsemble.cpp @@ -121,17 +121,6 @@ void RimDeltaSummaryEnsemble::setEnsemble2( RimSummaryEnsemble* ensemble ) RiaSummaryTools::updateSummaryEnsembleNames(); } -//-------------------------------------------------------------------------------------------------- -/// -//-------------------------------------------------------------------------------------------------- -std::vector RimDeltaSummaryEnsemble::allSummaryCases() const -{ - std::vector cases; - for ( auto sumCase : allDerivedCases( true ) ) - cases.push_back( sumCase ); - return cases; -} - //-------------------------------------------------------------------------------------------------- /// //-------------------------------------------------------------------------------------------------- @@ -164,48 +153,7 @@ void RimDeltaSummaryEnsemble::createDerivedEnsembleCases() { if ( !m_ensemble1 || !m_ensemble2 ) return; - setAllCasesNotInUse(); - - const auto cases1 = m_ensemble1->allSummaryCases(); - const auto cases2 = m_ensemble2->allSummaryCases(); - - for ( auto& sumCase1 : cases1 ) - { - auto crp = sumCase1->caseRealizationParameters(); - if ( !crp ) continue; - - RimSummaryCase* summaryCase2 = nullptr; - if ( m_matchOnParameters ) - { - summaryCase2 = findCaseByParametersHash( cases2, crp->parametersHash() ); - } - else - { - summaryCase2 = findCaseByRealizationNumber( cases2, crp->realizationNumber() ); - } - if ( !summaryCase2 ) continue; - - auto derivedCase = firstCaseNotInUse(); - derivedCase->setSummaryCases( sumCase1, summaryCase2 ); - derivedCase->setOperator( m_operator() ); - - int fixedTimeStepCase1 = -1; - int fixedTimeStepCase2 = -1; - if ( m_useFixedTimeStep == FixedTimeStepMode::FIXED_TIME_STEP_CASE_1 ) - { - fixedTimeStepCase1 = m_fixedTimeStepIndex; - } - else if ( m_useFixedTimeStep == FixedTimeStepMode::FIXED_TIME_STEP_CASE_2 ) - { - fixedTimeStepCase2 = m_fixedTimeStepIndex; - } - - derivedCase->setFixedTimeSteps( fixedTimeStepCase1, fixedTimeStepCase2 ); - derivedCase->createSummaryReaderInterface(); - derivedCase->setCaseRealizationParameters( crp ); - derivedCase->setInUse( true ); - derivedCase->updateDisplayNameFromCases(); - } + auto orphanedCases = rebuildDerivedCases(); // If other derived ensembles are referring to this ensemble, update their cases as well for ( auto referring : RimSummaryEnsembleTools::dependentDeltaEnsembles( this ) ) @@ -213,7 +161,111 @@ void RimDeltaSummaryEnsemble::createDerivedEnsembleCases() referring->createDerivedEnsembleCases(); } - deleteCasesNoInUse(); + for ( auto orphanedCase : orphanedCases ) + { + delete orphanedCase; + } +} + +//-------------------------------------------------------------------------------------------------- +/// The source case pairs this ensemble should have derived cases for, in the order of the cases in +/// the first source ensemble. Pure computation, the object graph is not modified. +//-------------------------------------------------------------------------------------------------- +std::vector> RimDeltaSummaryEnsemble::desiredSourceCasePairs() const +{ + std::vector> casePairs; + + if ( !m_ensemble1 || !m_ensemble2 ) return casePairs; + + const auto cases1 = m_ensemble1->allSummaryCases(); + const auto cases2 = m_ensemble2->allSummaryCases(); + + for ( auto sumCase1 : cases1 ) + { + auto crp = sumCase1->caseRealizationParameters(); + if ( !crp ) continue; + + RimSummaryCase* summaryCase2 = m_matchOnParameters ? findCaseByParametersHash( cases2, crp->parametersHash() ) + : findCaseByRealizationNumber( cases2, crp->realizationNumber() ); + if ( !summaryCase2 ) continue; + + casePairs.emplace_back( sumCase1, summaryCase2 ); + } + + return casePairs; +} + +//-------------------------------------------------------------------------------------------------- +/// Bring the derived cases in sync with desiredSourceCasePairs(). Existing derived cases are matched +/// on the source case pair, so a rebuild producing the same pairs reuses the same objects. +/// +/// Surplus cases are detached and returned, never deleted. Destroying them is the responsibility of +/// the caller, which may be several frames above a caller still iterating over these very objects. +//-------------------------------------------------------------------------------------------------- +std::vector RimDeltaSummaryEnsemble::rebuildDerivedCases() +{ + std::vector orphanedCases; + + std::map, RimDeltaSummaryCase*> reusableCases; + for ( auto derivedCase : allDerivedCases() ) + { + auto sourceCase1 = derivedCase->summaryCase1(); + auto sourceCase2 = derivedCase->summaryCase2(); + + // A case with unresolved sources, or a duplicate of a pair already seen, can not be reused + if ( sourceCase1 && sourceCase2 && reusableCases.try_emplace( { sourceCase1, sourceCase2 }, derivedCase ).second ) continue; + + orphanedCases.push_back( derivedCase ); + } + + int fixedTimeStepCase1 = -1; + int fixedTimeStepCase2 = -1; + if ( m_useFixedTimeStep == FixedTimeStepMode::FIXED_TIME_STEP_CASE_1 ) + { + fixedTimeStepCase1 = m_fixedTimeStepIndex; + } + else if ( m_useFixedTimeStep == FixedTimeStepMode::FIXED_TIME_STEP_CASE_2 ) + { + fixedTimeStepCase2 = m_fixedTimeStepIndex; + } + + for ( const auto& [sourceCase1, sourceCase2] : desiredSourceCasePairs() ) + { + RimDeltaSummaryCase* derivedCase = nullptr; + + auto it = reusableCases.find( { sourceCase1, sourceCase2 } ); + if ( it != reusableCases.end() ) + { + derivedCase = it->second; + reusableCases.erase( it ); + } + else + { + derivedCase = new RimDeltaSummaryCase(); + addCaseWithoutDependencyUpdate( derivedCase ); + derivedCase->setSummaryCases( sourceCase1, sourceCase2 ); + } + + derivedCase->setOperator( m_operator() ); + derivedCase->setFixedTimeSteps( fixedTimeStepCase1, fixedTimeStepCase2 ); + derivedCase->createSummaryReaderInterface(); + derivedCase->setCaseRealizationParameters( sourceCase1->caseRealizationParameters() ); + derivedCase->updateDisplayNameFromCases(); + } + + // The reusable cases not claimed by a desired pair are surplus + for ( const auto& [casePair, derivedCase] : reusableCases ) + { + orphanedCases.push_back( derivedCase ); + } + + for ( auto orphanedCase : orphanedCases ) + { + removeCase( orphanedCase, false ); + orphanedCase->clearSourceCases(); + } + + return orphanedCases; } //-------------------------------------------------------------------------------------------------- @@ -431,68 +483,17 @@ void RimDeltaSummaryEnsemble::defineEditorAttribute( const caf::PdmFieldHandle* //-------------------------------------------------------------------------------------------------- /// //-------------------------------------------------------------------------------------------------- -void RimDeltaSummaryEnsemble::setAllCasesNotInUse() +std::vector RimDeltaSummaryEnsemble::allDerivedCases() const { - for ( auto derCase : allDerivedCases( true ) ) - derCase->setInUse( false ); -} - -//-------------------------------------------------------------------------------------------------- -/// -//-------------------------------------------------------------------------------------------------- -void RimDeltaSummaryEnsemble::deleteCasesNoInUse() -{ - std::vector inactiveCases; - auto allCases = allDerivedCases( false ); - std::copy_if( allCases.begin(), - allCases.end(), - std::back_inserter( inactiveCases ), - []( RimDeltaSummaryCase* derCase ) { return !derCase->isInUse(); } ); - - for ( auto derCase : inactiveCases ) + std::vector derivedCases; + for ( auto sumCase : allSummaryCases() ) { - removeCase( derCase ); - delete derCase; - } -} - -//-------------------------------------------------------------------------------------------------- -/// -//-------------------------------------------------------------------------------------------------- -RimDeltaSummaryCase* RimDeltaSummaryEnsemble::firstCaseNotInUse() -{ - auto allCases = allDerivedCases( false ); - auto itr = std::find_if( allCases.begin(), allCases.end(), []( RimDeltaSummaryCase* derCase ) { return !derCase->isInUse(); } ); - if ( itr != allCases.end() ) - { - return *itr; - } - - // If no active case was found, add a new case to the collection - auto newCase = new RimDeltaSummaryCase(); - - // Show realization data source for the first case. If we create for all, the performance will be bad - newCase->setShowTreeNodes( m_cases.empty() ); - - m_cases.push_back( newCase ); - return newCase; -} - -//-------------------------------------------------------------------------------------------------- -/// -//-------------------------------------------------------------------------------------------------- -std::vector RimDeltaSummaryEnsemble::allDerivedCases( bool activeOnly ) const -{ - std::vector activeCases; - for ( auto sumCase : RimSummaryEnsemble::allSummaryCases() ) - { - auto derivedCase = dynamic_cast( sumCase ); - if ( derivedCase && ( !activeOnly || derivedCase->isInUse() ) ) + if ( auto derivedCase = dynamic_cast( sumCase ) ) { - activeCases.push_back( derivedCase ); + derivedCases.push_back( derivedCase ); } } - return activeCases; + return derivedCases; } //-------------------------------------------------------------------------------------------------- @@ -500,7 +501,7 @@ std::vector RimDeltaSummaryEnsemble::allDerivedCases( bool //-------------------------------------------------------------------------------------------------- void RimDeltaSummaryEnsemble::updateDerivedEnsembleCases() { - for ( auto& derivedCase : allDerivedCases( true ) ) + for ( auto& derivedCase : allDerivedCases() ) { derivedCase->createSummaryReaderInterface(); diff --git a/ApplicationLibCode/ProjectDataModel/Summary/RimDeltaSummaryEnsemble.h b/ApplicationLibCode/ProjectDataModel/Summary/RimDeltaSummaryEnsemble.h index 16546a14fe..f7fdf70f4d 100644 --- a/ApplicationLibCode/ProjectDataModel/Summary/RimDeltaSummaryEnsemble.h +++ b/ApplicationLibCode/ProjectDataModel/Summary/RimDeltaSummaryEnsemble.h @@ -53,7 +53,6 @@ public: void setEnsemble1( RimSummaryEnsemble* ensemble ); void setEnsemble2( RimSummaryEnsemble* ensemble ); - std::vector allSummaryCases() const override; std::set ensembleSummaryAddresses() const override; bool hasCaseReference( const RimSummaryCase* sumCase ) const; @@ -63,6 +62,13 @@ public: void onSourceEnsembleChanged(); void createDerivedEnsembleCases(); + std::vector> desiredSourceCasePairs() const; + + // Detaches and returns the derived cases no longer backed by a source case pair. The caller owns them. + [[nodiscard]] std::vector rebuildDerivedCases(); + + std::vector allDerivedCases() const; + bool discardMissingOrIncompleteRealizations() const; std::pair nameKeys() const override; @@ -76,11 +82,6 @@ private: void onSwapEnsemblesButtonClicked(); - void setAllCasesNotInUse(); - void deleteCasesNoInUse(); - RimDeltaSummaryCase* firstCaseNotInUse(); - std::vector allDerivedCases( bool activeOnly ) const; - void updateDerivedEnsembleCases(); bool isValid() const; diff --git a/ApplicationLibCode/ProjectDataModel/Summary/RimSummaryEnsemble.cpp b/ApplicationLibCode/ProjectDataModel/Summary/RimSummaryEnsemble.cpp index b9c7ed443a..ce59d075ad 100644 --- a/ApplicationLibCode/ProjectDataModel/Summary/RimSummaryEnsemble.cpp +++ b/ApplicationLibCode/ProjectDataModel/Summary/RimSummaryEnsemble.cpp @@ -155,13 +155,7 @@ void RimSummaryEnsemble::removeCase( RimSummaryCase* summaryCase, bool notifyCha //-------------------------------------------------------------------------------------------------- void RimSummaryEnsemble::addCase( RimSummaryCase* summaryCase, bool notifyChange ) { - summaryCase->nameChanged.connect( this, &RimSummaryEnsemble::onCaseNameChanged ); - - summaryCase->setShowTreeNodes( m_cases.empty() ); - - m_cases.push_back( summaryCase ); - m_cachedSortedEnsembleParameters.clear(); - m_analyzer.reset(); + addCaseWithoutDependencyUpdate( summaryCase ); // Update derived ensemble cases (if any) std::vector referringObjects = objectsWithReferringPtrFieldsOfType(); @@ -180,6 +174,21 @@ void RimSummaryEnsemble::addCase( RimSummaryCase* summaryCase, bool notifyChange } if ( notifyChange ) updateReferringCurveSetsZoomAll(); +} + +//-------------------------------------------------------------------------------------------------- +/// +//-------------------------------------------------------------------------------------------------- +void RimSummaryEnsemble::addCaseWithoutDependencyUpdate( RimSummaryCase* summaryCase ) +{ + summaryCase->nameChanged.connect( this, &RimSummaryEnsemble::onCaseNameChanged ); + + // Show realization data source for the first case. If we create for all, the performance will be bad + summaryCase->setShowTreeNodes( m_cases.empty() ); + + m_cases.push_back( summaryCase ); + m_cachedSortedEnsembleParameters.clear(); + m_analyzer.reset(); clearChildNodes(); } diff --git a/ApplicationLibCode/ProjectDataModel/Summary/RimSummaryEnsemble.h b/ApplicationLibCode/ProjectDataModel/Summary/RimSummaryEnsemble.h index ce17ef3ad0..f610ea0b48 100644 --- a/ApplicationLibCode/ProjectDataModel/Summary/RimSummaryEnsemble.h +++ b/ApplicationLibCode/ProjectDataModel/Summary/RimSummaryEnsemble.h @@ -128,6 +128,10 @@ public: protected: virtual void onLoadDataAndUpdate(); + // Add a case without notifying the delta ensembles depending on this ensemble. Used when the caller is itself the + // owner of the dependency update, as is the case when a delta ensemble rebuilds its derived cases. + void addCaseWithoutDependencyUpdate( RimSummaryCase* summaryCase ); + void defineUiOrdering( QString uiConfigName, caf::PdmUiOrdering& uiOrdering ) override; void buildMetaData(); void appendMenuItems( caf::CmdFeatureMenuBuilder& menuBuilder ) const override; diff --git a/ApplicationLibCode/UnitTests/RimDeltaSummaryEnsemble-Test.cpp b/ApplicationLibCode/UnitTests/RimDeltaSummaryEnsemble-Test.cpp index 7a8386f32b..b6e55c8401 100644 --- a/ApplicationLibCode/UnitTests/RimDeltaSummaryEnsemble-Test.cpp +++ b/ApplicationLibCode/UnitTests/RimDeltaSummaryEnsemble-Test.cpp @@ -165,6 +165,80 @@ TEST_F( RimDeltaSummaryEnsembleTest, DependencyOrder_CycleTerminates ) forceSourceEnsemble( deltaA, "Ensemble1", ensemble1 ); } +//-------------------------------------------------------------------------------------------------- +/// Derived cases are matched on the source case pair, so rebuilding without changing the sources must +/// reuse the very same objects. +//-------------------------------------------------------------------------------------------------- +TEST_F( RimDeltaSummaryEnsembleTest, Rebuild_IsIdempotent ) +{ + auto* ensemble1 = createEnsemble( "Ensemble 1", { 0, 1 } ); + auto* ensemble2 = createEnsemble( "Ensemble 2", { 0, 1 } ); + + auto* deltaEnsemble = createDeltaEnsemble( ensemble1, ensemble2 ); + deltaEnsemble->createDerivedEnsembleCases(); + + auto derivedCases = deltaEnsemble->allDerivedCases(); + ASSERT_EQ( size_t( 2 ), derivedCases.size() ); + + auto orphanedCases = deltaEnsemble->rebuildDerivedCases(); + EXPECT_TRUE( orphanedCases.empty() ); + + EXPECT_EQ( derivedCases, deltaEnsemble->allDerivedCases() ); +} + +//-------------------------------------------------------------------------------------------------- +/// +//-------------------------------------------------------------------------------------------------- +TEST_F( RimDeltaSummaryEnsembleTest, Rebuild_CreatesMissingAndOrphansSurplus ) +{ + auto* ensemble1 = createEnsemble( "Ensemble 1", { 0, 1 } ); + auto* ensemble2 = createEnsemble( "Ensemble 2", { 0, 1 } ); + + auto* deltaEnsemble = createDeltaEnsemble( ensemble1, ensemble2 ); + deltaEnsemble->createDerivedEnsembleCases(); + ASSERT_EQ( size_t( 2 ), deltaEnsemble->allDerivedCases().size() ); + + // A matching realization in both source ensembles gives one more derived case + auto* addedCase1 = createMockCase( 2 ); + auto* addedCase2 = createMockCase( 2 ); + ensemble1->addCase( addedCase1, false ); + ensemble2->addCase( addedCase2, false ); + + auto orphanedCases = deltaEnsemble->rebuildDerivedCases(); + EXPECT_TRUE( orphanedCases.empty() ); + EXPECT_EQ( size_t( 3 ), deltaEnsemble->allDerivedCases().size() ); + + // Removing it again makes the derived case surplus + ensemble1->removeCase( addedCase1, false ); + delete addedCase1; + + orphanedCases = deltaEnsemble->rebuildDerivedCases(); + ASSERT_EQ( size_t( 1 ), orphanedCases.size() ); + EXPECT_EQ( size_t( 2 ), deltaEnsemble->allDerivedCases().size() ); + + for ( auto* orphanedCase : orphanedCases ) + { + delete orphanedCase; + } +} + +//-------------------------------------------------------------------------------------------------- +/// +//-------------------------------------------------------------------------------------------------- +TEST_F( RimDeltaSummaryEnsembleTest, Rebuild_NoMatchingRealizations ) +{ + auto* ensemble1 = createEnsemble( "Ensemble 1", { 0, 1 } ); + auto* ensemble2 = createEnsemble( "Ensemble 2", { 5, 6 } ); + + auto* deltaEnsemble = createDeltaEnsemble( ensemble1, ensemble2 ); + + EXPECT_TRUE( deltaEnsemble->desiredSourceCasePairs().empty() ); + + auto orphanedCases = deltaEnsemble->rebuildDerivedCases(); + EXPECT_TRUE( orphanedCases.empty() ); + EXPECT_TRUE( deltaEnsemble->allDerivedCases().empty() ); +} + //-------------------------------------------------------------------------------------------------- /// //--------------------------------------------------------------------------------------------------