Skip to content

Commit 0f19fe1

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)
1 parent 4f2cf43 commit 0f19fe1

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
@@ -508,6 +508,89 @@ TEST(TTreeScan, chainNameWithDifferentTreeName)
508508
}
509509
}
510510

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

0 commit comments

Comments
 (0)