[tree] major and minor indices to long64, it was forgotten to change from 32bit of old interfaces (#14967)

Fixes https://its.cern.ch/jira/browse/ROOT-9028

[treeindex] print error when requesting out of bonds combination 31bit restriction was old, now they have separate 64-bit registers better document old2new

[roottest] adapt to new 64bit interface

[test] disable tests reaching the limits of ulong64_t
since the implicit conversion to long double later leads to platform-dependent test failures

[treeindex][nfc] mention that warning is printed at runtime when overflow detected

---------

Co-authored-by: Philippe Canal <[email protected]>

Author: ferdymercury

roottest/root/tree/index/index64.ref

@@ -1,16 +1,16 @@
 
 Processing runindex64.C...
 Tree BuildIndex returns 9
-Entry should be 3: 3
-Entry should be 6: 6
 Entries in chain: 9
 BuildIndex returns 9
-Try to get value that is not in the chain, this should return a -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
-3
+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 get value that is not in the chain, this should return a -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
-3
+Try to find the position of run=5, event=bigval in the chain, which was inserted in position 4:
+4
 (int) 0

roottest/root/tree/index/runindex64.C

@@ -8,9 +8,12 @@ const char* fname = "index64.root";
 // Apple M1 has long double == double; these values exceed its range
 // and cannot be represented as (even temporary) expression results.
 // There would be a warning if you'd try.
-static constexpr bool shortlongdouble = sizeof(long double) < 16; // was true for __APPLE__ and __arm64__
-const Long64_t bigval   = shortlongdouble ? 0xFFFFFFFFFFFF : 0xFFFFFFFFFFFFFFF; // still positive number
-const ULong64_t biguval = shortlongdouble ?  0xFFFFFFFFFFFF0 : 0xFFFFFFFFFFFFFFF0; // "negative" number
+// More info: https://github.com/root-project/roottest/commit/f3c97809c9064feccaed3844007de9e7c6a5980d and https://github.com/root-project/roottest/commit/9e3843d4bf50bc34e6e15dfe7c027f029417d6c0
+// static constexpr bool shortlongdouble = sizeof(long double) < 16; // was true for __APPLE__ and __arm64__
+// const Long64_t bigval   = shortlongdouble ?  0x0FFFFFFFFFFFF : 0x0FFFFFFFFFFFFFFF; // still positive number
+// const ULong64_t biguval = shortlongdouble ?  0xFFFFFFFFFFFF0 : 0xFFFFFFFFFFFFFFF0; // "negative" number
+const Long64_t bigval = 0xFFFFFFFFFFFF0; // larger values fail on __APPLE__ / __arm64__ because the long double is less than 16 bytes.
+// const ULong64_t biguval = bigval;
 
 int runindex64(){
 
@@ -25,20 +28,30 @@ int runindex64(){
   tree->Branch("run", &run, "run/l");
   tree->Branch("event", &event, "event/l");
 
-  ULong64_t events[] = { 1,2,3, bigval, biguval, 5 };
-  run = 5;
-  for(int i=0; i<sizeof(events)/sizeof(*events); i++){
+  ULong64_t   runs[] = { 8,5,5,5,      5, 0,      4, 6, bigval};
+  ULong64_t events[] = { 0,1,3,2, bigval, 5, bigval, 3, bigval};
+  for(size_t i=0; i<sizeof(events)/sizeof(*events); i++){
+    run = runs[i];
     event = events[i];
     tree->Fill();
   }
-  run = 4; event = bigval; tree->Fill();
-  run = 6; event = 3; tree->Fill();
-  run = biguval; event = bigval; tree->Fill();
   tree->Write();
-
+  
+  bool pass = true;
   cout<<"Tree BuildIndex returns "<<tree->BuildIndex("run", "event")<<endl;
-  cout << "Entry should be 3: " << tree->GetEntryNumberWithIndex(5,bigval) << endl;
-  cout << "Entry should be 6: " << tree->GetEntryNumberWithIndex(4,bigval) << 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();
@@ -60,8 +73,9 @@ bool test(TTree *chain)
 {
   cout<<"Entries in chain: "<<chain->GetEntries()<<endl;
   cout<<"BuildIndex returns "<<chain->BuildIndex("run", "event")<<endl;
-  cout<<"Try to get value that is not in the chain, this should return a -1:"<<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<<(int)chain->GetEntryNumberWithIndex(5, bigval)<<endl;
-  return (chain->GetEntryNumberWithIndex(500)==-1);
+  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);
 }

tree/tree/inc/TChain.h

@@ -104,7 +104,7 @@ class TChain : public TTree {
    Long64_t  GetEntries(const char *sel) override { return TTree::GetEntries(sel); }
    Int_t     GetEntry(Long64_t entry=0, Int_t getall=0) override;
    Long64_t  GetEntryNumber(Long64_t entry) const override;
-   Int_t     GetEntryWithIndex(Int_t major, Int_t minor=0) override;
+   Int_t     GetEntryWithIndex(Long64_t major, Long64_t minor=0) override;
    TFile            *GetFile() const;
    TLeaf    *GetLeaf(const char* branchname, const char* leafname) override;
    TLeaf    *GetLeaf(const char* name) override;

tree/tree/inc/TTree.h

@@ -541,7 +541,7 @@ class TTree : public TNamed, public TAttLine, public TAttFill, public TAttMarker
    virtual Long64_t        GetEstimate() const { return fEstimate; }
    virtual Int_t           GetEntry(Long64_t entry, Int_t getall = 0);
            Int_t           GetEvent(Long64_t entry, Int_t getall = 0) { return GetEntry(entry, getall); }
-   virtual Int_t           GetEntryWithIndex(Int_t major, Int_t minor = 0);
+   virtual Int_t           GetEntryWithIndex(Long64_t major, Long64_t minor = 0);
    virtual Long64_t        GetEntryNumberWithBestIndex(Long64_t major, Long64_t minor = 0) const;
    virtual Long64_t        GetEntryNumberWithIndex(Long64_t major, Long64_t minor = 0) const;
    TEventList             *GetEventList() const { return fEventList; }

tree/tree/src/TChain.cxx

@@ -972,13 +972,14 @@ Long64_t TChain::GetEntryNumber(Long64_t entry) const
 ////////////////////////////////////////////////////////////////////////////////
 /// Return entry corresponding to major and minor number.
 ///
-/// The function returns the total number of bytes read.
+/// The function returns the total number of bytes read; -1 if entry not found.
 /// If the Tree has friend trees, the corresponding entry with
 /// the index values (major,minor) is read. Note that the master Tree
 /// and its friend may have different entry serial numbers corresponding
 /// to (major,minor).
+/// \note See TTreeIndex::GetEntryNumberWithIndex for information about the maximum values accepted for major and minor
 
-Int_t TChain::GetEntryWithIndex(Int_t major, Int_t minor)
+Int_t TChain::GetEntryWithIndex(Long64_t major, Long64_t minor)
 {
    Long64_t serial = GetEntryNumberWithIndex(major, minor);
    if (serial < 0) return -1;

tree/tree/src/TTree.cxx

@@ -5998,13 +5998,14 @@ Long64_t TTree::GetEntryNumberWithIndex(Long64_t major, Long64_t minor) const
 ////////////////////////////////////////////////////////////////////////////////
 /// Read entry corresponding to major and minor number.
 ///
-///  The function returns the total number of bytes read.
+///  The function returns the total number of bytes read; -1 if entry not found.
 ///  If the Tree has friend trees, the corresponding entry with
 ///  the index values (major,minor) is read. Note that the master Tree
 ///  and its friend may have different entry serial numbers corresponding
 ///  to (major,minor).
+/// \note See TTreeIndex::GetEntryNumberWithIndex for information about the maximum values accepted for major and minor
 
-Int_t TTree::GetEntryWithIndex(Int_t major, Int_t minor)
+Int_t TTree::GetEntryWithIndex(Long64_t major, Long64_t minor)
 {
    // We already have been visited while recursively looking
    // through the friends tree, let's return.

tree/treeplayer/src/TTreeIndex.cxx

@@ -329,8 +329,11 @@ void TTreeIndex::Append(const TVirtualIndex *add, bool delaySort )
 
 
 ////////////////////////////////////////////////////////////////////////////////
-/// conversion from old 64bit indexes
-/// return true if index was converted
+/// Conversion from old 64bit indexes.
+/// Before, major and minor were stored as a single 64-bit register, with
+/// bits [0,30] for minor and bits [31,64] for major.
+/// Now, both minor and major have their own 64-bit register.
+/// \return true if index was converted
 
 bool TTreeIndex::ConvertOldToNew()
 {
@@ -353,6 +356,7 @@ bool TTreeIndex::ConvertOldToNew()
 /// In case this (friend) Tree and 'master' do not share an index with the same
 /// major and minor name, the entry serial number in the (friend) tree
 /// and in the master Tree are assumed to be the same
+/// \note An internal (intermediate) cast to double before storage as Long64_t
 
 Long64_t TTreeIndex::GetEntryNumberFriend(const TTree *parent)
 {
@@ -415,11 +419,16 @@ Long64_t TTreeIndex::FindValues(Long64_t major, Long64_t minor) const
 /// Return entry number corresponding to major and minor number.
 /// Note that this function returns only the entry number, not the data
 /// To read the data corresponding to an entry number, use TTree::GetEntryWithIndex
-/// the BuildIndex function has created a table of Double_t* of sorted values
-/// corresponding to val = major<<31 + minor;
+/// the BuildIndex function has created two tables of Long64_t sorted values
+/// (with an internal intermediate cast to LongDouble)
 /// The function performs binary search in this sorted table.
 /// If it finds a pair that maches val, it returns directly the
-/// index in the table.
+/// 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`.
+/// 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 <TTreeIndex::TTreeIndex>: In tree entry, value event possibly out of range for internal long double`
+///
 /// 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
 /// requested value, ie it will return -1 if the pair is lower than
@@ -430,7 +439,6 @@ Long64_t TTreeIndex::FindValues(Long64_t major, Long64_t minor) const
 Long64_t TTreeIndex::GetEntryNumberWithBestIndex(Long64_t major, Long64_t minor) const
 {
    if (fN == 0) return -1;
-
    Long64_t pos = FindValues(major, minor);
    if( pos < fN && fIndexValues[pos] == major && fIndexValuesMinor[pos] == minor )
       return fIndex[pos];
@@ -444,11 +452,13 @@ Long64_t TTreeIndex::GetEntryNumberWithBestIndex(Long64_t major, Long64_t minor)
 /// Return entry number corresponding to major and minor number.
 /// Note that this function returns only the entry number, not the data
 /// To read the data corresponding to an entry number, use TTree::GetEntryWithIndex
-/// the BuildIndex function has created a table of Double_t* of sorted values
-/// corresponding to val = major<<31 + minor;
+/// the BuildIndex function has created two tables of Long64_t sorted values
+/// (with an internal intermediate cast to LongDouble)
 /// The function performs binary search in this sorted table.
 /// If it finds a pair that maches val, it returns directly the
 /// 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`.
 ///
 /// See also GetEntryNumberWithBestIndex