diff --git a/roottest/root/tree/index/Makefile b/roottest/root/tree/index/Makefile index 655fc1f6c1dbe..533f4975453fb 100644 --- a/roottest/root/tree/index/Makefile +++ b/roottest/root/tree/index/Makefile @@ -1,13 +1,13 @@ # This is a template for all makefiles. #Set the list of files to be deleted by clean (Targets can also be specified).: -CLEAN_TARGETS += $(ALL_LIBRARIES) *.log *.clog newTestFile.root index64.root +CLEAN_TARGETS += $(ALL_LIBRARIES) *.log *.clog newTestFile.root index64.root indexl64.root # Set the list of target to make while testing. By default, mytest is the # only target added. If the name of the target is changed in the rules then # the name should be changed accordingly in this list. -TEST_TARGETS += chain mytest index64 +TEST_TARGETS += chain mytest index64 indexl64 # Search for Rules.mk in roottest/scripts # Algorithm: Find the current working directory and remove everything after diff --git a/roottest/root/tree/index/indexl64.ref b/roottest/root/tree/index/indexl64.ref new file mode 100644 index 0000000000000..6c0177ef5f57b --- /dev/null +++ b/roottest/root/tree/index/indexl64.ref @@ -0,0 +1,16 @@ + +Processing runindexl64.C... +Tree BuildIndex returns 9 +Entries in chain: 9 +BuildIndex returns 9 +Try to find the position of run=0, event=500 in the chain, as it does not exist, this should return a -1: +-1 +Try to find the position of run=5, event=bigval in the chain, which was inserted in position 4: +4 +Entries in chain: 9 +BuildIndex returns 1 +Try to find the position of run=0, event=500 in the chain, as it does not exist, this should return a -1: +-1 +Try to find the position of run=5, event=bigval in the chain, which was inserted in position 4: +4 +(int) 0 diff --git a/roottest/root/tree/index/runindexl64.C b/roottest/root/tree/index/runindexl64.C new file mode 100644 index 0000000000000..9617fde08fbce --- /dev/null +++ b/roottest/root/tree/index/runindexl64.C @@ -0,0 +1,79 @@ +#include "TFile.h" +#include "TChain.h" +#include "Riostream.h" + +bool test(TTree *); + +const char *fname = "indexl64.root"; +https : // github.com/root-project/root/pull/19561 + const Long64_t bigval = + 0x0FFFFFFFFFFFFFFF; // here we skip long double, so we can go higher than with runindex64.C + +int runindexl64() +{ + + /////////////////////////////////////////////////// + // Make a tree and a file and write them to disk // + /////////////////////////////////////////////////// + + TFile file(fname, "recreate"); + TTree *tree = new TTree("testTree", "my test tree"); + ULong64_t run, event; + // ULong64 is "l" + tree->Branch("run", &run, "run/l"); + tree->Branch("event", &event, "event/l"); + + // clang-format off + ULong64_t runs[] = { 8,5,5,5, 5, 0, 4, 6, bigval}; + ULong64_t events[] = { 0,1,3,2, bigval, 5, bigval, 3, bigval}; + // clang-format on + for (size_t i = 0; i < sizeof(events) / sizeof(*events); i++) { + run = runs[i]; + event = events[i]; + tree->Fill(); + } + tree->Write(); + + bool pass = true; + cout << "Tree BuildIndex returns " << tree->BuildIndex("run", "event", true, true) << endl; + for (size_t i = 0; i < sizeof(events) / sizeof(*events); i++) { + run = runs[i]; + event = events[i]; + pass &= (tree->GetEntryNumberWithIndex(run, event) == i); + } + if (!pass) { + tree->Scan("run:event", "", "colsize=30"); + for (size_t i = 0; i < sizeof(events) / sizeof(*events); i++) { + run = runs[i]; + event = events[i]; + cout << i << ": Run " << run << ", Event " << event + << " found at entry number: " << tree->GetEntryNumberWithIndex(run, event) << endl; + } + } + + test(tree); + file.Close(); + + //////////////////////////////////////////////////// + // Try loading back in as a TChain // + //////////////////////////////////////////////////// + TChain *chain = new TChain("testTree"); + chain->Add(fname); + bool result = !test(chain); + + delete chain; + + return result; +} + +bool test(TTree *chain) +{ + cout << "Entries in chain: " << chain->GetEntries() << endl; + cout << "BuildIndex returns " << chain->BuildIndex("run", "event", true, true) << endl; + cout << "Try to find the position of run=0, event=500 in the chain, as it does not exist, this should return a -1:" + << endl; + cout << chain->GetEntryWithIndex(500) << endl; + cout << "Try to find the position of run=5, event=bigval in the chain, which was inserted in position 4:" << endl; + cout << chain->GetEntryNumberWithIndex(5, bigval) << endl; + return (chain->GetEntryNumberWithIndex(500) == -1) && (chain->GetEntryNumberWithIndex(5, bigval) == 4); +} diff --git a/tree/tree/inc/TTree.h b/tree/tree/inc/TTree.h index d5735521b7229..98646a1191dea 100644 --- a/tree/tree/inc/TTree.h +++ b/tree/tree/inc/TTree.h @@ -451,7 +451,10 @@ class TTree : public TNamed, public TAttLine, public TAttFill, public TAttMarker virtual TBranch *BranchOld(const char* name, const char* classname, void* addobj, Int_t bufsize = 32000, Int_t splitlevel = 1); virtual TBranch *BranchRef(); void Browse(TBrowser*) override; - virtual Int_t BuildIndex(const char *majorname, const char *minorname = "0"); + virtual Int_t BuildIndex(const char *majorname, const char *minorname = "0", bool long64major = false, bool long64minor = false); + /// Build index with only a major formula. Minor formula will be set to "0" ie skip. + /// \see TTree::BuildIndex(const char *, const char *, bool, bool) + Int_t BuildIndex(const char *majorname, bool long64major) { return BuildIndex(majorname, "0", long64major, false); } TStreamerInfo *BuildStreamerInfo(TClass* cl, void *pointer = nullptr, bool canOptimize = true); virtual TFile *ChangeFile(TFile* file); virtual TTree *CloneTree(Long64_t nentries = -1, Option_t* option = ""); diff --git a/tree/tree/inc/TVirtualTreePlayer.h b/tree/tree/inc/TVirtualTreePlayer.h index 40e741eeb040d..9ea487a6b668f 100644 --- a/tree/tree/inc/TVirtualTreePlayer.h +++ b/tree/tree/inc/TVirtualTreePlayer.h @@ -46,7 +46,7 @@ class TVirtualTreePlayer : public TObject { TVirtualTreePlayer() { } ~TVirtualTreePlayer() override; - virtual TVirtualIndex *BuildIndex(const TTree *T, const char *majorname, const char *minorname) = 0; + virtual TVirtualIndex *BuildIndex(const TTree *T, const char *majorname, const char *minorname, bool long64major = false, bool long64minor = false) = 0; virtual TTree *CopyTree(const char *selection, Option_t *option="" ,Long64_t nentries=kMaxEntries, Long64_t firstentry=0) = 0; virtual Long64_t DrawScript(const char *wrapperPrefix, diff --git a/tree/tree/src/TTree.cxx b/tree/tree/src/TTree.cxx index fb37306f836c4..bbaabca1a55b7 100644 --- a/tree/tree/src/TTree.cxx +++ b/tree/tree/src/TTree.cxx @@ -2666,9 +2666,9 @@ void TTree::Browse(TBrowser* b) /// assigned to the TTree via the TTree::SetTreeIndex() method. /// \see TTree::SetTreeIndex() -Int_t TTree::BuildIndex(const char* majorname, const char* minorname /* = "0" */) +Int_t TTree::BuildIndex(const char* majorname, const char* minorname /* = "0" */, bool long64major, bool long64minor) { - fTreeIndex = GetPlayer()->BuildIndex(this, majorname, minorname); + fTreeIndex = GetPlayer()->BuildIndex(this, majorname, minorname, long64major, long64minor); if (fTreeIndex->IsZombie()) { delete fTreeIndex; fTreeIndex = nullptr; diff --git a/tree/treeplayer/inc/TChainIndex.h b/tree/treeplayer/inc/TChainIndex.h index 19f162b1e061b..87fc26a7ff4e4 100644 --- a/tree/treeplayer/inc/TChainIndex.h +++ b/tree/treeplayer/inc/TChainIndex.h @@ -72,7 +72,7 @@ class TChainIndex : public TVirtualIndex { TString fMinorName; // Index minor name TTreeFormula *fMajorFormulaParent; //! Pointer to major TreeFormula in Parent tree (if any) TTreeFormula *fMinorFormulaParent; //! Pointer to minor TreeFormula in Parent tree (if any) - std::vector fEntries; // descriptions of indices of trees in the chain. + std::vector fEntries; // descriptions of indices of trees in the chain. std::pair GetSubTreeIndex(Long64_t major, Long64_t minor) const; void ReleaseSubTreeIndex(TVirtualIndex* index, Int_t treeNo) const; @@ -83,7 +83,7 @@ class TChainIndex : public TVirtualIndex { public: TChainIndex(); - TChainIndex(const TTree *T, const char *majorname, const char *minorname); + TChainIndex(const TTree *T, const char *majorname, const char *minorname, bool long64major = false, bool long64minor = false); ~TChainIndex() override; void Append(const TVirtualIndex *, bool delaySort = false) override; Long64_t GetEntryNumberFriend(const TTree *parent) override; diff --git a/tree/treeplayer/inc/TTreeIndex.h b/tree/treeplayer/inc/TTreeIndex.h index 7e7b4f48dfabd..d565737d8bf6f 100644 --- a/tree/treeplayer/inc/TTreeIndex.h +++ b/tree/treeplayer/inc/TTreeIndex.h @@ -48,7 +48,7 @@ class TTreeIndex : public TVirtualIndex { public: TTreeIndex(); - TTreeIndex(const TTree *T, const char *majorname, const char *minorname); + TTreeIndex(const TTree *T, const char *majorname, const char *minorname, bool long64major = false, bool long64minor = false); ~TTreeIndex() override; void Append(const TVirtualIndex *,bool delaySort = false) override; bool ConvertOldToNew(); diff --git a/tree/treeplayer/inc/TTreePlayer.h b/tree/treeplayer/inc/TTreePlayer.h index b04eb85218895..d8aa5f7b389d0 100644 --- a/tree/treeplayer/inc/TTreePlayer.h +++ b/tree/treeplayer/inc/TTreePlayer.h @@ -61,7 +61,7 @@ class TTreePlayer : public TVirtualTreePlayer { public: TTreePlayer(); ~TTreePlayer() override; - TVirtualIndex *BuildIndex(const TTree *T, const char *majorname, const char *minorname) override; + TVirtualIndex *BuildIndex(const TTree *T, const char *majorname, const char *minorname, bool long64major = false, bool long64minor = false) override; TTree *CopyTree(const char *selection, Option_t *option ,Long64_t nentries, Long64_t firstentry) override; Long64_t DrawScript(const char* wrapperPrefix, diff --git a/tree/treeplayer/src/TChainIndex.cxx b/tree/treeplayer/src/TChainIndex.cxx index fe8ba2df2f3f1..3d33015bdd39d 100644 --- a/tree/treeplayer/src/TChainIndex.cxx +++ b/tree/treeplayer/src/TChainIndex.cxx @@ -85,7 +85,7 @@ TChainIndex::TChainIndex(): TVirtualIndex() /// ignore this warning except for special cases with prior knowledge that sorting the files and/or entries is actually /// more expensive, or just not possible. -TChainIndex::TChainIndex(const TTree *T, const char *majorname, const char *minorname) +TChainIndex::TChainIndex(const TTree *T, const char *majorname, const char *minorname, bool long64major, bool long64minor) : TVirtualIndex() { fTree = nullptr; @@ -122,7 +122,7 @@ TChainIndex::TChainIndex(const TTree *T, const char *majorname, const char *mino } } if (!index) { - chain->GetTree()->BuildIndex(majorname, minorname); + chain->GetTree()->BuildIndex(majorname, minorname, long64major, long64minor); index = chain->GetTree()->GetTreeIndex(); chain->GetTree()->SetTreeIndex(nullptr); entry.fTreeIndex = index; diff --git a/tree/treeplayer/src/TTreeIndex.cxx b/tree/treeplayer/src/TTreeIndex.cxx index f14319e456ab4..5e840188b2bb6 100644 --- a/tree/treeplayer/src/TTreeIndex.cxx +++ b/tree/treeplayer/src/TTreeIndex.cxx @@ -87,8 +87,10 @@ TTreeIndex::TTreeIndex(): TVirtualIndex() /// // Run=1234 and Event=56789 /// ~~~ /// Note that majorname and minorname may be expressions using original -/// Tree variables eg: "run-90000", "event +3*xx". However the result -/// must be integer. +/// Tree variables eg: "run-90000", "event +3*xx". These treeformulas will be calculated using +/// long double precision, and then cast to long64. If you want to directly +/// use long64 for the intermediate calculation, allowing for larger maximum indices, set long64major/minor to true. +/// Minor formula can be skipped by setting it to "0". /// /// In case an expression is specified, the equivalent expression must be computed /// when calling GetEntryWithIndex. @@ -125,7 +127,7 @@ TTreeIndex::TTreeIndex(): TVirtualIndex() /// It is possible to play with different TreeIndex in the same Tree. /// see comments in TTree::SetTreeIndex. -TTreeIndex::TTreeIndex(const TTree *T, const char *majorname, const char *minorname) +TTreeIndex::TTreeIndex(const TTree *T, const char *majorname, const char *minorname, bool long64major, bool long64minor) : TVirtualIndex() { fTree = (TTree*)T; @@ -198,8 +200,11 @@ TTreeIndex::TTreeIndex(const TTree *T, const char *majorname, const char *minorn } return ret; }; - tmp_major[i] = GetAndRangeCheck(true, i); - tmp_minor[i] = GetAndRangeCheck(false, i); + auto GetLong64 = [this](bool isMajor) { + return (isMajor ? fMajorFormula : fMinorFormula)->EvalInstance(); + }; + tmp_major[i] = long64major ? GetLong64(true) : GetAndRangeCheck(true, i); + tmp_minor[i] = long64minor ? GetLong64(false) : GetAndRangeCheck(false, i); } fIndex = new Long64_t[fN]; for(i = 0; i < fN; i++) { fIndex[i] = i; } @@ -428,6 +433,9 @@ Long64_t TTreeIndex::FindValues(Long64_t major, Long64_t minor) const /// for which the function works correctly and consistently in all platforms is `0xFFFFFFFFFFFF0`, which is less than `kMaxLong64`. /// A runtime-warning will be printed if values above this range are detected to lead to a corresponding precision loss in your current architecture: /// `Warning in : In tree entry, value event possibly out of range for internal long double` +/// This default behavior can be circumvented by setting long64major/minor to true in the TTreeIndex constructor, +/// which replaces `long double` with `Long64_t`, but it's the user responsibility as range checking will be deactivated. +/// In this case, you can go higher than `0xFFFFFFFFFFFF0` on all architectures without problems. /// /// If an entry corresponding to major and minor is not found, the function /// returns the index of the major,minor pair immediately lower than the @@ -459,6 +467,9 @@ Long64_t TTreeIndex::GetEntryNumberWithBestIndex(Long64_t major, Long64_t minor) /// index in the table, otherwise it returns -1. /// \warning Due to internal architecture details, the maximum value for `(major, minor)` /// for which the function works correctly and consistently in all platforms is `0xFFFFFFFFFFFF0`, which is less than `kMaxLong64`. +/// This default behavior can be circumvented by setting long64major/minor to true in the TTreeIndex constructor, +/// which replaces `long double` with `Long64_t`, but it's the user responsibility as range checking will be deactivated. +/// In this case, you can go higher than `0xFFFFFFFFFFFF0` on all architectures without problems. /// /// See also GetEntryNumberWithBestIndex diff --git a/tree/treeplayer/src/TTreePlayer.cxx b/tree/treeplayer/src/TTreePlayer.cxx index 8b56200328259..5766afe0fe170 100644 --- a/tree/treeplayer/src/TTreePlayer.cxx +++ b/tree/treeplayer/src/TTreePlayer.cxx @@ -153,11 +153,11 @@ TTreePlayer::~TTreePlayer() /// ignore the warning except for special cases with prior knowledge that sorting the files and/or entries is actually /// more expensive, or just not possible. -TVirtualIndex *TTreePlayer::BuildIndex(const TTree *T, const char *majorname, const char *minorname) +TVirtualIndex *TTreePlayer::BuildIndex(const TTree *T, const char *majorname, const char *minorname, bool long64major, bool long64minor) { TVirtualIndex *index; if (dynamic_cast(T)) { - index = new TChainIndex(T, majorname, minorname); + index = new TChainIndex(T, majorname, minorname, long64major, long64minor); if (index->IsZombie()) { delete index; Warning("BuildIndex", "Creating a TChainIndex unsuccessful - switching to TTreeIndex (much slower)"); @@ -165,7 +165,7 @@ TVirtualIndex *TTreePlayer::BuildIndex(const TTree *T, const char *majorname, co else return index; } - return new TTreeIndex(T,majorname,minorname); + return new TTreeIndex(T, majorname, minorname, long64major, long64minor); } ////////////////////////////////////////////////////////////////////////////////