Skip to content

Commit 40cef9d

Browse files
committed
[treeplayer] Fix Alt$(array) to iterate correctly when used standalone
A standalone Alt$ on a variable-length array, e.g. ```c++ t.Scan("Alt$(x,-1)"); ``` only ever evaluated the first element of each entry instead of looping over the whole array. The combined form `x:Alt$(x,-1)` worked because the sibling formula `x` drove the iteration, which made the bug easy to miss. Root cause: `TTreeFormula::ResetDimensions()` never accounted for the `kAlternate`/`kAlternateString` actions when computing `fMultiplicity`. By design, `Alt$` hides its primary operand from the TTreeFormulaManager so that, in expressions like `arr1+Alt$(arr2,0)`, `arr1` drives the loop and the alternate pads the entries where `arr2` is too short. But when `Alt$` is the *only* source of multiplicity there is no sibling to drive the loop, so the formula collapsed to a single instance. Fix: in `ResetDimensions()`, after the main operand loop, if the formula has no other multiplicity (`fMultiplicity == 0`) scan for `kAlternate`/ `kAlternateString` operands and register their primary with the manager when that primary is itself a genuine array. Adds two regression tests: **AltDollarVariableArray** (standalone `Alt$` now loops; `Alt$(x[2],-1)` keeps its fall-back behaviour) and **AltDollarPadsShorterArray** (the documented `arr1+Alt$(arr2,0)` padding still iterates over the longer array). Closes #6378. 🤖 Done with the help of [Claude Code](https://claude.com/claude-code) (Claude Opus 4.8) (cherry picked from commit 0f19fe1)
1 parent dcdc798 commit 40cef9d

2 files changed

Lines changed: 117 additions & 0 deletions

File tree

tree/treeplayer/src/TTreeFormula.cxx

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5526,6 +5526,40 @@ void TTreeFormula::ResetDimensions() {
55265526
//}
55275527

55285528
}
5529+
5530+
// Handle the case of an Alt$(primary,alternate) expression that provides
5531+
// the only source of multiplicity, i.e. it's used not alongside another
5532+
// array that drives the iteration. Example: Scan("Alt$(x,-1)"), instead of
5533+
// Scan("y-Alt$(x,-1)"). In that case the number of instances must be
5534+
// dictated by the `primary` array, so we register it with the manager. We
5535+
// only get here when fMultiplicity == 0, i.e. nothing else in this formula
5536+
// drives an iteration.
5537+
if (fMultiplicity == 0) {
5538+
for (i = 0; i < fNoper; i++) {
5539+
Int_t action = GetAction(i);
5540+
if (action == kAlternate || action == kAlternateString) {
5541+
TTreeFormula *primary = static_cast<TTreeFormula *>(fAliases.UncheckedAt(i));
5542+
R__ASSERT(primary);
5543+
switch (primary->GetManager()->GetMultiplicity()) {
5544+
case 1: // primary is a variable length array: drive a variable length loop
5545+
fMultiplicity = 1;
5546+
fManager->Add(primary);
5547+
break;
5548+
case 2: // primary is a fixed length array: drive a fixed length loop
5549+
if (fMultiplicity != 1)
5550+
fMultiplicity = 2;
5551+
fManager->Add(primary);
5552+
break;
5553+
// multiplicity 0 (scalar) or -1 (0-or-1 element): nothing to loop
5554+
// over, leave fMultiplicity at 0
5555+
}
5556+
// An Alt$(primary,alternate) expression occupies two consecutive
5557+
// operands: the `primary` action we just handled, immediately
5558+
// followed by the `alternate`, which we can skip.
5559+
++i;
5560+
}
5561+
}
5562+
}
55295563
}
55305564

55315565
////////////////////////////////////////////////////////////////////////////////

tree/treeplayer/test/regressions.cxx

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -507,6 +507,89 @@ TEST(TTreeScan, chainNameWithDifferentTreeName)
507507
}
508508
}
509509

