Improve support for aggregating functions

When aggregating functions (min, max, sum, avg) are applied to a single vector variable, they compute a scalar result over the whole vector. These must be pre-computed before the element-wise for loop, otherwise the variable gets replaced with its indexed version (e.g. min(b) -> min(b[i])) which changes the semantics: min(b[i]) returns b[i], not min of all b.

Add check for pattern: c := agg_func(if(condition, true_expr, false_expr)).  Computes a scalar aggregate of the element-wise if-result and assigns it as a constant to all elements of c.
This commit is contained in:
copilot-swe-agent[bot] authored and Magne Sjaastad committed 2026-02-23 19:19:09 +01:00
1 parent 812b605a31
commit 8bbd0d2551
2 files changed
+485 -16

No files matched your search

@@ -160,3 +160,325 @@ TEST( RicExpressionParserTest, ExpandIfStatementsAndEvaluate )
EXPECT_DOUBLE_EQ( c[0], 100.0 );
EXPECT_DOUBLE_EQ( c[9], 19.0 );
}
//--------------------------------------------------------------------------------------------------
///
//--------------------------------------------------------------------------------------------------
TEST( RicExpressionParserTest, ExpandIfWithMinAggregation )
{
// Test that min(b) in an if-condition is treated as the global minimum of vector b,
// not as a per-element operation. This was a bug where min(b) was incorrectly expanded
// to min(b[i]) which just returns b[i] (the identity for a single scalar argument).
// a = [5, 6, 25], b = [4, 7, 6]
// min(b) = 4
// Expected: c[i] = (a[i] > 4) ? a[i] : b[i]
// c[0] = (5 > 4) ? 5 : 4 = 5
// c[1] = (6 > 4) ? 6 : 7 = 6 <- key: should compare 6 with min(b)=4, not b[1]=7
// c[2] = (25 > 4) ? 25 : 6 = 25
std::vector<double> a = { 5.0, 6.0, 25.0 };
std::vector<double> b = { 4.0, 7.0, 6.0 };
std::vector<double> c( 3 );
ExpressionParser parser;
parser.assignVector( "a", a );
parser.assignVector( "b", b );
parser.assignVector( "c", c );
QString expr = "c := if(a > min(b), a, b)";
EXPECT_TRUE( parser.expandIfStatementsAndEvaluate( expr ) );
EXPECT_DOUBLE_EQ( c[0], 5.0 );
EXPECT_DOUBLE_EQ( c[1], 6.0 ); // Would be 7.0 with the bug (comparing a[1]=6 with b[1]=7)
EXPECT_DOUBLE_EQ( c[2], 25.0 );
}
//--------------------------------------------------------------------------------------------------
///
// The expression "c := min(if(a > 13, a, b))" computes if(a > 13, a, b) element-wise and assigns
// the minimum of that result as a constant to all elements of c.
//
// With a = [10..19] and b = [100..109]:
// if(a > 13, a, b) => [100, 101, 102, 103, 14, 15, 16, 17, 18, 19]
// min(...) => 14
//
// expandIfStatements detects the agg_func(if(...)) pattern and generates a two-pass expansion:
// a reducing loop that finds the aggregate, followed by an assignment loop over all c elements.
//--------------------------------------------------------------------------------------------------
TEST( RicExpressionParserTest, MinOfIfStatementProducesConstant )
{
std::vector<double> a( 10 );
std::iota( a.begin(), a.end(), 10 );
std::vector<double> b( 10 );
std::iota( b.begin(), b.end(), 100 );
std::vector<double> c( 10 );
ExpressionParser parser;
parser.assignVector( "a", a );
parser.assignVector( "b", b );
parser.assignVector( "c", c );
QString expr = "c := min(if(a > 13, a, b))";
EXPECT_TRUE( parser.expandIfStatementsAndEvaluate( expr ) );
// All elements of c should be the constant minimum value 14
for ( size_t i = 0; i < c.size(); i++ )
{
EXPECT_DOUBLE_EQ( c[i], 14.0 );
}
}
//--------------------------------------------------------------------------------------------------
///
//--------------------------------------------------------------------------------------------------
TEST( RicExpressionParserTest, MaxOfIfStatementProducesConstant )
{
// a = [10..19], b = [100..109]
// if(a > 13, a, b) => [100, 101, 102, 103, 14, 15, 16, 17, 18, 19]
// max(...) => 103
std::vector<double> a( 10 );
std::iota( a.begin(), a.end(), 10 );
std::vector<double> b( 10 );
std::iota( b.begin(), b.end(), 100 );
std::vector<double> c( 10 );
ExpressionParser parser;
parser.assignVector( "a", a );
parser.assignVector( "b", b );
parser.assignVector( "c", c );
EXPECT_TRUE( parser.expandIfStatementsAndEvaluate( "c := max(if(a > 13, a, b))" ) );
for ( size_t i = 0; i < c.size(); i++ )
{
EXPECT_DOUBLE_EQ( c[i], 103.0 );
}
}
//--------------------------------------------------------------------------------------------------
///
//--------------------------------------------------------------------------------------------------
TEST( RicExpressionParserTest, SumOfIfStatementProducesConstant )
{
// a = [10..19], b = [100..109]
// if(a > 13, a, b) => [100, 101, 102, 103, 14, 15, 16, 17, 18, 19]
// sum(...) => 505
std::vector<double> a( 10 );
std::iota( a.begin(), a.end(), 10 );
std::vector<double> b( 10 );
std::iota( b.begin(), b.end(), 100 );
std::vector<double> c( 10 );
ExpressionParser parser;
parser.assignVector( "a", a );
parser.assignVector( "b", b );
parser.assignVector( "c", c );
EXPECT_TRUE( parser.expandIfStatementsAndEvaluate( "c := sum(if(a > 13, a, b))" ) );
for ( size_t i = 0; i < c.size(); i++ )
{
EXPECT_DOUBLE_EQ( c[i], 505.0 );
}
}
//--------------------------------------------------------------------------------------------------
///
//--------------------------------------------------------------------------------------------------
TEST( RicExpressionParserTest, AvgOfIfStatementProducesConstant )
{
// a = [10..19], b = [100..109]
// if(a > 13, a, b) => [100, 101, 102, 103, 14, 15, 16, 17, 18, 19]
// avg(...) => 505 / 10 = 50.5
std::vector<double> a( 10 );
std::iota( a.begin(), a.end(), 10 );
std::vector<double> b( 10 );
std::iota( b.begin(), b.end(), 100 );
std::vector<double> c( 10 );
ExpressionParser parser;
parser.assignVector( "a", a );
parser.assignVector( "b", b );
parser.assignVector( "c", c );
EXPECT_TRUE( parser.expandIfStatementsAndEvaluate( "c := avg(if(a > 13, a, b))" ) );
for ( size_t i = 0; i < c.size(); i++ )
{
EXPECT_DOUBLE_EQ( c[i], 50.5 );
}
}
//--------------------------------------------------------------------------------------------------
///
//--------------------------------------------------------------------------------------------------
TEST( RicExpressionParserTest, MinOfIfStatementAllConditionsTrue )
{
// When all conditions are true the if always picks a.
// a = [10..19], b = [100..109], condition a > 5 is always true
// if(a > 5, a, b) => [10, 11, 12, 13, 14, 15, 16, 17, 18, 19]
// min(...) => 10
std::vector<double> a( 10 );
std::iota( a.begin(), a.end(), 10 );
std::vector<double> b( 10 );
std::iota( b.begin(), b.end(), 100 );
std::vector<double> c( 10 );
ExpressionParser parser;
parser.assignVector( "a", a );
parser.assignVector( "b", b );
parser.assignVector( "c", c );
EXPECT_TRUE( parser.expandIfStatementsAndEvaluate( "c := min(if(a > 5, a, b))" ) );
for ( size_t i = 0; i < c.size(); i++ )
{
EXPECT_DOUBLE_EQ( c[i], 10.0 );
}
}
//--------------------------------------------------------------------------------------------------
///
//--------------------------------------------------------------------------------------------------
TEST( RicExpressionParserTest, MinOfIfStatementAllConditionsFalse )
{
// When all conditions are false the if always picks b.
// a = [10..19], b = [100..109], condition a > 100 is always false
// if(a > 100, a, b) => [100, 101, 102, 103, 104, 105, 106, 107, 108, 109]
// min(...) => 100
std::vector<double> a( 10 );
std::iota( a.begin(), a.end(), 10 );
std::vector<double> b( 10 );
std::iota( b.begin(), b.end(), 100 );
std::vector<double> c( 10 );
ExpressionParser parser;
parser.assignVector( "a", a );
parser.assignVector( "b", b );
parser.assignVector( "c", c );
EXPECT_TRUE( parser.expandIfStatementsAndEvaluate( "c := min(if(a > 100, a, b))" ) );
for ( size_t i = 0; i < c.size(); i++ )
{
EXPECT_DOUBLE_EQ( c[i], 100.0 );
}
}
//--------------------------------------------------------------------------------------------------
///
// Simple vector aggregation: c := min(a), max(a), sum(a), avg(a) without any if-statement.
// These expressions contain no "if" so they are passed directly to the exprtk evaluator.
//
// a = [10..19]
// min(a) => 10
// max(a) => 19
// sum(a) => 145
// avg(a) => 14.5
//--------------------------------------------------------------------------------------------------
TEST( RicExpressionParserTest, SimpleVectorMin )
{
std::vector<double> a( 10 );
std::iota( a.begin(), a.end(), 10 );
std::vector<double> c( 10 );
ExpressionParser parser;
parser.assignVector( "a", a );
parser.assignVector( "c", c );
EXPECT_TRUE( parser.evaluate( "c := min(a)" ) );
for ( size_t i = 0; i < c.size(); i++ )
{
EXPECT_DOUBLE_EQ( c[i], 10.0 );
}
}
//--------------------------------------------------------------------------------------------------
///
//--------------------------------------------------------------------------------------------------
TEST( RicExpressionParserTest, SimpleVectorMax )
{
std::vector<double> a( 10 );
std::iota( a.begin(), a.end(), 10 );
std::vector<double> c( 10 );
ExpressionParser parser;
parser.assignVector( "a", a );
parser.assignVector( "c", c );
EXPECT_TRUE( parser.evaluate( "c := max(a)" ) );
for ( size_t i = 0; i < c.size(); i++ )
{
EXPECT_DOUBLE_EQ( c[i], 19.0 );
}
}
//--------------------------------------------------------------------------------------------------
///
//--------------------------------------------------------------------------------------------------
TEST( RicExpressionParserTest, SimpleVectorSum )
{
std::vector<double> a( 10 );
std::iota( a.begin(), a.end(), 10 );
std::vector<double> c( 10 );
ExpressionParser parser;
parser.assignVector( "a", a );
parser.assignVector( "c", c );
EXPECT_TRUE( parser.evaluate( "c := sum(a)" ) );
for ( size_t i = 0; i < c.size(); i++ )
{
EXPECT_DOUBLE_EQ( c[i], 145.0 );
}
}
//--------------------------------------------------------------------------------------------------
///
//--------------------------------------------------------------------------------------------------
TEST( RicExpressionParserTest, SimpleVectorAvg )
{
// a = [10..19], avg = (10+11+...+19) / 10 = 145 / 10 = 14.5
std::vector<double> a( 10 );
std::iota( a.begin(), a.end(), 10 );
std::vector<double> c( 10 );
ExpressionParser parser;
parser.assignVector( "a", a );
parser.assignVector( "c", c );
EXPECT_TRUE( parser.evaluate( "c := avg(a)" ) );
for ( size_t i = 0; i < c.size(); i++ )
{
EXPECT_DOUBLE_EQ( c[i], 14.5 );
}
}
+163 -16
View File
@@ -127,9 +127,9 @@ QString ExpressionParserImpl::parserErrorText(parser_t& parser)
}
//--------------------------------------------------------------------------------------------------
//
//
// The parser do not support single-line if statements on a vector.
// Expand a single line if-statement to a for loop over all items in the referenced
// Expand a single line if-statement to a for loop over all items in the referenced
// vectors. The script will add '[i]' as postfix to all variables, assuming all variables
// to be vectors. This statement will be put in a for loop over all elements counting
// up to size of the vector with minimum items.
@@ -144,33 +144,180 @@ QString ExpressionParserImpl::parserErrorText(parser_t& parser)
// c[i] : = if ((a[i] > 13), a[i], b[i]);
// }
//
// When aggregating functions (min, max, sum, avg) are applied to a single vector variable,
// they compute a scalar result over the whole vector. These must be pre-computed before the
// element-wise for loop, otherwise the variable gets replaced with its indexed version (e.g.
// min(b) -> min(b[i])) which changes the semantics: min(b[i]) returns b[i], not min of all b.
//
// Example with pre-computation:
// c := if(a > min(b), a, b)
//
// Expanded to:
// var ri_agg_0 := min(b);
// for (var i := 0; i < min(c[], a[], b[]); i += 1)
// {
// c[i] := if(a[i] > ri_agg_0, a[i], b[i]);
// }
//
// When an aggregating function wraps an entire if expression, the result is a scalar assigned
// to all elements of the LHS vector. This requires a two-pass expansion:
//
// c := min(if(a > 13, a, b))
//
// Expanded to:
// var ri_agg_0 := 1.7976931348623158e+308;
// for (var i := 0; i < min(a[], b[]); i += 1)
// {
// var ri_elem := if(a[i] > 13, a[i], b[i]);
// ri_agg_0 := min(ri_agg_0, ri_elem);
// }
// for (var i := 0; i < min(c[]); i += 1)
// {
// c[i] := ri_agg_0;
// }
//
//--------------------------------------------------------------------------------------------------
QString ExpressionParserImpl::expandIfStatements(const QString& expressionText)
{
QString expandedText;
// Check for pattern: c := agg_func(if(condition, true_expr, false_expr))
// Computes a scalar aggregate of the element-wise if-result and assigns it as a constant
// to all elements of c. The two closing parens )) close the if() and agg_func() respectively.
QRegularExpression outerAggIfPattern( R"(\b(\w+)\s*:=\s*(min|max|sum|avg)\(if\((.+)\)\)\s*$)",
QRegularExpression::CaseInsensitiveOption |
QRegularExpression::DotMatchesEverythingOption );
auto outerMatch = outerAggIfPattern.match( expressionText.trimmed() );
if ( outerMatch.hasMatch() )
{
QString textWithVectorBrackets = expressionText;
QString lhsVar = outerMatch.captured( 1 );
QString aggFunc = outerMatch.captured( 2 ).toLower();
QString ifArgs = outerMatch.captured( 3 );
QString listOfVars;
auto allVectorVariables = detectReferencedVariables(expressionText);
for (const QString& var : allVectorVariables)
// Detect referenced variables, excluding the outer aggregating function name itself.
// exprtk::collect_variables may treat the function name (e.g. "avg") as a variable
// when it appears in an expression context it doesn't fully recognise (avg(if(...))).
static const QStringList knownFunctions = { "min", "max", "sum", "avg" };
auto allVars = detectReferencedVariables( expressionText );
allVars.erase( std::remove_if( allVars.begin(),
allVars.end(),
[&]( const QString& v )
{ return knownFunctions.contains( v, Qt::CaseInsensitive ); } ),
allVars.end() );
QString listOfNonLhsVars;
for ( const QString& var : allVars )
{
listOfVars += QString("%1[],").arg(var);
QString regexpText = QString("\\b%1\\b").arg(var);
QRegularExpression regexp(regexpText);
if ( var != lhsVar )
listOfNonLhsVars += QString( "%1[]," ).arg( var );
}
listOfNonLhsVars = listOfNonLhsVars.left( listOfNonLhsVars.size() - 1 );
QString varWithBrackets = var + "[i]";
textWithVectorBrackets = textWithVectorBrackets.replace(regexp, varWithBrackets);
// Build element-wise version of the if expression
QString indexedIfExpr = QString( "if(%1)" ).arg( ifArgs );
for ( const QString& var : allVars )
{
if ( var == lhsVar ) continue;
QString regexpText = QString( "\\b%1\\b" ).arg( var );
QRegularExpression regexp( regexpText );
indexedIfExpr = indexedIfExpr.replace( regexp, var + "[i]" );
}
listOfVars = listOfVars.left(listOfVars.size() - 1);
QString preVarName = "ri_agg_0";
QString initVal;
QString extraVarDecl;
QString updateExpr;
expandedText = QString("for (var i := 0; i < min(%1); i += 1)\n").arg(listOfVars);
if ( aggFunc == "min" )
{
initVal = "1.7976931348623158e+308";
updateExpr = QString( "%1 := min(%1, ri_elem);" ).arg( preVarName );
}
else if ( aggFunc == "max" )
{
initVal = "-1.7976931348623158e+308";
updateExpr = QString( "%1 := max(%1, ri_elem);" ).arg( preVarName );
}
else if ( aggFunc == "sum" )
{
initVal = "0";
updateExpr = QString( "%1 := %1 + ri_elem;" ).arg( preVarName );
}
else // avg: Welford online algorithm to avoid a post-loop assignment
{
// ri_agg_0 accumulates the running mean; ri_k tracks the count.
// After the loop ri_agg_0 holds the average with no statement needed outside the loop.
initVal = "0";
extraVarDecl = "var ri_k := 0;\n";
updateExpr = "ri_k := ri_k + 1;\n"
" ri_agg_0 := ri_agg_0 + (ri_elem - ri_agg_0) / ri_k;";
}
QString expandedText;
expandedText += QString( "var %1 := %2;\n" ).arg( preVarName, initVal );
expandedText += extraVarDecl;
expandedText += QString( "for (var i := 0; i < min(%1); i += 1)\n" ).arg( listOfNonLhsVars );
expandedText += "{\n";
expandedText += QString(" %1;\n").arg(textWithVectorBrackets);
expandedText += QString( " var ri_elem := %1;\n" ).arg( indexedIfExpr );
expandedText += QString( " %1\n" ).arg( updateExpr );
expandedText += "}\n";
expandedText += QString( "for (var i := 0; i < min(%1[]); i += 1)\n" ).arg( lhsVar );
expandedText += "{\n";
expandedText += QString( " %1[i] := %2;\n" ).arg( lhsVar, preVarName );
expandedText += "}\n";
return expandedText;
}
auto allVectorVariables = detectReferencedVariables(expressionText);
QString textWithVectorBrackets = expressionText;
QString precomputedStatements;
// Pre-compute aggregating function calls with a single vector variable argument.
// For example, min(b) should be computed as a scalar before the element-wise loop.
// Without pre-computation, min(b) would expand to min(b[i]) which just returns b[i].
static const QStringList aggregatingFunctions = { "min", "max", "sum", "avg" };
int precomputedIndex = 0;
for ( const QString& func : aggregatingFunctions )
{
for ( const QString& var : allVectorVariables )
{
// Match func(var) where var is the sole argument (bounded by word boundaries)
QString patternStr = QString( "\\b%1\\(\\s*\\b%2\\b\\s*\\)" ).arg( func, var );
QRegularExpression pattern( patternStr, QRegularExpression::CaseInsensitiveOption );
if ( pattern.match( textWithVectorBrackets ).hasMatch() )
{
// Create a unique scalar variable name for the pre-computed value.
// Must start with a letter (exprtk requirement). Use ri_agg_N prefix.
QString preVarName = QString( "ri_agg_%1" ).arg( precomputedIndex++ );
precomputedStatements += QString( "var %1 := %2(%3);\n" ).arg( preVarName, func, var );
textWithVectorBrackets.replace( pattern, preVarName );
}
}
}
QString listOfVars;
for ( const QString& var : allVectorVariables )
{
listOfVars += QString( "%1[]," ).arg( var );
QString regexpText = QString( "\\b%1\\b" ).arg( var );
QRegularExpression regexp( regexpText );
QString varWithBrackets = var + "[i]";
textWithVectorBrackets = textWithVectorBrackets.replace( regexp, varWithBrackets );
}
listOfVars = listOfVars.left( listOfVars.size() - 1 );
QString expandedText = precomputedStatements;
expandedText += QString( "for (var i := 0; i < min(%1); i += 1)\n" ).arg( listOfVars );
expandedText += "{\n";
expandedText += QString( " %1;\n" ).arg( textWithVectorBrackets );
expandedText += "}\n";
return expandedText;
}