Skip to content

Commit

Permalink
Hookup model logic for correct skipping of branches which shouldn't b…
Browse files Browse the repository at this point in the history 
…e run
  • Loading branch information
@nyalldawson
nyalldawson committed Apr 16, 2020
1 parent bb96802 commit 3b3c7d8
Show file tree
Hide file tree
Showing 4 changed files with 101 additions and 22 deletions.
6 changes: 5 additions & 1 deletion python/core/auto_generated/processing/models/qgsprocessingmodelalgorithm.sip.in
View file
Expand Up @@ -177,11 +177,15 @@ then the child will not be activated and ``False`` will be returned.
.. seealso:: :py:func:`deactivateChildAlgorithm`
%End

QSet< QString > dependentChildAlgorithms( const QString &childId ) const;
QSet< QString > dependentChildAlgorithms( const QString &childId, const QString &conditionalBranch = QString() ) const;
%Docstring
Returns a list of the child algorithm IDs depending on the child
algorithm with the specified ``childId``.

Optionally, a specific conditional branch which is created by the child algorithm
can be specified in order to restrict the returned list to algorithms which depend
on this specific branch.

.. seealso:: :py:func:`dependsOnChildAlgorithms`
%End

Expand Down
51 changes: 32 additions & 19 deletions src/core/processing/models/qgsprocessingmodelalgorithm.cpp
View file
Expand Up @@ -359,20 +359,30 @@ QVariantMap QgsProcessingModelAlgorithm::processAlgorithm( const QVariantMap &pa

executed.insert( childId );

std::function< void( const QString & )> pruneAlgorithmBranchRecursive;
pruneAlgorithmBranchRecursive = [&]( const QString & id )
std::function< void( const QString &, const QString & )> pruneAlgorithmBranchRecursive;
pruneAlgorithmBranchRecursive = [&]( const QString & id, const QString &branch = QString() )
{
const QSet<QString> toPrune = dependentChildAlgorithms( id );
const QSet<QString> toPrune = dependentChildAlgorithms( id, branch );
for ( const QString &targetId : toPrune )
{
if ( executed.contains( targetId ) )
continue;

executed.insert( targetId );
pruneAlgorithmBranchRecursive( targetId );
pruneAlgorithmBranchRecursive( targetId, branch );
}
};

// prune remaining algorithms if they are dependent on a branch from this child which didn't eventuate
const QgsProcessingOutputDefinitions outputDefs = childAlg->outputDefinitions();
for ( const QgsProcessingOutputDefinition *outputDef : outputDefs )
{
if ( outputDef->type() == QgsProcessingOutputConditionalBranch::typeName() && !results.value( outputDef->name() ).toBool() )
{
pruneAlgorithmBranchRecursive( childId, outputDef->name() );
}
}

if ( childAlg->flags() & QgsProcessingAlgorithm::FlagPruneModelBranchesBasedOnAlgorithmResults )
{
// check if any dependent algorithms should be canceled based on the outputs of this algorithm run
Expand Down Expand Up @@ -402,7 +412,7 @@ QVariantMap QgsProcessingModelAlgorithm::processAlgorithm( const QVariantMap &pa
// skip the dependent alg..
executed.insert( candidateId );
//... and everything which depends on it
pruneAlgorithmBranchRecursive( candidateId );
pruneAlgorithmBranchRecursive( candidateId, QString() );
break;
}
}
Expand Down Expand Up @@ -1606,7 +1616,7 @@ QMap<QString, QgsProcessingModelParameter> QgsProcessingModelAlgorithm::paramete
return mParameterComponents;
}