510+
// Alt$(primary, alternate) used on its own, with 'primary' a variable-length
511+
// array, must loop over all the elements of the array instead of just the
512+
// first one. Previously the array dimension of 'primary' was not propagated to
513+
// the enclosing TTreeFormula manager when Alt$ was the only source of
514+
// multiplicity, so the formula reported a single instance per entry.
515+
TEST(TTreeFormulaRegressions, AltDollarVariableArray)
516+
{
517+
TTree t("t", "t");
518+
Int_t n = 3;
519+
Float_t x[3]{};
520+
t.Branch("n", &n);
521+
t.Branch("x", &x, "x[n]/F");
522+
x[0] = 1;
523+
x[1] = 2;
524+
x[2] = 3;
525+
t.Fill();
526+
x[0] = 4;
527+
x[1] = 5;
528+
x[2] = 6;
529+
t.Fill();
530+
n = 2;
531+
x[0] = -1;
532+
x[1] = -2;
533+
x[2] = -3;
534+
t.Fill();
535+
n = 1;
536+
x[0] = 0;
537+
t.Fill();
538+
539+
auto evalAll = [](TTree &tree, TTreeFormula &form, Long64_t entry) {
540+
tree.LoadTree(entry);
541+
const Int_t ndata = form.GetNdata();
542+
std::vector<double> values;
543+
for (Int_t i = 0; i < ndata; ++i)
544+
values.push_back(form.EvalInstance(i));
545+
return values;
546+
};
547+
548+
// 1) Standalone Alt$ over the whole array loops over its elements.
549+
{
550+
TTreeFormula form("form", "Alt$(x,-1)", &t);
551+
EXPECT_EQ(form.GetMultiplicity(), 1);
552+
const std::vector<std::vector<double>> expected{{1, 2, 3}, {4, 5, 6}, {-1, -2}, {0}};
553+
for (Long64_t entry = 0; entry < t.GetEntries(); ++entry)
554+
EXPECT_EQ(evalAll(t, form, entry), expected[entry]) << "entry " << entry;
555+
}
556+
557+
// 2) A fixed index into the variable-length array stays a single value per
558+
// entry and falls back to the alternate when the index is out of range.
559+
// This is the documented Alt$(arr[i], default) use case and must not be
560+
// turned into a zero-instance (dropped) entry.
561+
{
562+
TTreeFormula form("form", "Alt$(x[2],-1)", &t);
563+
const std::vector<std::vector<double>> expected{{3}, {6}, {-1}, {-1}}; // x[2] only exists for n==3
564+
for (Long64_t entry = 0; entry < t.GetEntries(); ++entry)
565+
EXPECT_EQ(evalAll(t, form, entry), expected[entry]) << "entry " << entry;
566+
}
567+
}
568+
569+
// Companion to AltDollarVariableArray: when Alt$ is combined with another array
570+
// that drives the iteration, the alternate must pad the shorter array rather
571+
// than shrinking the loop. See the TTree::Draw documentation for Alt$.
572+
TEST(TTreeFormulaRegressions, AltDollarPadsShorterArray)
573+
{
574+
TTree t("t", "t");
575+
Int_t n1 = 3, n2 = 2;
576+
Float_t a1[3]{10, 20, 30};
577+
Float_t a2[3]{1, 2, 3};
578+
t.Branch("n1", &n1);
579+
t.Branch("n2", &n2);
580+
t.Branch("a1", &a1, "a1[n1]/F");
581+
t.Branch("a2", &a2, "a2[n2]/F");
582+
t.Fill();
583+
584+
// The loop is driven by a1 (3 elements); Alt$(a2,0) yields a2[0],a2[1],0.
585+
TTreeFormula form("form", "a1+Alt$(a2,0)", &t);
586+
t.LoadTree(0);
587+
ASSERT_EQ(form.GetNdata(), 3);
588+
const std::vector<double> expected{11, 22, 30};
589+
for (Int_t i = 0; i < 3; ++i)
590+
EXPECT_FLOAT_EQ(form.EvalInstance(i), expected[i]) << "instance " << i;
591+
}
592+
510593
// https://github.com/root-project/root/issues/20249
511594
TEST(TTreeScan, TTreeGetBranchOfFriendTChain)
512595
{

0 commit comments

Comments
 (0)