void QgsProcessingModelAlgorithm::dependentChildAlgorithmsRecursive( const QString &childId, QSet<QString> &depends ) const
void QgsProcessingModelAlgorithm::dependentChildAlgorithmsRecursive( const QString &childId, QSet<QString> &depends, const QString &branch ) const
{
QMap< QString, QgsProcessingModelChildAlgorithm >::const_iterator childIt = mChildAlgorithms.constBegin();
for ( ; childIt != mChildAlgorithms.constEnd(); ++childIt )
Expand All @@ -1619,7 +1629,7 @@ void QgsProcessingModelAlgorithm::dependentChildAlgorithmsRecursive( const QStri
bool hasDependency = false;
for ( const QgsProcessingModelChildDependency &dep : constDependencies )
{
if ( dep.childId == childId )
if ( dep.childId == childId && ( branch.isEmpty() || dep.conditionalBranch == branch ) )
{
hasDependency = true;
break;
Expand All @@ -1629,7 +1639,7 @@ void QgsProcessingModelAlgorithm::dependentChildAlgorithmsRecursive( const QStri
if ( hasDependency )
{
depends.insert( childIt->childId() );
dependentChildAlgorithmsRecursive( childIt->childId(), depends );
dependentChildAlgorithmsRecursive( childIt->childId(), depends, branch );
continue;
}

Expand All @@ -1645,23 +1655,23 @@ void QgsProcessingModelAlgorithm::dependentChildAlgorithmsRecursive( const QStri
&& source.outputChildId() == childId )
{
depends.insert( childIt->childId() );
dependentChildAlgorithmsRecursive( childIt->childId(), depends );
dependentChildAlgorithmsRecursive( childIt->childId(), depends, branch );
break;
}
}
}
}
}

QSet<QString> QgsProcessingModelAlgorithm::dependentChildAlgorithms( const QString &childId ) const
QSet<QString> QgsProcessingModelAlgorithm::dependentChildAlgorithms( const QString &childId, const QString &conditionalBranch ) const
{
QSet< QString > algs;

// temporarily insert the target child algorithm to avoid
// unnecessarily recursion though it
algs.insert( childId );

dependentChildAlgorithmsRecursive( childId, algs );
dependentChildAlgorithmsRecursive( childId, algs, conditionalBranch );

// remove temporary target alg
algs.remove( childId );
Expand Down Expand Up @@ -1734,16 +1744,19 @@ QList<QgsProcessingModelChildDependency> QgsProcessingModelAlgorithm::availableD
{
// check first if algorithm provides output branches
bool hasBranches = false;
const QgsProcessingOutputDefinitions defs = it->algorithm()->outputDefinitions();
for ( const QgsProcessingOutputDefinition *def : defs )
if ( it->algorithm() )
{
if ( def->type() == QgsProcessingOutputConditionalBranch::typeName() )
const QgsProcessingOutputDefinitions defs = it->algorithm()->outputDefinitions();
for ( const QgsProcessingOutputDefinition *def : defs )
{
hasBranches = true;
QgsProcessingModelChildDependency alg;
alg.childId = it->childId();
alg.conditionalBranch = def->name();
res << alg;
if ( def->type() == QgsProcessingOutputConditionalBranch::typeName() )
{
hasBranches = true;
QgsProcessingModelChildDependency alg;
alg.childId = it->childId();
alg.conditionalBranch = def->name();
res << alg;
}
}
}

Expand Down
9 changes: 7 additions & 2 deletions src/core/processing/models/qgsprocessingmodelalgorithm.h
View file
Expand Up @@ -159,9 +159,14 @@ class CORE_EXPORT QgsProcessingModelAlgorithm : public QgsProcessingAlgorithm
/**
* Returns a list of the child algorithm IDs depending on the child
* algorithm with the specified \a childId.
*
* Optionally, a specific conditional branch which is created by the child algorithm
* can be specified in order to restrict the returned list to algorithms which depend
* on this specific branch.
*
* \see dependsOnChildAlgorithms()
*/
QSet< QString > dependentChildAlgorithms( const QString &childId ) const;
QSet< QString > dependentChildAlgorithms( const QString &childId, const QString &conditionalBranch = QString() ) const;

/**
* Returns a list of the child algorithm IDs on which the child
Expand Down Expand Up @@ -532,7 +537,7 @@ class CORE_EXPORT QgsProcessingModelAlgorithm : public QgsProcessingAlgorithm
QStringList mParameterOrder;

void dependsOnChildAlgorithmsRecursive( const QString &childId, QSet<QString> &depends ) const;
void dependentChildAlgorithmsRecursive( const QString &childId, QSet<QString> &depends ) const;
void dependentChildAlgorithmsRecursive( const QString &childId, QSet<QString> &depends, const QString &branch ) const;

QVariantMap parametersForChildAlgorithm( const QgsProcessingModelChildAlgorithm &child, const QVariantMap &modelParameters, const QVariantMap &results, const QgsExpressionContext &expressionContext ) const;

Expand Down
57 changes: 57 additions & 0 deletions tests/src/analysis/testqgsprocessing.cpp
View file
Expand Up @@ -585,6 +585,7 @@ class TestQgsProcessing: public QObject
void modelerAlgorithm();
void modelExecution();
void modelBranchPruning();
void modelBranchPruningConditional();
void modelWithProviderWithLimitedTypes();
void modelVectorOutputIsCompatibleType();
void modelAcceptableValues();
Expand Down Expand Up @@ -9604,6 +9605,62 @@ void TestQgsProcessing::modelBranchPruning()
QVERIFY( !results.contains( QStringLiteral( "buffer3:BUFFER3_OUTPUT" ) ) );
}

void TestQgsProcessing::modelBranchPruningConditional()
{
QgsProcessingContext context;

context.expressionContext().appendScope( new QgsExpressionContextScope() );
context.expressionContext().scope( 0 )->setVariable( QStringLiteral( "var1" ), 1 );
context.expressionContext().scope( 0 )->setVariable( QStringLiteral( "var2" ), 0 );

// test that model branches are trimmed for algorithms which depend on conditional branches
QgsProcessingModelAlgorithm model1;

// first add the filter by layer type alg
QgsProcessingModelChildAlgorithm algc1;
algc1.setChildId( "branch" );
algc1.setAlgorithmId( "native:condition" );
QVariantMap config;
QVariantList conditions;
QVariantMap cond1;
cond1.insert( QStringLiteral( "name" ), QStringLiteral( "name1" ) );
cond1.insert( QStringLiteral( "expression" ), QStringLiteral( "@var1" ) );
conditions << cond1;
QVariantMap cond2;
cond2.insert( QStringLiteral( "name" ), QStringLiteral( "name2" ) );
cond2.insert( QStringLiteral( "expression" ), QStringLiteral( "@var2" ) );
conditions << cond2;
config.insert( QStringLiteral( "conditions" ), conditions );
algc1.setConfiguration( config );
model1.addChildAlgorithm( algc1 );

//then create some branches which come off this
QgsProcessingModelChildAlgorithm algc2;
algc2.setChildId( "exception" );
algc2.setAlgorithmId( "native:raiseexception" );
algc2.setDependencies( QList< QgsProcessingModelChildDependency >() << QgsProcessingModelChildDependency( QStringLiteral( "branch" ), QStringLiteral( "name1" ) ) );
model1.addChildAlgorithm( algc2 );

QgsProcessingModelChildAlgorithm algc3;
algc2.setChildId( "exception" );
algc3.setAlgorithmId( "native:raisewarning" );
algc3.setDependencies( QList< QgsProcessingModelChildDependency >() << QgsProcessingModelChildDependency( QStringLiteral( "branch" ), QStringLiteral( "name2" ) ) );
model1.addChildAlgorithm( algc3 );

QgsProcessingFeedback feedback;
QVariantMap params;
bool ok = false;
QVariantMap results = model1.run( params, context, &feedback, &ok );
QVERIFY( !ok ); // the branch with the exception should be hit

// flip the condition results
context.expressionContext().scope( 0 )->setVariable( QStringLiteral( "var1" ), 0 );
context.expressionContext().scope( 0 )->setVariable( QStringLiteral( "var2" ), 1 );

results = model1.run( params, context, &feedback, &ok );
QVERIFY( ok ); // the branch with the exception should NOT be hit
}

void TestQgsProcessing::modelWithProviderWithLimitedTypes()
{
QgsApplication::processingRegistry()->addProvider( new DummyProvider4() );
Expand Down

0 comments on commit 3b3c7d8

Please sign in to comment.